Bug 211031

Summary: valgrind doesn't show symbols for programs compiled with mingw's gcc
Product: valgrind Reporter: Dan Kegel <dank>
Component: generalAssignee: Julian Seward <jseward>
Status: REPORTED ---    
Severity: normal CC: austinenglish, cpigat242, fenrir+kde-bugs, fracting, io_fx, jeffersoncarpenter2, rbernon, tom
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Quick&Dirty patch to load debug informations from mingw exe and dll
rebased patch
Hackish patch: Add read_pe_debug_info
Hackish patch: Add read_pe_debug_info: v2
Add read_pe_debug_info: v3
the impossible happened
compiled test/pdb
DWARF in PE parsing support for WIne
DWARF in PE parsing support for Wine

Description Dan Kegel 2009-10-19 02:59:31 UTC
Version:            (using Devel)
Compiler:          i586-mingw32msvc-gcc (GCC) 4.2.1 
OS:                Linux
Installed from:    Compiled sources

When running executables under wine, valgrind generates
nice symbolic stack traces for visual-C-compiled
executables with .pdb files, but doesn't show symbols
inside mingw-compiled executables.

To repeat:
  cd memcheck/tests
  i586-mingw32msvc-gcc -g badfree.c (or, better, compile on windows with the native mingw gcc)
  valgrind --trace-children=yes --workaround-gcc296-bugs=yes -q wine a.exe

The output shows e.g.

Invalid free() / delete / delete[]
   at 0x7568048: notify_free (heap.c:222)
   by 0x756BD73: RtlFreeHeap (heap.c:1474)
   by 0x774BB4B: MSVCRT_free (heap.c:272)
   by 0x4013CC: ???
   by 0x4011E8: ???
   by 0x401232: ???
   by 0x7D081AA: start_process (process.c:953)

but should show symbols for the ???'s from the
test program.

