Bug 253657 - missing some amd64 to let wine/amd64 run on valgrind/amd64
Summary: missing some amd64 to let wine/amd64 run on valgrind/amd64
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.5.0
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks: 253451
  Show dependency treegraph
 
Reported: 2010-10-09 14:05 UTC by Eric Pouech
Modified: 2020-01-25 22:24 UTC (History)
13 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patchset for preliminary registers support in AMD64 (5.83 KB, patch)
2010-10-09 14:05 UTC, Eric Pouech
Details
patchset (17.04 KB, patch)
2011-04-26 21:08 UTC, Eric Pouech
Details
new version of the patchset (16.05 KB, application/x-gzip)
2011-05-07 09:05 UTC, Eric Pouech
Details
a test case (499 bytes, text/plain)
2016-03-07 11:05 UTC, Stas Sergeev
Details
patches for wine64 and pdbs (13.04 KB, application/gzip)
2018-09-11 05:20 UTC, Daniel Lehman
Details
simple test case for wine64 pdb (585 bytes, text/x-csrc)
2018-09-11 05:22 UTC, Daniel Lehman
Details
leakage.exe and pdb (1.12 MB, application/x-bzip)
2019-03-13 04:54 UTC, Daniel Lehman
Details
patches for wine64 and pdbs (12.27 KB, application/gzip)
2019-08-20 04:00 UTC, Daniel Lehman
Details
Rollup patch, which is simply the 15 patches merged (38.05 KB, patch)
2020-01-24 14:08 UTC, Julian Seward
Details
bool to skip msvc x64 code path in non-wine case (3.46 KB, text/plain)
2020-01-25 22:24 UTC, Daniel Lehman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Pouech 2010-10-09 14:05:59 UTC
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
Comment 1 Julian Seward 2011-04-26 11:12:16 UTC
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)
Comment 2 Eric Pouech 2011-04-26 21:08:50 UTC
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.
Comment 3 Eric Pouech 2011-05-07 09:05:16 UTC
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)
Comment 4 Frédéric Delanoy 2011-08-09 13:30:48 UTC
Patch B03 fails to apply on top of current trunk (r11958). Please rebase.
Comment 5 Eric Pouech 2011-08-28 07:26:22 UTC
since the valgrind folks don't show any interest in this patchset, I'll let rot it here
Comment 6 Bart Van Assche 2011-08-28 07:52:36 UTC
(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.
Comment 7 Tom Hughes 2011-08-28 08:28:25 UTC
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.
Comment 8 Frédéric Delanoy 2011-08-28 08:36:16 UTC
(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)
Comment 9 Michal 2012-11-19 09:51:33 UTC
what is the status of this?

What does the patchset apply to?

It definitely does not apply to 3.8.1
Comment 10 Philippe Waroquiers 2015-06-20 17:14:33 UTC
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
Comment 11 Stas Sergeev 2015-06-20 17:22:34 UTC
(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.
Comment 12 Philippe Waroquiers 2015-06-21 15:22:31 UTC
(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 ?
Comment 13 Stas Sergeev 2015-06-21 16:03:15 UTC
(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.
Comment 14 Philippe Waroquiers 2015-06-21 16:35:48 UTC
(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.
Comment 15 Stas Sergeev 2015-07-11 18:10:07 UTC
Any news?
Is this doomed to never materialize? :)
Is the only problem remained is to setup the
wine test-case for this?
Comment 16 Stas Sergeev 2016-03-07 11:05:34 UTC
Created attachment 97737 [details]
a test case

Sorry for delay.
Here's the segregs test case.
Comment 17 Daniel Lehman 2018-09-11 05:20:59 UTC
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)
Comment 18 Daniel Lehman 2018-09-11 05:22:32 UTC
Created attachment 114891 [details]
simple test case for wine64 pdb
Comment 19 Austin English 2019-03-12 14:47:42 UTC
(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
Comment 20 Daniel Lehman 2019-03-13 04:54:03 UTC
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)
Comment 21 Austin English 2019-03-13 15:43:50 UTC
(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?
Comment 22 Austin English 2019-03-13 15:48:32 UTC
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
Comment 23 Daniel Lehman 2019-03-14 02:09:35 UTC
> 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
Comment 24 Austin English 2019-03-14 15:31:01 UTC
(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"
Comment 25 Daniel Lehman 2019-08-20 04:00:14 UTC
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
Comment 26 Tom Hughes 2019-11-29 17:57:42 UTC
*** Bug 414659 has been marked as a duplicate of this bug. ***
Comment 27 Julian Seward 2019-12-28 19:30:17 UTC
(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?
Comment 28 Daniel Lehman 2020-01-01 19:49:51 UTC
nothing new to add for now
Comment 29 Julian Seward 2020-01-24 14:08:02 UTC
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.
Comment 30 Daniel Lehman 2020-01-25 22:24:40 UTC
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")