Size of SUBHEAP in heap.c

[Martin Profittlich]

Hello,

when trying to fix an issue with Guitar Rig 3 on Wine (Bug 10129, "Guitar Rig 3 crashes"), I found out something odd: The crash is affected by the size of the SUBHEAP structure in dlls/ntdll/heap.c. Just by adding 8 unused bytes to SUBHEAP, GR3 does not crash anymore. Add another 8 bytes, GR3 crashes again, yet another 8 bytes - no crash and so on.

The bug surfaced with this commit:

[1d063ae18d990343fc077dcbf650add797924018]
ntdll: Remove assumptions that the subheap is at the beginning of the memory block.

I don't think the commit is the cause of the problem. It merely changed the size of SUBHEAP to a problematic value.

I don't really understand the inner workings of heap.c. Does anyone have an idea what the cause might be, or where to look for more information?

Cheers,
Martin

P.S. Link to the bug: http://bugs.winehq.org/show_bug.cgi?id=10129

This sounds like a segment / Boundry issue... how far up does it repeat this action? or have you tried??? is it an even number of blocks that the error does not occur?

Chris

-----Original Message-----
From: Martin Profittlich 
To: [e-mail]
Sent: Wed, 10 Sep 2008 12:52 pm
Subject: Size of SUBHEAP in heap.c

Hello,

