mORMot Open Source

adriany

Member

Registered: 2014-07-25

Posts: 12

64bit pointer arithmetic bugs

Hello Arnaud,

I have been using your excellent PDF components for some time now and also SynCrypto.pas to generate MD5s. But I believe I have found a number of bugs in the 64bit version of your tools.

If you look at TMD5.Update, specifically the two Move(...) calls:

    Move(p^, Pointer(Cardinal(@in_) + 64 - t)^, len);

    Move(p^, Pointer(Cardinal(@in_) + 64 - t)^, t);
   
You are taking the 64bit address of @in_ truncating it to a 32-bit cardinal, adding an offset and converting it back to a 64bit pointer. I'm sure you see where I am going with this! If the address of @in_ is less than 4GB then this code works fine. If the address of @in_ is greater than 4GB an access violation occurs as an invalid pointer is created as the address of @in_ is truncated.

I have a dll which works perfect from a small test program, but as soon as it is run from IIS I get an access violation on the Move in SynCrypto.pas. This is probably because IIS is in a higher address space.

I'm sure you have tested under 64bit but I believe you have just been lucky and everything has been below 4GB. There is a similar bug and discussion here on StackOverflow:

http://stackoverflow.com/questions/1593 … on-on-win8

A quick search of your source code shows other pointer arithmetic bugs where the pointer is first truncated to 32bit. A simple fix for the two lines above is:

    Move(p^, Pointer(NativeUInt(@in_) + 64 - t)^, len);

    Move(p^, Pointer(NativeUInt(@in_) + 64 - t)^, t);

So simply replace the Cardinal with NativeUInt.
   
One way to test for these kinds of bugs is to do top down allocation tests as described here:

http://stackoverflow.com/questions/5442 … ows#545097

Anyway I hope you can fix your source soon and please keep up the good work, these really are excellent components.

Adrian

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: 64bit pointer arithmetic bugs

Thanks for the feedback!

Should be fixed now by http://synopse.info/fossil/info/179ad88e87e1946

Hope it helps.

adriany

Member

Registered: 2014-07-25

Posts: 12

Re: 64bit pointer arithmetic bugs

That's great, thanks for the quick response.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: 64bit pointer arithmetic bugs

I found this:

You should use the full version of FastMM and define AlwaysAllocateTopDown. This forces the calls that FastMM makes to VirtualAlloc to pass the MEM_TOP_DOWN flag. This will flush out most of your erroneous casts as runtime pointer truncation errors.
Source: http://stackoverflow.com/a/14231644/458259

Very interesting!

In fact, defining FullDebugMode is enough to enable AlwaysAllocateTopDown.
I just run the full set of regression tests (TestSQL3.dpr) under Win64 and it reported no memory leak nor corruption.
So we could assume that pointer arithmetic is correct...

adriany

Member

Registered: 2014-07-25

Posts: 12

Re: 64bit pointer arithmetic bugs

Hello Arnaud,

I just run the full set of regression tests (TestSQL3.dpr) under Win64 and it reported no memory leak nor corruption.
So we could assume that pointer arithmetic is correct...

I'm not too sure about this, what about these lines for example from TAES.DoBlocks:

    oIn := pointer(integer(pIn)+Count);
    oOut := pointer(integer(pOut)+Count);

I too am using FullDebugMode, it's turning out very useful!

Adrian

Last edited by adriany (2014-07-28 09:53:27)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: 64bit pointer arithmetic bugs

Those lines are only for PADLOCK extension of VIA cpu... so were not tested on my hardware, in fact...

Should be fixed by http://synopse.info/fossil/info/3687bb78b1

adriany

Member

Registered: 2014-07-25

Posts: 12

Re: 64bit pointer arithmetic bugs

It's great that you fix things so quickly!

I've been doing a lot of investigation today and it seems that just setting FullDebugMode is not enough to catch all 64bit pointer arithmetic errors. For example if we take the line of code in TMD5.Update where I originally had my AV:

    Move(p^, Pointer(PtrUInt(@in_) + 64 - t)^, len);

Evaluating the source and destination of the Move at runtime (with FullDebugMode on) I get:

Source = 00007FF5FFA18010
Dest = 0000000008CDF6B0

As you can see the source address is over 4GB but the destination isn't. This is probably because "in_" is a static array and not dynamically allocated at run time and is therefore in the same address space as the application. So to catch all pointer arithmetic errors the application needs to be loaded at an address higher than 4GB. This is the reason I had my AV when my dll was loaded by IIS and not when it was loaded by my test program. IIS loaded it a higher address.

The second StackOverflow link in my original message describes a registry key that will change the memory allocation to top down on a machine wide setting. But it seems that this can cause instability problems as some 64bit applications have pointer arithmetic errors!!!!

HKEY_LOCAL_MACHINE\System\CurrentControlSet\Control\Session Manager\Memory Management\AllocationPreference

Interesting stuff this! One thing I have learnt over my many years of doing this is that you cannot make assumptions about anything!

Adrian

Last edited by adriany (2014-07-28 14:34:33)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: 64bit pointer arithmetic bugs

Indeed.