Created attachment 52366 [details] patchset for preliminary registers support in AMD64 Version: 3.5.0 (using KDE 4.5.0) OS: Linux the attached patch set add support in amd64 VEX for: - definition of segment registers (cs...gs) - move from/to registers - push/pop registers - iret (as poorly implemented as on x86) what's missing (not included in this patchset to be further developped): - gdt/ldt/idt support - segmented addressing (required in wine as on Windows gs points to the Process Environment Block, so assembly is often generated to access PEB fields with gs:offset type of addressing). when done, current kludges (FS_ZERO and GS_0x60) are to be removed Reproducible: Didn't try Steps to Reproduce: run any wine/amd64 program under valgrind
Some questions: * is this the final version of the patch you want to merge? IOW, does it implement enough functionality that you can do whatever it is you wanted, with 64-bit Wine? * Does it cause any regression test failures on x86-linux or x86_64-linux? * Does it work on amd64-darwin? * Testcase? Please add a test case, in the style of none/tests/amd64/*.c * rebase -- please rebase to current svn trunk (or at least check it still applies)
Created attachment 59340 [details] patchset I've attached latest version of the patchset (with a small improvement over the version posted on vg mailing list) order to apply patchset: - all VEX-{01-04}* patches in VEX - patches {01-04}* in vg main dir for basic tools upgrade to registers - patch 05* for a test case of segmented address usage from your questions: * is this the final version of the patch you want to merge? IOW, does it implement enough functionality that you can do whatever it is you wanted, with 64-bit Wine? => with the patchset I no longer see missing IR as I did, and wine runs (I'm able to go through a good deal of the Wine regression tests). However, there are still some errors (some that already exist in 32bit vg+wine, some that are new). I haven't investigated all the details yet. * Does it cause any regression test failures on x86-linux or x86_64-linux? => no, same tests results before and after patchset (except for the new test that now succeed) * Does it work on amd64-darwin? Didn't test. * Testcase? Please add a test case, in the style of none/tests/amd64/*.c I've added one about segmented addressing. None yet about segment push/pop. * rebase -- please rebase to current svn trunk (or at least check it still applies) Done.
Created attachment 59716 [details] new version of the patchset latest version of the patchset. main changes are: - new test added (push/pop seg, mov to/from reg insns) - last bugs fixed - added also support for prlimit64 syscall on x86 (also needed when running Wine)
Patch B03 fails to apply on top of current trunk (r11958). Please rebase.
since the valgrind folks don't show any interest in this patchset, I'll let rot it here
(In reply to comment #5) > since the valgrind folks don't show any interest in this patchset, I'll let rot > it here Maybe it would help if the Wine people would take the time to answer polite questions posted by Valgrind developers, such as this one: http://www.winehq.org/pipermail/wine-patches/2011-May/101688.html.
Also, there hasn't actually been a release since the latest version of the patch was posted, so it will quite likely get merged during the next release.
(In reply to comment #6) > (In reply to comment #5) > > since the valgrind folks don't show any interest in this patchset, I'll let rot > > it here > > Maybe it would help if the Wine people would take the time to answer polite > questions posted by Valgrind developers, such as this one: > http://www.winehq.org/pipermail/wine-patches/2011-May/101688.html. wine-patches mailing list is only for patches. For discussions, you should use wine-devel (or IRC #winehackers on Freenode)
what is the status of this? What does the patchset apply to? It definitely does not apply to 3.8.1
I started to look at the patch, which at first sight looks pretty good to me (e.g. has regression tests). So, I first installed wine on my amd64/Ubuntu 14.04 system (wine-1.6.2) and then did a trial to start write.exe using: wine <windows7installmounted>/Windows/System32/write.exe So, something starts. But Help->About tells me this is wine wordpad, while I was expecting a 'genuine microsoft wordpad', as that path is to a windows7 'real' stuff. Note that on linux, file <windows7installmounted>/Windows/System32/write.exe tells me this is a PE32+ executable GUI x86-64. So, not completely sure this is all a 64 bit exe. It looks at least that some bit and pieces are 64 bits, as I see e.g. ==20677== Warning: client switching stacks? SP change: 0x7fffff7df118 --> 0x7ffffe50e5f8 After that, I use the last version of Valgrind (svn 15346) to run the above: valgrind --trace-children=yes wine <windows7installmounted>/Windows/System32/write.exe and that seems to work ok. So, I guess I am not doing the good test in order to show a crash or whatever that needs this patch. Can you give me some idiot-proof instructions about how to produce a bug, preferrably with the setup I have (i.e. wine 1.6.2 on Ubuntu) ? Thanks
(In reply to Philippe Waroquiers from comment #10) > Can you give me some idiot-proof instructions about how to produce a bug, > preferrably with the setup I have (i.e. wine 1.6.2 on Ubuntu) ? Would you mind a much simpler test-case? valgrind --trace-children=yes dosemu It uses segment registers even before starting to execute DOS, so if that even starts - you are doing good.
(In reply to Stas Sergeev from comment #11) > (In reply to Philippe Waroquiers from comment #10) > > Can you give me some idiot-proof instructions about how to produce a bug, > > preferrably with the setup I have (i.e. wine 1.6.2 on Ubuntu) ? > Would you mind a much simpler test-case? > valgrind --trace-children=yes dosemu > It uses segment registers even before starting to > execute DOS, so if that even starts - you are doing good. dosemu has effectively a problem. It would be nice in any case to have a similar test case with wine. The attached patch is very old, does not apply anymore, contains calls to functions that have disappeared. Is there an up to date version somewhere ?
(In reply to Philippe Waroquiers from comment #12) > The attached patch is very old, does not apply anymore, > contains calls to functions that have disappeared. > Is there an up to date version somewhere ? As per Comment #5, there is likely not. Looking into wine log, I also think Eric Pouech is not active with wine any more. IMHO it would be nice if valgrind devs to pick it up, after all its their fault letting it to rot here. :) > dosemu has effectively a problem. Not sure what you mean. It simply reads the segment registers upon init, because in signal handler it needs to restore them by hands, as linux does not do that. That's why dosemu is a simplest test-case: you don't need to setup it anyhow: if it just passes a start-up and creates an X window - you are already good. Hope someone else can help with wine here.
(In reply to Stas Sergeev from comment #13) > (In reply to Philippe Waroquiers from comment #12) > > The attached patch is very old, does not apply anymore, > > contains calls to functions that have disappeared. > > Is there an up to date version somewhere ? > As per Comment #5, there is likely not. > Looking into wine log, I also think Eric Pouech > is not active with wine any more. > IMHO it would be nice if valgrind devs to pick it up, > after all its their fault letting it to rot here. :) My first commit is in December 2011, So I was not born when comment 5 was done, and will thus consider myself not guilty :) I started rebasing the patch, but I am not familiar with some of the modified area, so not a straightforward action for me. > > dosemu has effectively a problem. > Not sure what you mean. I mean: when running dosemu, it works. when running dosemu under Valgrind, it does not work.
Any news? Is this doomed to never materialize? :) Is the only problem remained is to setup the wine test-case for this?
Created attachment 97737 [details] a test case Sorry for delay. Here's the segregs test case.
Created attachment 114890 [details] patches for wine64 and pdbs The attached patches get wine64 with pdbs working. I'm sure additional patches will be needed, but it's a start. The attached leakage.c produces the following output on 64-bit Wine with these changes: ==689== 12,345 bytes in 1 blocks are definitely lost in loss record 70 of 71 ==689== at 0x7BC5ACCD: initialize_block (heap.c:238) ==689== by 0x7BC5ACCD: RtlAllocateHeap (???:0) ==689== by 0x140006C0A: a (leakage.c:9) ==689== by 0x140006C28: b (leakage.c:14) ==689== by 0x140006C48: c (leakage.c:19) ==689== by 0x140006D28: main (leakage.c:43) ==689== ==689== 23,456 bytes in 1 blocks are possibly lost in loss record 71 of 71 ==689== at 0x7BC5ACCD: initialize_block (heap.c:238) ==689== by 0x7BC5ACCD: RtlAllocateHeap (???:0) ==689== by 0x140006CBC: setframe (leakage.c:28) ==689== by 0x140006CE8: d (leakage.c:33) ==689== by 0x140006D08: e (leakage.c:38) ==689== by 0x140006D3C: main (leakage.c:44) leakage.c built 32-bit with pdb fails with and without my changes. But with the attached, both 32-bit and 64-bit Wine tests still run under valgrind. Tested with the following: - Visual Studio 2017 (15.8.3) (Optimizing Compiler Version 19.15.26726 for x64) - Wine 3.15 - valgrind-3.14.0.GIT (097b2076013bd6082be189ab55dbdb2eb1e572b7)
Created attachment 114891 [details] simple test case for wine64 pdb
(In reply to Daniel Lehman from comment #18) > Created attachment 114891 [details] > simple test case for wine64 pdb Hi Daniel, Would you mind attaching a pre-built binary? I'd like to test this, but I don't currently have Visual Studio set up (and won't be able to for a week or so at the earliest). Thanks! -Austin
Created attachment 118764 [details] leakage.exe and pdb attached tested with: - Visual Studio 2017 (15.9.8) - wine 4.3 - valgrind-3.15.0.GIT (4816357b5c7ee5284cdf72800a81d2dd1845388f)
(In reply to Daniel Lehman from comment #20) > Created attachment 118764 [details] > leakage.exe and pdb > > attached tested with: > - Visual Studio 2017 (15.9.8) > - wine 4.3 > - valgrind-3.15.0.GIT (4816357b5c7ee5284cdf72800a81d2dd1845388f) Thanks. I wasn't able to reproduce, with same valgrind git commit (+ your patches) and wine-4.0-rc1 (last wow64 build I have handy, but given that you used 3.15 before I don't think it matters). What VALGRIND_OPTS are you using? I tried with: austin@laptop:~$ export VALGRIND_OPTS="-q --trace-children=yes --track-origins=yes --leak-check=full --num-callers=20 --vex-iropt-register-updates=allregs-at-mem-access" and got: austin@laptop:~$ ~/src/valgrind/vg-in-place /opt/oldwow64/wine-4.0-rc1/bin/wine leakage.exe ; echo $? preloader: Warning: failed to reserve range 00110000-68000000 preloader: Warning: failed to reserve range 7f000000-82000000 preloader: Warning: failed to reserve range 0000000000110000-0000000068000000 ==6365== Conditional jump or move depends on uninitialised value(s) ==6365== at 0x14005F7DE: ??? ==6365== by 0x51B686B7C272: ??? ==6365== Uninitialised value was created by a stack allocation ==6365== at 0x14004BB17: ??? ==6365== 00007FFFFE01B5F0 setframe: 00007FFFFE20FCD0 00007FFFFE01F7A0 ==6365== 12,345 bytes in 1 blocks are definitely lost in loss record 86 of 88 ==6365== at 0x7BC5B385: initialize_block (heap.c:238) ==6365== by 0x7BC5B385: RtlAllocateHeap (???:0) ==6365== by 0x140006BDA: ??? ==6365== ==6365== 23,456 bytes in 1 blocks are definitely lost in loss record 88 of 88 ==6365== at 0x7BC5B385: initialize_block (heap.c:238) ==6365== by 0x7BC5B385: RtlAllocateHeap (???:0) ==6365== by 0x140006C8C: ??? ==6365== by 0x51B686B7CD62: ??? ==6365== 0 did I miss a step?
That said, it's still an improvement. Without the patchset, I get: austin@laptop:~$ /opt/valgrind/bin/valgrind /opt/oldwow64/wine-4.0-rc1/bin/wine leakage.exe ; echo $? preloader: Warning: failed to reserve range 00110000-68000000 preloader: Warning: failed to reserve range 7f000000-82000000 preloader: Warning: failed to reserve range 0000000000110000-0000000068000000 0030:err:seh:segv_handler Got unexpected trap 0 ==6737== Invalid write of size 8 ==6737== at 0x7BC95B68: ??? (in /opt/oldwow64/wine-4.0-rc1/lib64/wine/ntdll.dll.so) ==6737== by 0x7BC95B62: ??? (in /opt/oldwow64/wine-4.0-rc1/lib64/wine/ntdll.dll.so) ==6737== by 0x7BC95C3A: ??? (in /opt/oldwow64/wine-4.0-rc1/lib64/wine/ntdll.dll.so) ==6737== Address 0x7ffffe20f4b8 is in a rw- anonymous segment ==6737== 0030:err:seh:NtRaiseException Unhandled exception code c000001d flags 0 addr 0x7bc95b63 29
> 0030:err:seh:NtRaiseException Unhandled exception code c000001d flags 0 addr 0x7bc95b63 that's probably the iretq instruction from set_cpu_context > What VALGRIND_OPTS are you using? -v --trace-children=yes --error-limit=no --log-file=output.txt --leak-check=full --leak-resolution=high --show-leak-kinds=all > did I miss a step? no, sorry, i forgot that i had to create a symlink to find the pdb: ln -s $HOME $WINEPREFIX/drive_Y because the baked-in path is Y:/stuff/leakage
(In reply to Daniel Lehman from comment #23) > > 0030:err:seh:NtRaiseException Unhandled exception code c000001d flags 0 addr 0x7bc95b63 > > that's probably the iretq instruction from set_cpu_context > > > What VALGRIND_OPTS are you using? > > -v --trace-children=yes --error-limit=no --log-file=output.txt > --leak-check=full --leak-resolution=high --show-leak-kinds=all > > > did I miss a step? > > no, sorry, i forgot that i had to create a symlink to find the pdb: > ln -s $HOME $WINEPREFIX/drive_Y > because the baked-in path is Y:/stuff/leakage That's weird..when first trying this, I missed your 'drive_Y' bit and was symlinking to dosdevices/y:, which failed. With drive_Y, it works (not valgrind's problem, of course). I can verify that works for me: + /home/austin/src/valgrind/vg-in-place wine64 leakage.exe preloader: Warning: failed to reserve range 0000000000110000-0000000068000000 ==32694== Conditional jump or move depends on uninitialised value(s) ==32694== at 0x14005F7DE: __strncnt (strncnt.cpp:21) ==32694== by 0x7B9D4E0A8F2C: ??? ==32694== Uninitialised value was created by a stack allocation ==32694== at 0x14004BB17: setSBUpLow (in /tmp/tmp.sYSJGwj3qz/stuff/leakage/leakage.exe) ==32694== 00007FFFFE018520 setframe: 00007FFFFE20FCD0 00007FFFFE01C6D0 006e:fixme:kernelbase:AppPolicyGetProcessTerminationMethod 0xfffffffffffffffa, 0x7ffffe20fd10 ==32694== 12,345 bytes in 1 blocks are definitely lost in loss record 92 of 93 ==32694== at 0x7BC5BB35: initialize_block (heap.c:238) ==32694== by 0x7BC5BB35: RtlAllocateHeap (???:0) ==32694== by 0x140006BDA: a (leakage.c:9) ==32694== by 0x140006BF8: b (leakage.c:14) ==32694== by 0x140006C18: c (leakage.c:19) ==32694== by 0x140006CF8: main (leakage.c:43) ==32694== ==32694== 23,456 bytes in 1 blocks are definitely lost in loss record 93 of 93 ==32694== at 0x7BC5BB35: initialize_block (heap.c:238) ==32694== by 0x7BC5BB35: RtlAllocateHeap (???:0) ==32694== by 0x140006C8C: setframe (leakage.c:28) ==32694== by 0x140006CB8: d (leakage.c:33) ==32694== by 0x140006CD8: e (leakage.c:38) ==32694== by 0x140006D0C: main (leakage.c:44) ==32694== ================================================== For our valgrind friends, if you'd like to test Daniel's patch, here's a short script to do so. Get wine64 from your package manager (4.0 stable should work, or probably any version), then run the script, no special wine knowledge needed :) #!/bin/bash # Requires wine64 (nothing special needed, get it from your package manager) # and valgrind, of course export VALGRIND="${VALGRIND:-valgrind}" export VALGRIND_OPTS="-q --trace-children=yes --track-origins=yes --leak-check=full --num-callers=20 --vex-iropt-register-updates=allregs-at-mem-access" export WINEPREFIX="${WINEPREFIX:-$HOME/.wine}" tmpdir="$(mktemp -d)" cd "$tmpdir" mkdir -p stuff/leakage cd stuff/leakage wget -O stuff.tar.bz2 "https://bugs.kde.org/attachment.cgi?id=118764" tar xjf stuff.tar.bz2 ln -sf "$tmpdir" "$WINEPREFIX/drive_Y" # Here we go: "$VALGRIND" wine64 leakage.exe cd rm -rf "$tmpdir"
Created attachment 122256 [details] patches for wine64 and pdbs rebased to latest master with fix from: https://bugs.kde.org/show_bug.cgi?id=400538 and retested with: - wine 4.13 - vs2017 15.9.15
*** Bug 414659 has been marked as a duplicate of this bug. ***
(In reply to Daniel Lehman from comment #25) > Created attachment 122256 [details] > patches for wine64 and pdbs I will try and look at these patches in the next couple of weeks, to see if they can be integrated. If you have any more recent version of them, can you please attach them instead?
nothing new to add for now
Created attachment 125367 [details] Rollup patch, which is simply the 15 patches merged Here's a rollup patch, which is just the previous 15 patches merged and with a bit of reformatting. It applies to the trunk as at 24 Jan 2020. I haven't landed it because the change in coregrind/m_stacktrace.c potentially gives a large performance problem. If the call to VG_(use_CFI_info) fails, then we now try with VG_(use_MSVC_x64_info). That is guaranteed to fail in "normal" (non-Wine) use. But before it fails it could potentially visit the entire debugInfo list, which can contains hundreds of DebugInfos for large complex applications. The obvious fix is to keep a boolean indicating whether any Windows unwind info has been loaded, and if false, just skip this call. That would remove the perf hit for non-Wine use.
Created attachment 125416 [details] bool to skip msvc x64 code path in non-wine case > The obvious fix is to keep a boolean indicating whether any Windows unwind > info has been loaded, and if false, just skip this call. That would remove the > perf hit for non-Wine use. something like the attached? i based it on FPO_info_present. the patch is rebased against the rollup. it still works with the above test case on 64-bit wine but i verified it is not called with non-wine (using python -c "import numpy")