Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Properly fix mis-associated function with vtable entry

Open PathogenDavid opened this issue 2 years ago • 0 comments

The trampolines api work revealed a subtle bug in our logic for associating VTable entries with their corresponding functions.

Right now we resolve using TranslatedVTableEntry.MethodReference. This works most of the time but in situations where the function gets duplicated (such as with LiftBaseMethodsTransformation in Mochi.DirectX) it ends up selecting the wrong function.

This wasn't noticed before because the logic in EmitFunctionContext would select the this pointer type based on the containing record regardless of where the TranslatedFunction actually came from. However with the new trampoline API the type of the this pointer is effectively determined during CreateTrampolinesTransformation.

For now I've added a hacky workaround which prefers to associate with a function which is on the same record as the vtable, but we either need to:

  • A) Formalize the __HACK__CouldResolveTo APIs.
    • I've had desire for this API before so this wouldn't be such a bad idea.
  • B) Provide a TryResolve which can be scoped to a specific declaration
  • C) Handle the situation better in transformations like LiftBaseMethodsTransformation (by duplicating the methods and re-associating the vtable entries.
    • This is probably the most correct thing to do in the current model -- and is maybe even a good idea regardless of what we do -- but I think this creates a weird gotcha if we don't also do one of the other solutions.

PathogenDavid avatar Feb 21 '22 03:02 PathogenDavid