I'm not at all sure that the mingw included with
ubuntu creates valid debugging info, as the real
mingw gdb under Windows explodes when you try to
debug files compiled with it.  But even if you compile
with native mingw gcc on windows (which does work
with native mingw's gdb on windows), you still don't 
see symbols under valgrind, you see those ???'s.
Comment 1 Dan Kegel 2009-10-19 03:14:43 UTC
(FWIW, mingw's gdb-7.0.2 does work on Wine under wineconsole,
and does show symbolic backtraces for programs compiled
with mingw's gcc.)
Comment 2 Tom Hughes 2009-10-19 09:38:35 UTC
What sort of debugging info does gcc under Windows create? stabs? DWARF? PDB files?
Comment 3 Dan Kegel 2009-10-19 13:20:50 UTC
According to
http://code.google.com/p/jrfonseca/wiki/DrMingw
gcc-for-windows generates stabs format debugging info.

(According to 
http://lists.fedoraproject.org/pipermail/fedora-mingw/2009-February/000663.html
it can also generate coff which is useful in a limited way, but that's not the default.  Also it can generate gdb-specific info with -ggdb.)

I haven't looked at the format myself, but stabs seems plausible.
Comment 4 Tom Hughes 2009-10-19 13:48:54 UTC
None of which is really supported by valgrind ;-) We do have stabs support in theory but it's horrible and crufty and doesn't work well at all.
Comment 5 fenrir+kde-bugs 2010-12-02 22:35:56 UTC
I also tried valgrind + wine to debug VLC compiled under linux (debian sid 32 bits) using  i586-mingw32msvc-gcc (4.4.4).

After looking at the generated .exe and .dll, the debugs symbols seems to be DWARF (not sure of the version).

 I have attached a very hackish patch against valgrind 3.6.0 that I have made  to load the informations. It partially works for VLC: I have the file and line number working but that's all. e.g.

==16848==  Address 0x7f0511f0 is 0 bytes inside a block of size 20 free'd
==16848==    at 0x7BC48758: RtlFreeHeap (heap.c:1750)
==16848==    by 0x753C3C5: MSVCRT_free (heap.c:290)
==16848==    by 0x6A5DE8DC: ??? (thread.c:532) <=== (thread.c is a VLC file)

 The function and or variable names are not working, and for some files, the readdwarf reject all the informations with something like:

--16848-- warning: addVar: in range 0x65b41290 .. 0x65b4138b outside segment 0x7f861000 .. 0x7f86dfff (p_module)
Comment 6 fenrir+kde-bugs 2010-12-02 22:41:54 UTC
Created attachment 54012 [details]
Quick&Dirty patch to load debug informations from mingw exe and dll

It is not meant to be applied, it's just a proof of concept for now.
Comment 7 Alexandru Munteanu 2011-01-22 14:57:51 UTC
I confirm that the patch works for showing the exe/dll name and (some?) source files and that it does not work for function names.
Tested with valgrind r11508.
Comment 8 Austin English 2014-05-22 06:06:08 UTC
[austin@localhost tests]$ cd^C
[austin@localhost tests]$ valgrind --trace-children=yes --workaround-gcc296-bugs=yes -q wine a.exe 
preloader: Warning: failed to reserve range 00110000-68000000
preloader: Warning: failed to reserve range 7f000000-82000000
==32668== Find PDB file: /tmp/valgrind_petmp32668_96fb3161 is empty
==32668== Warning: Missing or un-stat-able /home/austin/src/valgrind/memcheck/tests/a.pdb
==32668== Invalid free() / delete / delete[] / realloc()
==32668==    at 0x7BC4FE11: RtlFreeHeap (heap.c:263)
==32668==    by 0x4DABF08: MSVCRT_free (heap.c:321)
==32668==    by 0x401599: ???
==32668==    by 0x4013DD: ???
==32668==    by 0x7B85ED8B: ??? (in /usr/local/lib/wine/kernel32.dll.so)
==32668==    by 0x7B85FDA2: start_process (process.c:1097)
==32668==    by 0x7BC7F86F: ??? (in /usr/local/lib/wine/ntdll.dll.so)
==32668==    by 0x7BC8281C: call_thread_func (signal_i386.c:2630)
==32668==    by 0x7BC7F84D: ??? (in /usr/local/lib/wine/ntdll.dll.so)
==32668==    by 0x7BC53DDD: start_process (loader.c:2856)
==32668==    by 0x401B69C: ??? (in /usr/local/lib/libwine.so.1.0)
==32668==  Address 0x87654321 is on thread 1's stack
==32668== 
==32668== Invalid free() / delete / delete[] / realloc()
==32668==    at 0x7BC4FE11: RtlFreeHeap (heap.c:263)
==32668==    by 0x4DABF08: MSVCRT_free (heap.c:321)
==32668==    by 0x4015AD: ???
==32668==    by 0x4013DD: ???
==32668==    by 0x7B85ED8B: ??? (in /usr/local/lib/wine/kernel32.dll.so)
==32668==    by 0x7B85FDA2: start_process (process.c:1097)
==32668==    by 0x7BC7F86F: ??? (in /usr/local/lib/wine/ntdll.dll.so)
==32668==    by 0x7BC8281C: call_thread_func (signal_i386.c:2630)
==32668==    by 0x7BC7F84D: ??? (in /usr/local/lib/wine/ntdll.dll.so)
==32668==    by 0x7BC53DDD: start_process (loader.c:2856)
==32668==    by 0x401B69C: ??? (in /usr/local/lib/libwine.so.1.0)
==32668==  Address 0x503fd80 is on thread 1's stack

[austin@localhost tests]$ valgrind --version
valgrind-3.9.0
Comment 9 Austin English 2014-05-22 06:51:31 UTC
Created attachment 86763 [details]
rebased patch
Comment 10 Austin English 2015-05-05 19:58:06 UTC
Still in 3.10.0

I've resorted to using msvc to compile wine_gecko and using pdb's in the meantime, which gives pretty nice stacks.
Comment 11 Qian Hong 2015-10-27 19:02:59 UTC
*** Bug 354192 has been marked as a duplicate of this bug. ***
Comment 12 Qian Hong 2015-10-27 19:09:33 UTC
(In reply to Tom Hughes from comment #2)
> What sort of debugging info does gcc under Windows create? stabs? DWARF? PDB
> files?

Dear valgrind developer, I can confirm MinGW gcc creates DWARF debugging info under Windows. Any plan to implement support for DWARF in PE32?

We've recently built hundreds of open source win32 program on Wine in order to test on Wine, we use valgrind to find memory bugs and it works great except no debugging info available. Any progress is great appreciated and I would be glad to test and provide feedback.

Thanks for the great job!
Comment 13 Austin English 2015-10-28 01:13:31 UTC
This is also relevant again for wine-gecko. Upstream Firefox moved to require Visual Studio 2013+, and those newer format PDBs don't work with Wine:
https://bugs.winehq.org/show_bug.cgi?id=37746
https://bugs.winehq.org/show_bug.cgi?id=38594
https://bugs.winehq.org/show_bug.cgi?id=38604
Comment 14 Austin English 2017-02-12 01:44:31 UTC
(In reply to Austin English from comment #13)
> This is also relevant again for wine-gecko. Upstream Firefox moved to
> require Visual Studio 2013+, and those newer format PDBs don't work with
> Wine:
> https://bugs.winehq.org/show_bug.cgi?id=37746
> https://bugs.winehq.org/show_bug.cgi?id=38594
> https://bugs.winehq.org/show_bug.cgi?id=38604

Still present in wine-2.1 / valgrind-3.13.0.SVN-16222-vex-3303
Comment 15 Austin English 2019-11-28 06:36:14 UTC
As of wine-4.6, wine now supports building its dlls with mingw:
https://source.winehq.org/git/wine.git/commitdiff/a3cf86a18446e9d7df4e045b83d3aa3d9193512a

the plan is to eventually make that the default (not sure if there's a timeline for that yet, but I would guess that before next stable release is likely).

When compiling wine with mingw support, stacktraces are pretty poor. As a comparison, here's the same test with native gcc (8.3.0) versus mingw (i686-w64-mingw32-gcc (Gentoo 9.2.0-r2 p3) 9.2.0). With dlls/comctl32/tests/edit.c in wine-4.20-213-gddec23013e

native:
==10945== Invalid read of size 2
==10945==    at 0x4F0C401: LocalLock (memory.c:661)
==10945==    by 0x63FAE1A: EDIT_LockBuffer (edit.c:1195)
==10945==    by 0x640216D: EDIT_WindowProc (edit.c:4608)
==10945==    by 0x55FB4A7: ??? (in /home/austin/wine-valgrind-no-mingw/dlls/user32/user32.dll.so)
==10945==    by 0x55FBA94: call_window_proc (winproc.c:249)
==10945==    by 0x55FCEFE: WINPROC_CallProcAtoW (winproc.c:609)
==10945==    by 0x55FDB4D: WINPROC_call_window (winproc.c:956)
==10945==    by 0x55C38CE: call_window_proc (message.c:2225)
==10945==    by 0x55C6EB6: send_message (message.c:3294)
==10945==    by 0x55C9B02: SendMessageA (message.c:3517)
==10945==    by 0x5016FDF: test_EM_GETHANDLE (edit.c:3041)
==10945==    by 0x5018F22: func_edit (edit.c:3411)
==10945==    by 0x50A0581: run_test (test.h:637)
==10945==    by 0x50A0E08: main (test.h:721)
==10945==  Address 0x4952a58 is 16 bytes after a recently re-allocated block of size 32 alloc'd
==10945==    at 0x7BC6656F: notify_alloc (heap.c:260)
==10945==    by 0x7BC695B5: RtlAllocateHeap (heap.c:1725)
==10945==    by 0x59177B7: create_family (freetype.c:1594)
==10945==    by 0x59278A4: load_font_list_from_cache (freetype.c:1768)
==10945==    by 0x59283B9: WineEngInit (freetype.c:4458)
==10945==    by 0x5928CCD: DllMain (gdiobj.c:693)
==10945==    by 0x594011C: __wine_spec_dll_entry (dll_entry.c:44)
==10945==    by 0x7BC6AA95: ??? (in /home/austin/wine-valgrind-no-mingw/dlls/ntdll/ntdll.dll.so)
==10945==    by 0x7BC6EC1A: MODULE_InitDLL (loader.c:1331)
==10945==    by 0x7BC6EEBB: process_attach (loader.c:1425)
==10945==    by 0x7BC6EDFD: process_attach (loader.c:1411)
==10945==    by 0x7BC6EDFD: process_attach (loader.c:1411)
==10945==    by 0x7BC6EDFD: process_attach (loader.c:1411)
==10945==    by 0x7BC715B9: LdrInitializeThunk (loader.c:3782)
==10945==    by 0x7BC9C556: attach_thread (signal_i386.c:2746)
==10945==    by 0x7BC991C3: ??? (in /home/austin/wine-valgrind-no-mingw/dlls/ntdll/ntdll.dll.so)

mingw:
==15488== Invalid read of size 2
==15488==    at 0x7BC74A86: RtlInitNlsTables (locale.c:760)
==15488==    by 0x7125EB11: ???
==15488==    by 0x7126254F: ???
==15488==    by 0x71262871: ???
==15488==    by 0x7BC6AAD5: ??? (in /home/austin/wine-valgrind-mingw/dlls/ntdll/ntdll.dll.so)
==15488==    by 0x7BC6EC5A: MODULE_InitDLL (loader.c:1331)
==15488==    by 0x7BC6EEFB: process_attach (loader.c:1425)
==15488==    by 0x7BC6EE3D: process_attach (loader.c:1411)
==15488==    by 0x7BC6EE3D: process_attach (loader.c:1411)
==15488==    by 0x7BC715F9: LdrInitializeThunk (loader.c:3782)
==15488==    by 0x7BC9D12A: attach_thread (signal_i386.c:2746)
==15488==    by 0x7BC99D97: ??? (in /home/austin/wine-valgrind-mingw/dlls/ntdll/ntdll.dll.so)
==15488==  Address 0x2 is not stack'd, malloc'd or (recently) free'd
==15488== 

interestingly, mingw shows RtlInitNlsTables while gcc doesn't (but that's likely it, since RtlInitNlsTables is in dlls/ntdll/locale.c at line 760 and that's the ??? in the gcc output.
Comment 16 Austin English 2020-01-26 08:17:49 UTC
I recently tested llvm-mingw (from https://github.com/mstorsjo/llvm-mingw/), and built wine with llvm's pdb debug symbols. That gives similar results to gnu mingw:
==23786== Invalid read of size 2
==23786==    at 0x7B01F09E: ???
==23786==    by 0x60B93DA: ???
==23786==    by 0x60B89AC: ???
==23786==    by 0x507872F: ??? (in /home/austin/wine-valgrind-mingw/dlls/user32/user32.dll.so)
==23786==    by 0x5078CFE: call_window_proc (winproc.c:249)
==23786==    by 0x507A02C: WINPROC_CallProcAtoW (winproc.c:609)
==23786==    by 0x507ABC4: WINPROC_call_window (winproc.c:956)
==23786==    by 0x5043C8D: call_window_proc (message.c:2225)
==23786==    by 0x5046E09: send_message (message.c:3294)
==23786==    by 0x50498B9: SendMessageA (message.c:3517)
==23786==    by 0x41C0A6: ???
==23786==    by 0x4143D1: ???
==23786==    by 0x49F862: ???
==23786==    by 0x49F71C: ???
==23786==    by 0x401394: ???
==23786==    by 0x7B449E31: ??? (in /home/austin/wine-valgrind-mingw/dlls/kernel32/kernel32.dll.so)
==23786==    by 0x7B44A262: start_process (process.c:153)
==23786==    by 0x7B449E3D: ??? (in /home/austin/wine-valgrind-mingw/dlls/kernel32/kernel32.dll.so)
==23786==  Address 0x5ec1a08 is 744 bytes inside a block of size 1,024 free'd
==23786==    at 0x7BC6255E: notify_free (heap.c:268)
==23786==    by 0x7BC64B94: RtlFreeHeap (heap.c:1771)
==23786==    by 0x52123C3: free_heap_bits (bitblt.c:168)
==23786==    by 0x521CF08: nulldrv_StretchDIBits (dib.c:606)
==23786==    by 0x521D285: StretchDIBits (dib.c:636)
==23786==    by 0x500C450: create_icon_from_bmi (cursoricon.c:1265)
==23786==    by 0x500D26B: CURSORICON_Load (cursoricon.c:1867)
==23786==    by 0x500F153: LoadImageW (cursoricon.c:3065)
==23786==    by 0x6111289: ???
==23786==    by 0x60B1CE7: ???
==23786==    by 0x61221E0: ???
==23786==    by 0x7BC667C9: ??? (in /home/austin/wine-valgrind-mingw/dlls/ntdll/ntdll.dll.so)
==23786==    by 0x7BC6A582: MODULE_InitDLL (loader.c:1331)
==23786==    by 0x7BC6A80A: process_attach (loader.c:1425)
==23786==    by 0x7BC6CBD2: LdrLoadDll (loader.c:3094)
==23786==    by 0x7B01436E: ???
==23786==    by 0x7B014279: ???
==23786==    by 0x7B01419B: ???
==23786==    by 0x7B01415E: ???
==23786==    by 0x4145B5: ???
==23786==  Block was alloc'd at
==23786==    at 0x7BC62514: notify_alloc (heap.c:260)
==23786==    by 0x7BC65377: RtlAllocateHeap (heap.c:1725)
==23786==    by 0x5212AB4: convert_bits (bitblt.c:183)
==23786==    by 0x521D045: nulldrv_StretchDIBits (dib.c:589)
==23786==    by 0x521D285: StretchDIBits (dib.c:636)
==23786==    by 0x500C450: create_icon_from_bmi (cursoricon.c:1265)
==23786==    by 0x500D26B: CURSORICON_Load (cursoricon.c:1867)
==23786==    by 0x500F153: LoadImageW (cursoricon.c:3065)
==23786==    by 0x6111289: ???
==23786==    by 0x60B1CE7: ???
==23786==    by 0x61221E0: ???
==23786==    by 0x7BC667C9: ??? (in /home/austin/wine-valgrind-mingw/dlls/ntdll/ntdll.dll.so)
==23786==    by 0x7BC6A582: MODULE_InitDLL (loader.c:1331)
==23786==    by 0x7BC6A80A: process_attach (loader.c:1425)
==23786==    by 0x7BC6CBD2: LdrLoadDll (loader.c:3094)
==23786==    by 0x7B01436E: ???
==23786==    by 0x7B014279: ???
==23786==    by 0x7B01419B: ???
==23786==    by 0x7B01415E: ???
==23786==    by 0x4145B5: ???
==23786== 

which is interesting, given that valgrind has some pdb support. LLVM uses the publicly documented pdb info from https://github.com/Microsoft/microsoft-pdb, fwiw. Note that I did run until other issues in some tests with this setup, see bug 416779.

Tested with VALGRIND_3_15_0-193-gfe6805efc
Comment 17 Jefferson Carpenter 2020-03-22 17:04:11 UTC
(In reply to fenrir+kde-bugs from comment #5)
>  I have attached a very hackish patch against valgrind 3.6.0 that I have
> made  to load the informations. It partially works for VLC: I have the file
> and line number working but that's all. e.g.

In order to get the function names, you may have to iterate through the PE/COFF symbol table, and insert the symbols into the debuginfo.

I have also attached a very hackish patch, one that reads debug info from the PE32 file itself having been compiled with mingw.  It does not support PE64.  In addition to being hackish, it contains numerous lies.  It currently assumes that the .text section is sections[0] for no particular reason.  It also adds a DebugInfoMap covering the range [image_base, image_base + size_of_image) assuming that's where the image will be during execution (I think a better way to do that would be to intercept the call that moves it there, be that a memcpy, memmove, mmap, or what have you).

Furthermore, I was not able to tell if there is a way to get the ending address of symbols - only the starting address.  So I have it pushing all of the symbols into an array, sorting it descending by symbol starting address, and setting each symbol's address range to go from its starting address to just before starting address of the next symbol.  (The last symbol is then said to end at the end of the text section.)
Comment 18 Jefferson Carpenter 2020-03-22 17:05:13 UTC
Created attachment 126950 [details]
Hackish patch: Add read_pe_debug_info
Comment 19 Jefferson Carpenter 2020-03-24 00:48:04 UTC
Created attachment 126977 [details]
Hackish patch: Add read_pe_debug_info: v2

Fixed some of the most significant lies from my first patch.  No longer assumes .text is the first section.  Instead of "main.exe" uses the basename of the executable as the soname (I don't know whether this is even correct).  Uses correct-looking arenas for allocations.  Correctly sorts symbols in descending order and corrects addSym calls.
Comment 20 Austin English 2020-03-24 07:49:07 UTC
(In reply to Jefferson Carpenter from comment #19)
> Created attachment 126977 [details]
> Hackish patch: Add read_pe_debug_info: v2
> 
> Fixed some of the most significant lies from my first patch.  No longer
> assumes .text is the first section.  Instead of "main.exe" uses the basename
> of the executable as the soname (I don't know whether this is even correct).
> Uses correct-looking arenas for allocations.  Correctly sorts symbols in
> descending order and corrects addSym calls.

Fails to build:
make[3]: *** No rule to make target 'm_debuginfo/readpe.c', needed by 'm_debuginfo/libcoregrind_amd64_linux_a-readpe.o'.  Stop.

(try 1 compiles okay)
Comment 21 Jefferson Carpenter 2020-03-24 20:11:54 UTC
(In reply to Austin English from comment #20)
> Fails to build:
> make[3]: *** No rule to make target 'm_debuginfo/readpe.c', needed by
> 'm_debuginfo/libcoregrind_amd64_linux_a-readpe.o'.  Stop.
> 
> (try 1 compiles okay)

I created the patch using git format-patch, so you should apply it using either

$ git apply 0001-Add-read_pe_debug_info-V2.patch

or

$ patch -p 1 <0001-Add-read_pe_debug_info-V2.patch

I'm still not clear on the best way to create patch files.
Comment 22 Jefferson Carpenter 2020-03-24 21:31:56 UTC
(In reply to Jefferson Carpenter from comment #21)
> I created the patch using git format-patch, so you should apply it using
> either
> 
> $ git apply 0001-Add-read_pe_debug_info-V2.patch
> 
> or
> 
> $ patch -p 1 <0001-Add-read_pe_debug_info-V2.patch
> 
> I'm still not clear on the best way to create patch files.

Here's what I think happened - by default patch strips all directories off of the path in the patch file.  Since Makefile.am does exist in the root directory patch attempts to apply HUNK 1 but fails because HUNK 1 is actually for the coregrind/Makefile.am file.  (Then, since coregrind/m_debuginfo/debuginfo.c is modified but debuginfo.c does not exist in the root directory of valgrind, patch has to ask for its directory.  priv_readpe.h and readpe.c are both new files, but patch again strips their directories and places them in the root directory).  Moral of the story: run patch with a -p option if the patch file has knowledge of the directory structure of your project.
Comment 23 Austin English 2020-04-30 09:51:33 UTC
I usually use -p1, so not sure what was wrong..anyway, recently tried again, with a clean tree, and it applies fine, so yeah, it was something on my end.

With the patch, I see an improvement using badfree.c from the original post:

No patch:
==10542== Invalid free() / delete / delete[] / realloc()
==10542==    at 0x7BC70261: notify_free (heap.c:268)
==10542==    by 0x7BC70261: RtlFreeHeap.part.7 (???:0)
==10542==    by 0x7BC72034: RtlFreeHeap (heap.c:1748)
==10542==    by 0x516469D: msvcrt_heap_free (heap.c:114)
==10542==    by 0x51651DF: MSVCRT_free (heap.c:414)
==10542==    by 0x401599: ???
==10542==    by 0x401395: ???
==10542==    by 0x7B4504F1: ??? (in /usr/local/lib64/wine/kernel32.dll.so)
==10542==    by 0x7B4508DF: start_process (process.c:153)
==10542==    by 0x7B4504FD: ??? (in /usr/local/lib64/wine/kernel32.dll.so)
==10542==  Address 0x87654321 is on thread 1's stack
==10542== 

Patch:
==8665== Invalid free() / delete / delete[] / realloc()
==8665==    at 0x7BC70261: notify_free (heap.c:268)
==8665==    by 0x7BC70261: RtlFreeHeap.part.7 (???:0)
==8665==    by 0x7BC72034: RtlFreeHeap (heap.c:1748)
==8665==    by 0x516469D: msvcrt_heap_free (heap.c:114)
==8665==    by 0x51651DF: MSVCRT_free (heap.c:414)
==8665==    by 0x401599: _main (badfree.c:12)
==8665==    by 0x401395: ??? (crtexe.c:339)
==8665==    by 0x7B4504F1: ??? (in /usr/local/lib64/wine/kernel32.dll.so)
==8665==    by 0x7B4508DF: start_process (process.c:153)
==8665==    by 0x7B4504FD: ??? (in /usr/local/lib64/wine/kernel32.dll.so)
==8665==  Address 0x87654321 is on thread 1's stack
==8665== 

namely badfree.c:12 and crtexe.c:339. Note that there's some spurious output from the patch, e.g.,:
size: 130284

@Julian, if feasible, it would be great to target this for valgrind-3.16.0.
Comment 24 Austin English 2020-04-30 11:05:21 UTC
Getting a failed assertion in some tests:

valgrind: m_debuginfo/debuginfo.c:694 (check_CFSI_related_invariants): Assertion '!ranges_overlap(map->avma, map->size, map2->avma, map2->size)' failed.
valgrind: DiCfsi invariant (1) verification failed

host stacktrace:
==6286==    at 0x58041CF9: show_sched_status_wrk (m_libcassert.c:406)
==6286==    by 0x58041E20: report_and_quit (m_libcassert.c:477)
==6286==    by 0x58041F14: vgPlain_assert_fail (m_libcassert.c:543)
==6286==    by 0x58075670: check_CFSI_related_invariants (debuginfo.c:692)
==6286==    by 0x58075670: di_notify_ACHIEVE_ACCEPT_STATE (debuginfo.c:992)
==6286==    by 0x58075670: vgPlain_di_notify_mmap (debuginfo.c:1326)
==6286==    by 0x580A8D51: vgModuleLocal_generic_PRE_sys_mmap (syswrap-generic.c:2400)
==6286==    by 0x580B5F25: vgSysWrap_x86_linux_sys_mmap2_before (syswrap-x86-linux.c:987)
==6286==    by 0x580A45BE: vgPlain_client_syscall (syswrap-main.c:1914)
==6286==    by 0x580A090C: handle_syscall (scheduler.c:1208)
==6286==    by 0x580A246F: vgPlain_scheduler (scheduler.c:1526)
==6286==    by 0x580F7312: thread_wrapper (syswrap-linux.c:101)
==6286==    by 0x580F7312: run_a_thread_NORETURN (syswrap-linux.c:154)

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable syscall 192 (lwpid 6286)
==6286==    at 0x435A253: mmap64 (mmap64.c:56)
==6286==    by 0x7BCAAFFE: map_pe_header (virtual.c:1491)
==6286==    by 0x7BCAC3E6: map_image (virtual.c:1569)
==6286==    by 0x7BCADA02: virtual_map_section (virtual.c:1838)
==6286==    by 0x7BC68BFC: open_dll_file (loader.c:2515)
==6286==    by 0x7BC68DA8: search_dll_file (loader.c:3074)
==6286==    by 0x7BC6923F: find_dll_file (loader.c:3154)
==6286==    by 0x7BC6CBF9: load_dll (loader.c:3186)
==6286==    by 0x7BC6C0F8: import_dll (loader.c:801)
==6286==    by 0x7BC6C64B: fixup_imports (loader.c:1165)
==6286==    by 0x7BC6C8AF: load_native_dll (loader.c:2570)
==6286==    by 0x7BC6CB26: load_builtin_dll (loader.c:2930)
==6286==    by 0x7BC6CE08: load_dll (loader.c:3231)
==6286==    by 0x7BC6C0F8: import_dll (loader.c:801)
==6286==    by 0x7BC6C64B: fixup_imports (loader.c:1165)
==6286==    by 0x7BC6E372: LdrInitializeThunk (loader.c:3987)
==6286==    by 0x7BC99953: attach_thread (signal_i386.c:3050)
==6286==    by 0x7BC9634F: ??? (in /home/austin/wine-valgrind/dlls/ntdll/ntdll.dll.so)
client stack range: [0x4A62000 0x4B5FFFF] client SP: 0x4B5D22C
valgrind stack range: [0x881D3000 0x882D2FFF] top usage: 7092 of 1048576


Note: see also the FAQ in the source distribution.
It contains workarounds to several common problems.
In particular, if Valgrind aborted or crashed after
identifying problems in your program, there's a good chance
that fixing those problems will prevent Valgrind aborting or
crashing, especially if it happened in m_mallocfree.c.

e.g., avifil32/tests/api.c
Comment 25 Jefferson Carpenter 2020-05-04 12:33:28 UTC
Removed the spurious output.  Did not manually add a mapping, to fix the "DiCfsi invariant (1) verification failed" message.  Also removed the dwarf3 code - the patch still seems to work for me, and native PE files will of course not have dwarf3 debug info.

There are still some things I'm very unsure about whether this patch does correctly.

1: Latest version, I changed the search_all_symtabs logic to search for text sections using di->text_present and so on, instead of looking at di->fsm properties.  It seems logical, but since I'm not very familiar with the Valgrind code it could be a breaking change.  Someone would have to look at it.

2: The way read_pe_debug_info is called into is IMO a messy hack.  I did add a function for detecting whether a binary is a PE object file, but I think passing a boolean argument into di_notify_ACHIEVE_ACCEPT_STATE is not the cleanest way to do it.  The way PDB debug info is requested is through client requests, so maybe PE debug info should be requested the same way.

3: There's generally a little bit of code duplication.  Some duplicate "if (!ML_(img_valid)(...))" statements I could try to combine or replace with a macro.  Also the PE executable file format is modeled both in readpdb.c and readpe.c currently and should be factored out into a header.  I just didn't bother to do that at this early stage.
Comment 26 Jefferson Carpenter 2020-05-04 12:36:09 UTC
Created attachment 128132 [details]
Add read_pe_debug_info: v3

Apparently the attachment didn't attach.  Attached it again.
Comment 27 Austin English 2020-05-05 12:04:24 UTC
No assertions so far (running still, up to d3d8:visual).

Stacktraces look good. Lots of (valid) warnings for missing symbols in wine:

symbol table size is 0.  Not reading debug information for /home/austin/.wine-valgrind/drive_c/windows/system32/winex11.drv
==14040== LOAD_PDB_DEBUGINFO: Find PDB file: /tmp/valgrind_petmp14040_6f021bc4 is empty
==14040== Warning: Missing or un-stat-able /home/austin/.wine-valgrind/drive_c/windows/system32/winex11.pdb


The symbol table size warning should match current format, of course. I haven't checked, but it likely corresponds to the un-stat-able message, so is it really needed?
Comment 28 Austin English 2020-05-05 12:22:25 UTC
Created attachment 128170 [details]
the impossible happened

Upon manual review, it didn't assert, but the impossible happened:
                                    
PUTI(136:8xI8)[t1,0] = 0x0:I8                                                                        
                                                                                                     
vex: the `impossible' happened:                                                                      
   stmt_is_guardable: unhandled stmt                                                                 
vex storage: T total 9579053828 bytes allocated                                                      
vex storage: P total 496 bytes allocated                                                             
                                                                                                     
valgrind: the 'impossible' happened:                                                                 
   LibVEX called failure_exit().                                                                     
                                     

in dlls/atl/tests/registrar.c
Comment 29 Jefferson Carpenter 2020-05-25 18:54:09 UTC
(In reply to Austin English from comment #28)
> Created attachment 128170 [details]
> the impossible happened
> 
> Upon manual review, it didn't assert, but the impossible happened:
>                                     
> PUTI(136:8xI8)[t1,0] = 0x0:I8                                               
> 
>                                                                             
> 
> vex: the `impossible' happened:                                             
> 
>    stmt_is_guardable: unhandled stmt                                        
> 
> vex storage: T total 9579053828 bytes allocated                             
> 
> vex storage: P total 496 bytes allocated                                    
> 
>                                                                             
> 
> valgrind: the 'impossible' happened:                                        
> 
>    LibVEX called failure_exit().                                            
> 
>                                      
> 
> in dlls/atl/tests/registrar.c

I ran the same test (valgrind --trace-children=yes ../../../../wine-source/tools/runtest -q -P ../../../loader/wine -T ../../.. -M atl.dll -p atl_test.exe.so registrar), and the impossible didn't happen on my machine.  (My configure command included --without-mingw because that's what causes the .exe.so files to be generated).  Can you confirm that the bug happens with my patch applied and not on master?  It seems like some spooky action at a distance if my patch caused new problems generating IR.
Comment 30 Austin English 2020-05-31 05:35:39 UTC
Created attachment 128946 [details]
compiled test/pdb

(In reply to Jefferson Carpenter from comment #29)
> (In reply to Austin English from comment #28)
> > Created attachment 128170 [details]
> > the impossible happened
> > 
> > Upon manual review, it didn't assert, but the impossible happened:
> >                                     
> > PUTI(136:8xI8)[t1,0] = 0x0:I8                                               
> > 
> >                                                                             
> > 
> > vex: the `impossible' happened:                                             
> > 
> >    stmt_is_guardable: unhandled stmt                                        
> > 
> > vex storage: T total 9579053828 bytes allocated                             
> > 
> > vex storage: P total 496 bytes allocated                                    
> > 
> >                                                                             
> > 
> > valgrind: the 'impossible' happened:                                        
> > 
> >    LibVEX called failure_exit().                                            
> > 
> >                                      
> > 
> > in dlls/atl/tests/registrar.c
> 
> I ran the same test (valgrind --trace-children=yes
> ../../../../wine-source/tools/runtest -q -P ../../../loader/wine -T ../../..
> -M atl.dll -p atl_test.exe.so registrar), and the impossible didn't happen
> on my machine.  (My configure command included --without-mingw because
> that's what causes the .exe.so files to be generated).  Can you confirm that
> the bug happens with my patch applied and not on master?  It seems like some
> spooky action at a distance if my patch caused new problems generating IR.

After some more testing, it only occurs if:
./configure --with-mingw CROSSDEBUG=pdb

is used for the build.

I'm not sure why the other log shows .exe.so. Maybe a dirty tree, or maybe wine's build system changed.. I've attached the exe and pdb file, it should be enough to reproduce without having to get llvm-mingw/rebuild wine.

$ export VALGRIND_OPTS="-q --trace-children=yes --track-origins=yes --gen-suppressions=all --suppressions=/home/austin/wine-valgrind/tools/valgrind/valgrind-suppressions-ignore --suppressions=/home/austin/wine-valgrind/tools/valgrind/valgrind-suppressions-external --suppressions=/home/austin/wine-valgrind/tools/valgrind/valgrind-suppressions-known-bugs --suppressions=/home/austin/wine-valgrind/tools/valgrind/valgrind-suppressions-gecko --leak-check=full --num-callers=20 --workaround-gcc296-bugs=yes --vex-iropt-register-updates=allregs-at-mem-access"

# make sure to use the full path for wine:
$ valgrind /usr/local/bin/wine avifil32_test.exe api

preloader: Warning: failed to reserve range 00110000-68000000
preloader: Warning: failed to reserve range 7f000000-82000000
0140:err:heap:HEAP_GetPtr Invalid heap (nil)!
size: 136926
==31466== LOAD_PDB_DEBUGINFO: Find PDB file: /tmp/valgrind_petmp31466_1cb84028 is empty
==31466== Warning: Missing or un-stat-able /home/austin/.wine/drive_c/windows/system32/kernelbase.pdb
size: 135558
==31466== LOAD_PDB_DEBUGINFO: Find PDB file: /tmp/valgrind_petmp31466_1cb84028 is empty
==31466== Warning: Missing or un-stat-able /usr/local/lib64/wine/kernelbase.pdb
symbol table size is 0.  Not reading debug information for /tmp/tmp.emEgO3p387/bug-211031-avifil32-test/avifil32_test.exe

valgrind: m_debuginfo/debuginfo.c:1672 (vgPlain_di_notify_pdb_debuginfo): Assertion 'di && !di->fsm.have_rx_map && !di->fsm.have_rw_map' failed.

host stacktrace:
==31466==    at 0x58041CF9: show_sched_status_wrk (m_libcassert.c:406)
==31466==    by 0x58041E20: report_and_quit (m_libcassert.c:477)
==31466==    by 0x58041F14: vgPlain_assert_fail (m_libcassert.c:543)
==31466==    by 0x58076146: vgPlain_di_notify_pdb_debuginfo (debuginfo.c:1672)
==31466==    by 0x580A252D: do_client_request (scheduler.c:2121)
==31466==    by 0x580A252D: vgPlain_scheduler (scheduler.c:1516)
==31466==    by 0x580F72C2: thread_wrapper (syswrap-linux.c:101)
==31466==    by 0x580F72C2: run_a_thread_NORETURN (syswrap-linux.c:154)

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable (lwpid 31466)
==31466==    at 0x7BCD6494: map_image (virtual.c:1749)
==31466==    by 0x7BCD6A71: virtual_map_section (virtual.c:1835)
==31466==    by 0x7BC79293: open_dll_file (loader.c:2371)
==31466==    by 0x7BC79D9E: find_dll_file (loader.c:3014)
==31466==    by 0x7BC7FDAB: load_dll (loader.c:3044)
==31466==    by 0x7BC84857: __wine_process_init (loader.c:4426)
==31466==    by 0x7BC84F42: __wine_set_unix_funcs (loader.c:4488)
==31466==    by 0x7C001341: main (main.c:285)
client stack range: [0xFECB8000 0xFECBDFFF] client SP: 0xFECBA900
valgrind stack range: [0x881FD000 0x882FCFFF] top usage: 7372 of 1048576
Comment 31 Rémi Bernon 2020-11-30 11:34:21 UTC
Created attachment 133748 [details]
DWARF in PE parsing support for WIne

Hi!

I wasn't aware there was already some work in progress (TBF I didn't look for it), and I sent these patches to the valgrind-developers mailing list earlier today. They implement DWARF parsing from PE files in a similar way it is done for ELF files, to support the use case reported here.

It's probably a bit redundant with some of the patches attached here, but I'm re-using the code from readpdb.c instead of duplicating the structure definitions. Also, because Wine mmap calls are a bit difficult to interpret from Valgrind side, I'm using the PDB hook to read PE files when no PDB are found.

The first patch fixes the readpdb.c structures definitions, in the same way as some of the patches for https://bugs.kde.org/show_bug.cgi?id=253657 do. For some reason these fixes never landed.
Comment 32 Austin English 2020-12-18 07:56:42 UTC
Hi Remi,

Using both wine patches:
https://source.winehq.org/patches/data/196764
https://source.winehq.org/patches/data/196765

and valgrind patches:
https://sourceforge.net/p/valgrind/mailman/message/37164939/
https://sourceforge.net/p/valgrind/mailman/message/37164938/
https://sourceforge.net/p/valgrind/mailman/message/37164940/

on top of wine-6.0-rc2 and valgrind-VALGRIND_3_15_0-405-g41504d33d, with llvm-mingw, I don't get symbols:
==8917== 2,048 bytes in 1 blocks are possibly lost in loss record 86 of 92
==8917==    at 0x7BC23550: ??? (in /home/austin/wine-valgrind-pdb/dlls/ntdll/ntdll.dll)
==8917==    by 0x7BC2F91F: ??? (in /home/austin/wine-valgrind-pdb/dlls/ntdll/ntdll.dll)
==8917==    by 0x7BC29921: ??? (in /home/austin/wine-valgrind-pdb/dlls/ntdll/ntdll.dll)
==8917==    by 0x7BC2E1F7: ??? (in /home/austin/wine-valgrind-pdb/dlls/ntdll/ntdll.dll)
==8917==    by 0x7BC2DC01: ??? (in /home/austin/wine-valgrind-pdb/dlls/ntdll/ntdll.dll)
==8917==    by 0x46E061E: start_main_thread (loader.c:1580)
==8917==    by 0x46E061E: __wine_main (???:0)
==8917==    by 0x7D001287: main (main.c:157)
==8917== 


compared to non pdb build:
==31534== 2,048 bytes in 1 blocks are possibly lost in loss record 100 of 108
==31534==    at 0x7BC51E61: RtlAllocateHeap (heap.c:261)
==31534==    by 0x7BC625C3: init_load_order (loadorder.c:234)
==31534==    by 0x7BC625C3: get_load_order (???:0)
==31534==    by 0x7BC5DA27: load_dll (loader.c:2644)
==31534==    by 0x7BC5FFD1: process_init (loader.c:4017)
==31534==    by 0x7BC61D7B: __wine_set_unix_funcs (loader.c:4114)
==31534==    by 0x46E061E: start_main_thread (loader.c:1580)
==31534==    by 0x46E061E: __wine_main (???:0)
==31534==    by 0x7D001287: main (main.c:157)
==31534== 

(that's dlls/avifil32/tests/api.c)
Comment 33 Austin English 2021-01-08 07:09:10 UTC
To follow up, I misunderstood the scope of Remi's work.

The patchset *does* work for DWARF symbols in PE binaries in modern wine (i.e., with wine having a mingw compiled ntdll).

PDB support is a separate problem and should not stop the patches from going in.
Comment 34 Rémi Bernon 2021-04-08 12:14:27 UTC
Just a reminder that this is still broken and the patches attached should fix the missing symbols when Wine is built with MinGW and DWARF debug info (still requires some Wine-side fixes too).

I believe the Valgrind patches still apply without conflict.

The only remaining issue that I could see is that Wine munmaps its DLLs whenever they are unloaded, and that makes Valgrind drop all symbols in the range, making some later leak reports miss the symbols. But that's possibly an issue to be worked around on Wine side.
Comment 35 Rémi Bernon 2021-04-08 12:17:27 UTC
Talked too soon, it now doesn't build anymore, I'll update the patches.
Comment 36 Rémi Bernon 2021-04-08 12:31:19 UTC
Created attachment 137430 [details]
DWARF in PE parsing support for Wine

Patches rebased on top of 5e41080720e451cefc2265c64f82fb77a9f29151.