Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
doReadClassNamewill now first validate that the vtable pointer points to mapped memory before attempting to read it, and throws an exception if it does not.The validation process is quite slow, but this only needs to be done once per unique vtable pointer encountered per session, as these are being cached elsewhere, and there are just not that many vtable pointers so the impact is cheap amortized over the lifetime of the executable. The choice to throw an exception instead of returning an error result is because an exception will cause a Lua traceback, which will give more diagnostic information than returning an error result would.
One call to
doReadClassNamethat was bypassing the cache was changed to not bypass it, which means this patch should actually improve DFHack's overall performance.The motivation for this change is that currently, when DFHack encountered an object, almost always in DF-owned memory, that is purportedly of a type having a vtable but where the pointer is invalid, DFHack crashes hard in this function. This results in tracebacks that make it look like DFHack is at found, and forces me to spend significant time convincing Putnam and others that it is, in fact, not DFHack that is at fault. Converting these crashes to an exception that will be caught by Lua will prevent DF from crashing when DFHack accesses this data, and so when DF eventually does crash when the invalid vtable is referenced, it won't be DFHack's fault and I won't have to explain this scenario yet again. Also, the Lua traceback will likely make it much easier to tell what DF data structure contains an invalid pointer, which should assist in both DFHack and DF troubleshooting of problems of this nature.