Created attachment 124817 [details] 16-bit bswap test case Steps to reproduce (on Arch Linux): 1. If ~/.wine exists, delete it. 2. Clone Wine with `git clone git://source.winehq.org/git/wine.git` 3. Compile a 32-bit-only version of Wine by running `./configure && make -j8` 4. Download the Leawo Blu-ray Player installer from https://www.leawo.com/blu-ray-player/ 5. Run `./wine blurayplayer_setup.exe` and click through the setup wizard. On the last screen, uncheck "Launch Leawo Blu-Ray Player". 6. Run `valgrind --trace-children=yes ./wine 'C:\Program Files\Leawo\Blu-ray Player\Leawo Blu-ray Player.exe'` The program exits almost immediately with the following error: vex x86->IR: unhandled instruction bytes: 0x66 0xF 0xCE 0x4F ==84194== valgrind: Unrecognised instruction at address 0xd272c9a. ==84194== at 0xD272C9A: ??? ==84194== Your program just tried to execute an instruction that Valgrind ==84194== did not recognise. There are two possible reasons for this. ==84194== 1. Your program has a bug and erroneously jumped to a non-code ==84194== location. If you are running Memcheck and you just saw a ==84194== warning about a bad jump, it's probably your program's fault. ==84194== 2. The instruction is legitimate but Valgrind doesn't handle it, ==84194== i.e. it's Valgrind's fault. If you think this is the case or ==84194== you are not sure, please let us know and we'll try to fix it. ==84194== Either way, Valgrind will now raise a SIGILL signal which will ==84194== probably kill your program. ==84194== valgrind: Unrecognised instruction at address 0xd272c9a. ==84194== at 0xD272C9A: ??? ==84194== Your program just tried to execute an instruction that Valgrind ==84194== did not recognise. There are two possible reasons for this. ==84194== 1. Your program has a bug and erroneously jumped to a non-code ==84194== location. If you are running Memcheck and you just saw a ==84194== warning about a bad jump, it's probably your program's fault. ==84194== 2. The instruction is legitimate but Valgrind doesn't handle it, ==84194== i.e. it's Valgrind's fault. If you think this is the case or ==84194== you are not sure, please let us know and we'll try to fix it. ==84194== Either way, Valgrind will now raise a SIGILL signal which will ==84194== probably kill your program. 0009:err:seh:segv_handler Got unexpected trap 0 0009:err:module:LdrInitializeThunk "panda.dll" failed to initialize, aborting 0009:err:module:LdrInitializeThunk Initializing dlls for L"C:\\Program Files\\Leawo\\Blu-ray Player\\Leawo Blu-ray Player.exe" failed, status c000001d The unrecognized instruction comes from closed-source Leawo code. 0F CE is BSWAP and 66 is the 16-bit instruction prefix. According to the Intel and AMD documentation, the result of a 16-bit BSWAP is undefined.[1][2] However, on all Intel and AMD CPUs (since the 486 when BSWAP was added), a 16-bit BSWAP returns the value 0.[3] The attached test program from Doug Johnson confirms this. Because there is software in the wild that depends on this behavior, I think Valgrind should return 0 as well. [1] https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf#page=214 [2] https://www.amd.com/system/files/TechDocs/24594.pdf#page=153 [3] https://gynvael.coldwind.pl/?id=268 $ sha256sum blurayplayer_setup.exe 8b7eef385ba336b0a44fc6295c0f6e3d869555d9eb8c262f470cad2028ba7d4d
Created attachment 124818 [details] Proposed patch The attached patch fixes the unhandled instruction problem. Unfortunately, Leawo then pops up the following error dialog: A debugger has been found running in your system. Please, unload it from memory and restart your program. There's probably a way past this, but even if there isn't, Valgrind produced several warnings about the Wine code that is executed up to that point. So if this patch is accepted, even though it doesn't get Leawo working all the way on Valgrind, it still would help me and other Wine developers to start to debug it.
(In reply to Alex Henrie from comment #1) > There's probably a way past this, but even if there isn't, Valgrind produced > several warnings about the Wine code that is executed up to that point. Make sure you're working with the trunk code, so you have the latest false-positive-avoidance stuff in place. As a side comment: I'm not sure if the Wine developers are aware of this, but some years back I added to Valgrind, a transformation aimed at reducing the false positive level from code compiled by MSVC. It's not enabled by default but you could easily do so. In VEX/priv/ir_opt.c, find this if (0) bb = do_MSVC_HACKS(bb); and change it to "if (1)". The transformation that it does is described in a big comment beginning "MSVC specific transformation hacks" in that same source file.
Created attachment 124859 [details] 16-bit bswap test case I had added a check to Doug's test program to make sure that it was running in 32-bit mode, but it actually works in 64-bit mode too. Here's the same program without the check I hacked in.
Created attachment 124860 [details] Leawo output with patched Valgrind (In reply to Julian Seward from comment #2) > Make sure you're working with the trunk code, so you have the latest > false-positive-avoidance stuff in place. > > As a side comment: I'm not sure if the Wine developers are aware of this, > but some years back I added to Valgrind, a transformation aimed at reducing > the false positive level from code compiled by MSVC. It's not enabled by > default but you could easily do so. In VEX/priv/ir_opt.c, find this > > if (0) > bb = do_MSVC_HACKS(bb); > > and change it to "if (1)". The transformation that it does is described in > a big comment beginning "MSVC specific transformation hacks" in that same > source file. Thanks for the tips! I'm attaching the warnings I'm seeing from Leawo running on trunk Valgrind with the 16-bit BSWAP patch. I don't think the MSVC hacks would help here.
And you're aware of the as-yet unmerged PDB reading fixes at [1], yes? https://bugs.kde.org/show_bug.cgi?id=253657
(In reply to Julian Seward from comment #5) > And you're aware of the as-yet unmerged PDB reading fixes at [1], yes? > https://bugs.kde.org/show_bug.cgi?id=253657 Yes, I know about those patches. There are also patches somewhere to make Wine produce PDB files for its own DLLs, but they have not been accepted into Wine yet.
Fixed, 6e0573777c487e83d5fbd2fd764b041e59784766. Thanks for the patch and analysis.
Thank you!