Skip to content

Conversation

@ab9rf
Copy link
Member

@ab9rf ab9rf commented Jan 16, 2026

doReadClassName will 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 doReadClassName that 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.

ab9rf added 2 commits January 16, 2026 14:10
doReadClassName will now first validate that the vtable pointer points to mapped memory before attempting to read it, and throws an exception if it does not
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant