ntdll: implement FLG_HEAP_ENABLE_TAIL_CHECK

[Dan Kegel]

(Without a change like this, Valgrind can't properly
detect buffer overruns, but even without Valgrind,
this change will improve Wine's ability to detect
buffer overruns.)

With this change, users can enable buffer overrun
detection in Wine by setting WINE_GLOBAL_FLAGS=0x10.
This is like the overrun detection in Windows enabled by
setting NtGlobalFlags to 0x10, but checks 16 bytes
rather than 8, and works for large allocations,
not just small ones.

With this enabled, a remarkable number of tests break.

I'm posting this to wine-devel for review while I
add valgrind annotations (which should make
it easier to tell whether the errors it detects are real or spurious)
and fix the spurious breakages.

Note that this change is a no-op under normal circumstances;
without setting WINE_GLOBAL_FLAGS, you shouldn't
see any breakage.

[Eric Pouech]

Dan Kegel a écrit :
> (Without a change like this, Valgrind can't properly
> detect buffer overruns, but even without Valgrind,
> this change will improve Wine's ability to detect
> buffer overruns.)
>
> With this change, users can enable buffer overrun
> detection in Wine by setting WINE_GLOBAL_FLAGS=0x10.
> This is like the overrun detection in Windows enabled by
> setting NtGlobalFlags to 0x10, but checks 16 bytes
> rather than 8, and works for large allocations,
> not just small ones.
>
> With this enabled, a remarkable number of tests break.
>
> I'm posting this to wine-devel for review while I
> add valgrind annotations (which should make
> it easier to tell whether the errors it detects are real or spurious)
> and fix the spurious breakages.
>
> Note that this change is a no-op under normal circumstances;
> without setting WINE_GLOBAL_FLAGS, you shouldn't
> see any breakage.

Hi Dan

interesting stuff
however, you don't handle correctly HeapReAlloc(..., ZERO_MEMORY,...) 
calls when the already allocated block grows in place

=> you get a block like OGN (O = old block, G=guard for old block, N=new 
block extension)
what's zeroed out is N, whereas you should "repaint" GN into ZG (zero of 
size(N), Guard)
which may also explain lots of errors when running the tool

A+

[Dan Kegel]

On Wed, Nov 18, 2009 at 12:47 PM, Eric Pouech  wrote:
> interesting stuff
> however, you don't handle correctly HeapReAlloc(..., ZERO_MEMORY,...) calls
> when the already allocated block grows in place

Thanks for catching that.
I decided to start over and push the tail fill down into the heap
routines rather than try to put it in a wrapper; the extra
abstraction wasn't helping.
Here's a second try, this time only for large arenas, but
with valgrind integration working.  (It also has a workaround for
a strange valgrind problem that leaves an empty variable in
the environment that confuses wine.)

This time around, the only test that fails with WINE_GLOBAL_FLAG=0x10
is comctl32/dpa.c, and I could well imagine that might have a real problem
lurking.

Now to tackle the small arena case...
- Dan

[Dan Kegel]

On Wed, Nov 18, 2009 at 2:28 PM, Dan Kegel  wrote:
> Here's a second try, this time only for large arenas, but
> with valgrind integration working.  (It also has a workaround for
> a strange valgrind problem that leaves an empty variable in
> the environment that confuses wine.)

Found buglets already, so don't bother doing detailed review...

[Dan Kegel]

OK, here's something that seems to mostly work,
and even gives reasonable valgrind errors.

To use it, apply the patch and then do
  export WINE_HEAP_REDZONE=16
or so.  (Bigger values catch more problems but add more overhead.)

Only eleven tests generate warnings with that set:

runtest -q -P wine -M advapi32.dll -T ../../.. -p advapi32_test.exe.so lsa.c
 invalid arena 0x14c520, buffer 0x14c528, corrupt at 0x14c548 (byte 32 of 26)
runtest -q -P wine -M advpack.dll -T ../../.. -p advpack_test.exe.so files.c
 invalid arena 0x14d108, buffer 0x14d110, corrupt at 0x14d12e (byte 30 of 30)
 invalid arena 0x14d140, buffer 0x14d148, corrupt at 0x14d166 (byte 30 of 30)
runtest -q -P wine -M gdi32.dll -T ../../.. -p gdi32_test.exe.so path.c
 invalid arena 0x14bb40, buffer 0x14bb48, corrupt at 0x14bb48 (byte 0 of 0)
runtest -q -P wine -M hlink.dll -T ../../.. -p hlink_test.exe.so hlink.c
 invalid arena 0x154158, buffer 0x154160, corrupt at 0x154166 (byte 6 of 6)
runtest -q -P wine -M kernel32.dll -T ../../.. -p kernel32_test.exe.so
format_msg.c
 invalid arena 0x14c128, buffer 0x14c130, corrupt at 0x14c132 (byte 2 of 2)
 invalid arena 0x14c148, buffer 0x14c150, corrupt at 0x14c152 (byte 2 of 2)
 invalid arena 0x14c168, buffer 0x14c170, corrupt at 0x14c172 (byte 2 of 2)
 invalid arena 0x14c188, buffer 0x14c190, corrupt at 0x14c192 (byte 2 of 2)
runtest -q -P wine -M msi.dll -T ../../.. -p msi_test.exe.so automation.c
 invalid arena 0x164c20, buffer 0x164c28, corrupt at 0x164c2d (byte 5 of 5)
runtest -q -P wine -M netapi32.dll -T ../../.. -p netapi32_test.exe.so access.c
 invalid arena 0x110b60, buffer 0x110b68, corrupt at 0x110bea (byte 130 of 130)
runtest -q -P wine -M ole32.dll -T ../../.. -p ole32_test.exe.so moniker.c
 invalid arena 0x14fdf8, buffer 0x14fe00, corrupt at 0x14fe00 (byte 0 of 0)
...
 invalid arena 0x155ce8, buffer 0x155cf0, corrupt at 0x155cf0 (byte 0 of 0)
runtest -q -P wine -M ole32.dll -T ../../.. -p ole32_test.exe.so usrmarshal.c
 invalid arena 0x14fbc0, buffer 0x14fbc8, corrupt at 0x14fbee (byte 38 of 38)
runtest -q -P wine -M oleaut32.dll -T ../../.. -p oleaut32_test.exe.so
tmarshal.c
 invalid arena 0x1568f0, buffer 0x1568f8, corrupt at 0x1568fe (byte 6 of 6)
runtest -q -P wine -M oleaut32.dll -T ../../.. -p oleaut32_test.exe.so
usrmarshal.c
 invalid arena 0x14ebb8, buffer 0x14ebc0, corrupt at 0x14ebc0 (byte 0 of 0)

and at least some of those seem to be real errors.
To get more details, run under valgrind; it will show you the stack of
where the bad write occurred.  I'll try to provide logs tomorrow.

There are still some rough edges, but it might be useful already.

It also adds a missing valgrind annotation in mark_free_block()
that's only needed in the WINEDEBUG=+heap case,
works around a small valgrind quirk that confuses wine's environment setup,
and fixes a whitespace mistake...
- Dan