when trying to fix an issue with Guitar Rig 3 on Wine (Bug 10129, "Guitar Rig 3 
crashes"), I found out something odd: The crash is affected by the size of the 
SUBHEAP structure in dlls/ntdll/heap.c. Just by adding 8 unused bytes to 
SUBHEAP, GR3 does not crash anymore. Add another 8 bytes, GR3 crashes again, yet 
another 8 bytes - no crash and so on.

The bug surfaced with this commit:

[1d063ae18d990343fc077dcbf650add797924018]
ntdll: Remove assumptions that the subheap is at the beginning of the memory 
block.

I don't think the commit is the cause of the problem. It merely changed the size 
of SUBHEAP to a problematic value.

I don't really understand the inner workings of heap.c. Does anyone have an idea 
what the cause might be, or where to look for more information?

Cheers,
Martin

P.S. Link to the bug: http://bugs.winehq.org/show_bug.cgi?id=10129

[Martin Profittlich]

> This sounds like a segment / Boundry issue... how far up does it repeat
> this action? or have you tried??? is it an even number of blocks that the
> error does not occur?

I tried this: I added DWORD reserved[N] to SUBHEAP and tested various values of N. Odd numbers trigger an assertion due to 8-byte alignment (or lack thereof). If N is 4 * x, then GR3 crashes. If N is 4 * x + 2, it does not crash. I tested this with the following values: 1 2 3 4 6 8 10 12 14 16 18 20 200 202 204 206 1020 1022 1024 1026. The sizeof (SUBHEAP) is 32 for N=0.

Martin

[Michael Karcher]

Am Mittwoch, den 10.09.2008, 18:52 +0200 schrieb Martin Profittlich:
> when trying to fix an issue with Guitar Rig 3 on Wine (Bug 10129,
> "Guitar Rig 3 crashes"), I found out something odd: The crash is
> affected by the size of the SUBHEAP structure in dlls/ntdll/heap.c.
> Just by adding 8 unused bytes to SUBHEAP, GR3 does not crash anymore.
> Add another 8 bytes, GR3 crashes again, yet another 8 bytes - no crash
> and so on.

Have you tried to run with warn+heap? In that case, on each
HeapAlloc/HeapFree, heap integrity will be checked. Also freed blocks
are directly overwritten with dummy data. The behaviour of warn+heap
might be insightful.

The size-dependent behaviour might also indicate alignment issues: There
are SSE instructions that insist on 16-byte alignment. If these are used
and our size of SUBHEAP happens to make the data only 8-byte aligned,
that might cause a crash, too.

In this particular case, I think the crash log does not really indicate
an alignment problem, but a "x/i $eip" in winedbg on the crash might
help to check further. I suspect a [edx-1] address while edx contains a
NULL pointer.

Regards,
  Michael Karcher

[Martin Profittlich]

Michael Karcher wrote:
> Have you tried to run with warn+heap? In that case, on each
> HeapAlloc/HeapFree, heap integrity will be checked. Also freed blocks
> are directly overwritten with dummy data. The behaviour of warn+heap
> might be insightful.

warn+heap does not produce any additional messages.
> The size-dependent behaviour might also indicate alignment issues: There
> are SSE instructions that insist on 16-byte alignment. If these are used
> and our size of SUBHEAP happens to make the data only 8-byte aligned,
> that might cause a crash, too.
>
> In this particular case, I think the crash log does not really indicate
> an alignment problem, but a "x/i $eip" in winedbg on the crash might
> help to check further. I suspect a [edx-1] address while edx contains a
> NULL pointer.

x/i $eip does not show anything after the address.
x $eip shows 3004290f

Regards,
Martin

[Michael Karcher]

Am Donnerstag, den 11.09.2008, 18:21 +0200 schrieb Martin Profittlich:
> > In this particular case, I think the crash log does not really indicate
> > an alignment problem, but a "x/i $eip" in winedbg on the crash might
> > help to check further. I suspect a [edx-1] address while edx contains a
> > NULL pointer.
>
> x/i $eip does not show anything after the address.

Seems like winedbg dislikes SSE2.
> x $eip shows 3004290f

Thats
  movapd [esi+eax],xmm0
if my table-lookup-skills did not fool me. movapd is an instruction that
needs the memory address 16 byte aligned. eax is 0x00040000 (2^18) [from
1], probably some kind of index into a buffer, whereas esi is 0x6d50028.
Is the size of SUBHEAP 0x28 by any chance? Then esi is a pointer to a
block allocated in a fresh heap rooted at 0x6d50000.

So you should find out where the HeapAlloc call was issued from that
returned 0x6d50028. If it was from a wine dll, that dll might need to be
fixed to ensure more strict alignment (might be some kind of audio
buffer in dsound stuff); if it is directly from guitar rig, wine has to
implement the needed alignment in its ntdll implementation.

[1]: http://bugs.winehq.org/attachment.cgi?id=8706

[Martin Profittlich]

Hello,

it took me a while, but I got some more info.

Michael Karcher wrote:
> So you should find out where the HeapAlloc call was issued from that
> returned 0x6d50028. If it was from a wine dll, that dll might need to be
> fixed to ensure more strict alignment (might be some kind of audio
> buffer in dsound stuff); if it is directly from guitar rig, wine has to
> implement the needed alignment in its ntdll implementation.

It seems to me like RtlAllocateHeap was called directly from GR3, not even through HeapAlloc. Before the crash, the following backtrace was produced:

Backtrace:
=>1 0x7ef9bb60 myGR3DebugBreak() [/home/martin/Code/wine/wine-1.1.4-findbuggr3/dlls/ntdll/heap.c:1191] in ntdll (0x0033eb44)
  2 0x7ef9e07b RtlAllocateHeap+0x58b(heap=0x4380000, flags=2, size=8388608) [/home/martin/Code/wine/wine-1.1.4-findbuggr3/dlls/ntdll/heap.c:1313] in ntdll (0x0033ebe4)
  3 0x00a2cce3 in guitar rig 3 (+0x62cce3) (0x0033ec20)
  4 0x00a2cbda in guitar rig 3 (+0x62cbda) (0x0033ecdc)
  5 0x0071a15f in guitar rig 3 (+0x31a15f) (0x0033ed18)
  6 0x0048193c in guitar rig 3 (+0x8193c) (0x0033ee24)
  7 0x0047fe54 in guitar rig 3 (+0x7fe54) (0x0033f02c)
  8 0x0047c6d8 in guitar rig 3 (+0x7c6d8) (0x0033f068)
  9 0x0041bec6 in guitar rig 3 (+0x1bec6) (0x0033f104)
  10 0x004bedd6 in guitar rig 3 (+0xbedd6) (0x0033f204)
  11 0x005b4dda in guitar rig 3 (+0x1b4dda) (0x049f5a78)
  12 0x04380100 (0x00d17460)
  13 0x004c165c in guitar rig 3 (+0xc165c) (0x006140d8)
  14 0x001ae8cb (0x8bd98b53)

RtlAllocateHeap gets called over and over again, so for my breakpoint I added myGR3DebugBreak() which only gets called when the return value is 0x7320028 (which is the content of ESI at the crash). There should actually be another debug function between myGR3DebugBreak and RtlAllocateHeap. I'm not sure why it doesn't appear in the backtrace.

So I guess this means the alignment has to be fixed in ntdll. How would that be done? Change #define ALIGNMENT to 16? Add 8 bytes to SUBHEAP? Add an assert somewhere to ensure this? Or is this more complex?

Martin

[Alexandre Julliard]

"Martin Profittlich"  writes:
> Hello,
>
> when trying to fix an issue with Guitar Rig 3 on Wine (Bug 10129, "Guitar Rig 3 crashes"), I found out something odd: The crash is affected by the size of the SUBHEAP structure in dlls/ntdll/heap.c. Just by adding 8 unused bytes to SUBHEAP, GR3 does not crash anymore. Add another 8 bytes, GR3 crashes again, yet another 8 bytes - no crash and so on.
>
> The bug surfaced with this commit:
>
> [1d063ae18d990343fc077dcbf650add797924018]
> ntdll: Remove assumptions that the subheap is at the beginning of the memory block.
>
> I don't think the commit is the cause of the problem. It merely changed the size of SUBHEAP to a problematic value.
>
> I don't really understand the inner workings of heap.c. Does anyone have an idea what the cause might be, or where to look for more information?

Probably the app expects some heap block to be 16-byte aligned, and uses
MMX vector functions that crash on unaligned data. I guess you need to
figure out where the block comes from, and why it would always be
aligned on Windows. Maybe it's large enough that Windows allocates it
with VirtualAlloc or something like that.

[Martin Profittlich]

Alexandre Julliard wrote:
> Probably the app expects some heap block to be 16-byte aligned, and uses
> MMX vector functions that crash on unaligned data. I guess you need to
> figure out where the block comes from, and why it would always be
> aligned on Windows. Maybe it's large enough that Windows allocates it
> with VirtualAlloc or something like that.

Any hints on how I would do that? I'm pretty new to this...

Regards,
Martin