`lea` instruction tends to confuse HLIL translation

[alexrp]

Version and Platform (required):

• Binary Ninja Version: 4.1.4920-dev Personal (23c8f602)

Bug Description: BNDB

Something is clearly suspect about these this != -0x10 comparisons. I've seen similar patterns many times before, and what they all seem to have in common is the use of the lea instruction.

Disassembly:

Some things that stand out to me here:

• At 7ff69b3fe209, rbx holds a pointer to the FMallocThreadSafeProxy::lock field (typed as FCriticalSection). Yet BN seems to think that rbx holds an FMallocThreadSafeProxy pointer.
• The test at 7ff69b3fe20d is checking that the pointer to the lock field is non-null (which it can obviously never be).

There's nothing interesting in LLIL here, but switching to MLIL, it seems like the types are right:

None of the advanced IL forms reveal anything interesting AFAICT. The problem seems to occur somewhere during the translation to HLIL.

I don't know if the lea instruction is actually causing this issue, but again, whenever I see this particular pattern of incorrect decompilation, it's always involved a lea.

[alexrp]

Thinking about it more, I can sort of see what's going on here: BN is essentially reasoning that (this + 0x10) != 0 can be simplified to this != -0x10. This seems like a reasonable transformation for integer types, but I wonder if it should be prevented for pointer types for clarity? If the check was instead translated to &this->lock != 0, it would be much more obvious what's happening here.

This still doesn't really explain why the disassembly shows the wrong types at 7ff69b3fe209 though.

[plafosse]

Yes your guess is correct Before:

After:

I also agree with your assessment that we shouldn't be allowing this transform to happen with pointer. I don't however know why we're not showing this as &this->lock != 0 which would be ideal. Assigning this to Rusty to triage