Bug 415757

Summary: vex x86->IR: unhandled instruction bytes: 0x66 0xF 0xCE 0x4F
Product: [Developer tools] valgrind Reporter: Alex Henrie <alexhenrie24>
Component: vexAssignee: Julian Seward <jseward>
Status: RESOLVED FIXED    
Severity: minor CC: dougvj
Priority: NOR    
Version First Reported In: 3.15 SVN   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: 16-bit bswap test case
Proposed patch
16-bit bswap test case
Leawo output with patched Valgrind

Description Alex Henrie 2020-01-01 03:50:43 UTC 
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
Comment 1 Alex Henrie 2020-01-01 03:55:34 UTC 
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.
Comment 2 Julian Seward 2020-01-02 17:46:31 UTC 
(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.
Comment 3 Alex Henrie 2020-01-02 17:48:31 UTC 
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.
Comment 4 Alex Henrie 2020-01-02 18:00:40 UTC 
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.
Comment 5 Julian Seward 2020-01-02 18:23:21 UTC 
And you're aware of the as-yet unmerged PDB reading fixes at [1], yes?
https://bugs.kde.org/show_bug.cgi?id=253657
Comment 6 Alex Henrie 2020-01-02 19:09:09 UTC 
(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.
Comment 7 Julian Seward 2020-01-22 08:33:18 UTC 
Fixed, 6e0573777c487e83d5fbd2fd764b041e59784766.  Thanks for the 
patch and analysis.
Comment 8 Alex Henrie 2020-01-22 17:42:28 UTC 
Thank you!