pleriche / FastMM4

A memory manager for Delphi and C++ Builder with powerful debugging facilities
447 stars 158 forks source link

Runtime-crash when using EnableMemoryLeakReportingUsesQualifiedClassName #88

Closed AtlasHackert closed 3 years ago

AtlasHackert commented 3 years ago

I'm on Delphi 7, and I'm getting a runtime-crash on exit when there's a memory leak if EnableMemoryLeakReportingUsesQualifiedClassName has been enabled. The problem is the following line in the AppendClassNameToBuffer function: UnitName := @PClassData(PByte(LClassInfo) + 2 + PByte(PByte(LClassInfo) + 1)^).UnitName;

Due to either Delphi 7 or my project settings, the pointer arithmetic fails, probably raising an exception that tries to allocate memory, resulting in an invalid operation because the MM has already been uninstalled.

I fixed it by changing the line to: UnitName := @PClassData(PByte(LClassInfo) + 2 + Integer(PByte(PByte(LClassInfo) + 1)^)).UnitName;

PatrickvL commented 3 years ago

Integer must not be used for portable pointer math, use IntPtr or NativeInt instead. Also, a few more braces would help in this expression.

Edit : oh, and if indeed this additional type cast is required, mention that in a comment to prevent someone removing this seemingly needless cast later on

AtlasHackert commented 3 years ago

Good suggestions. Both datatypes didn't exist in Delphi 7, hence my lack of knowledge of them. But I see FastMM4.pas defines them, so IntPtr is probably a better choice, yes.

And adding a comment seems smart; I'll leave that up to the developer.

AtlasHackert commented 3 years ago

Oops, this issue is sort-of a duplicate: https://github.com/pleriche/FastMM4/issues/87

pleriche commented 3 years ago

Hi, I've made a few changes that I believe should fix the issues with Delphi 7 and older.

AtlasHackert commented 3 years ago

Confirmed: your change works perfectly in my Delphi 7.

pleriche commented 3 years ago

Thank you for the feedback. Closing the ticket.