Bug 489088 - Valgrind throws unhandled instruction bytes: 0xC5 0x79 0xD6 0xE0 0xC5 0xFA 0x7E 0xDB 0xC5 0xF8
Summary: Valgrind throws unhandled instruction bytes: 0xC5 0x79 0xD6 0xE0 0xC5 0xFA 0x...
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (show other bugs)
Version: 3.24 GIT
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Mark Wielaard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-24 05:14 UTC by Sam James
Modified: 2024-06-30 18:49 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
test.c (29.62 KB, text/plain)
2024-06-24 05:14 UTC, Sam James
Details
foo.xz (26.56 KB, application/x-xz)
2024-06-24 05:16 UTC, Sam James
Details
Implement VMOVQ xmm1, xmm2/m64 (14.28 KB, text/plain)
2024-06-30 17:00 UTC, Mark Wielaard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James 2024-06-24 05:14:48 UTC
Created attachment 170894 [details]
test.c

I hit the following when reducing a possible GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115533).

At points during my reduction of a test from flac, I got the following:
```
 $ gcc-15 test.c -o foo -O3 -ggdb3 -march=native -fipa-pta -fno-vect-cost-model && valgrind ./foo
==2627495== Memcheck, a memory error detector
==2627495== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==2627495== Using Valgrind-3.24.0.GIT and LibVEX; rerun with -h for copyright info
==2627495== Command: ./foo
==2627495==
vex amd64->IR: unhandled instruction bytes: 0xC5 0x79 0xD6 0xE0 0xC5 0xFA 0x7E 0xDB 0xC5 0xF8
vex amd64->IR:   REX=0 REX.W=0 REX.R=1 REX.X=0 REX.B=0
vex amd64->IR:   VEX=1 VEX.L=0 VEX.nVVVV=0x0 ESC=0F
vex amd64->IR:   PFX.66=1 PFX.F2=0 PFX.F3=0
==2627495== valgrind: Unrecognised instruction at address 0x109e00.
==2627495==    at 0x109E00: filter (test.c:64)
==2627495==    by 0x109E00: AnalyzeSamples (test.c:100)
==2627495==    by 0x109366: main (test.c:167)
==2627495== Your program just tried to execute an instruction that Valgrind
==2627495== did not recognise.  There are two possible reasons for this.
==2627495== 1. Your program has a bug and erroneously jumped to a non-code
==2627495==    location.  If you are running Memcheck and you just saw a
==2627495==    warning about a bad jump, it's probably your program's fault.
==2627495== 2. The instruction is legitimate but Valgrind doesn't handle it,
==2627495==    i.e. it's Valgrind's fault.  If you think this is the case or
==2627495==    you are not sure, please let us know and we'll try to fix it.
==2627495== Either way, Valgrind will now raise a SIGILL signal which will
==2627495== probably kill your program.
==2627495==
==2627495== Process terminating with default action of signal 4 (SIGILL): dumping core
==2627495==  Illegal opcode at address 0x109E00
==2627495==    at 0x109E00: filter (test.c:64)
==2627495==    by 0x109E00: AnalyzeSamples (test.c:100)
==2627495==    by 0x109366: main (test.c:167)
```

It's possible that the program was just jumping to genuine junk, but I thought I'd report it on the off-chance it isn't.
Comment 1 Sam James 2024-06-24 05:16:12 UTC
Created attachment 170895 [details]
foo.xz

foo.xz is `test` from above (compiled binary from `test.c` attached earlier, using \gcc-15 test.c -o foo -O3 -ggdb3 -march=native -fipa-pta -fno-vect-cost-model`).

On this system, -march=native is -march=znver2 (pretty much).

```
$ gcc-15 --version
gcc-15 (Gentoo Hardened 15.0.9999 p, commit e275a9e2e7e85e8c0071f7cffbd7948bc891573d) 15.0.0 20240621 (experimental) 52a82359073653e312aaa5703f7e0ce339588961
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```
Comment 2 Sam James 2024-06-24 05:16:52 UTC
Note that this is also unreleased gcc and using binutils from recent trunk too.

```
$ ld --version
GNU ld (Gentoo 9999 p1) 2.42.50.20240615
Copyright (C) 2024 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.
```

Again, I don't mind if the bug is junk / not a Valgrind problem :)
Comment 3 Mark Wielaard 2024-06-24 09:36:44 UTC
This looks like vmovq:

   0x0000000000109df7 <+1111>:	vshufps $0x55,%xmm1,%xmm1,%xmm4
   0x0000000000109dfc <+1116>:	vmovshdup %xmm0,%xmm1
=> 0x0000000000109e00 <+1120>:	vmovq  %xmm12,%xmm0
   0x0000000000109e04 <+1124>:	vmovq  %xmm3,%xmm3
   0x0000000000109e08 <+1128>:	vmovaps %xmm2,-0x50(%rbp)
Comment 4 Mark Wielaard 2024-06-24 11:38:45 UTC
See also https://bugs.kde.org/show_bug.cgi?id=391148 which comes with a patch.
Comment 5 Sam James 2024-06-24 21:57:47 UTC
(In reply to Mark Wielaard from comment #4)
> See also https://bugs.kde.org/show_bug.cgi?id=391148 which comes with a
> patch.

Hi Mark, thanks for looking!

That patch seems to work:

```
==144687== Invalid read of size 8
==144687==    at 0x109F43: filter (test.c:59)
==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
==144687==    by 0x109366: main (test.c:167)
==144687==  Address 0x4637bfeabfe4bbd4 is not stack'd, malloc'd or (recently) free'd
==144687==
==144687==
==144687== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==144687==  General Protection Fault
==144687==    at 0x109F43: filter (test.c:59)
==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
==144687==    by 0x109366: main (test.c:167)
```
although the address looks a bit suspicious and I wasn't aware of out of bounds access in the testcase, so not sure if it really is decoding fully correctly?
Comment 6 Mark Wielaard 2024-06-25 20:42:52 UTC
(In reply to Sam James from comment #5)
> (In reply to Mark Wielaard from comment #4)
> > See also https://bugs.kde.org/show_bug.cgi?id=391148 which comes with a
> > patch.
> 
> Hi Mark, thanks for looking!
> 
> That patch seems to work:
> 
> ```
> ==144687== Invalid read of size 8
> ==144687==    at 0x109F43: filter (test.c:59)
> ==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
> ==144687==    by 0x109366: main (test.c:167)
> ==144687==  Address 0x4637bfeabfe4bbd4 is not stack'd, malloc'd or
> (recently) free'd
> ==144687==
> ==144687==
> ==144687== Process terminating with default action of signal 11 (SIGSEGV):
> dumping core
> ==144687==  General Protection Fault
> ==144687==    at 0x109F43: filter (test.c:59)
> ==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
> ==144687==    by 0x109366: main (test.c:167)
> ```
> although the address looks a bit suspicious and I wasn't aware of out of
> bounds access in the testcase, so not sure if it really is decoding fully
> correctly?

hmmm, yeah that is kind of odd. That address is so weird that it really must be invalid. Does the original program also try to read from that address and produce a SIGSEGV when not run under valgrind?

Maybe as a quick hack check you could change that DIP("vmovq %s,%s\n", nameXMMReg(rG), nameIReg64(rE)); in the patch to vex_printf(...)

If it decodes correctly it should print vmovq  %xmm12,%xmm0

(You can also get some of that with something like --trace-flags=10000000 --trace-notbelow=1000 but that is really verbose)
Comment 7 Sam James 2024-06-26 07:58:23 UTC
(In reply to Mark Wielaard from comment #6)
> (In reply to Sam James from comment #5)
> > (In reply to Mark Wielaard from comment #4)
> > > See also https://bugs.kde.org/show_bug.cgi?id=391148 which comes with a
> > > patch.
> > 
> > Hi Mark, thanks for looking!
> > 
> > That patch seems to work:
> > 
> > ```
> > ==144687== Invalid read of size 8
> > ==144687==    at 0x109F43: filter (test.c:59)
> > ==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
> > ==144687==    by 0x109366: main (test.c:167)
> > ==144687==  Address 0x4637bfeabfe4bbd4 is not stack'd, malloc'd or
> > (recently) free'd
> > ==144687==
> > ==144687==
> > ==144687== Process terminating with default action of signal 11 (SIGSEGV):
> > dumping core
> > ==144687==  General Protection Fault
> > ==144687==    at 0x109F43: filter (test.c:59)
> > ==144687==    by 0x109F43: AnalyzeSamples (test.c:100)
> > ==144687==    by 0x109366: main (test.c:167)
> > ```
> > although the address looks a bit suspicious and I wasn't aware of out of
> > bounds access in the testcase, so not sure if it really is decoding fully
> > correctly?
> 
> hmmm, yeah that is kind of odd. That address is so weird that it really must
> be invalid. Does the original program also try to read from that address and
> produce a SIGSEGV when not run under valgrind?

Nope!

> 
> Maybe as a quick hack check you could change that DIP("vmovq %s,%s\n",
> nameXMMReg(rG), nameIReg64(rE)); in the patch to vex_printf(...)
> 
> If it decodes correctly it should print vmovq  %xmm12,%xmm0
> 

==402260==
vmovq %xmm12,%r8
vmovq %xmm14,%r10
==402260== Invalid read of size 8
==402260==    at 0x109F46: filter (test.c:59)
==402260==    by 0x109F46: AnalyzeSamples (test.c:100)
==402260==    by 0x109366: main (test.c:167)
==402260==  Address 0x4637bfeabfe4bbd4 is not stack'd, malloc'd or (recently) free'd
Comment 8 Mark Wielaard 2024-06-27 22:19:01 UTC
So I think the implementation should be something like:

diff --git a/VEX/priv/guest_amd64_toIR.c b/VEX/priv/guest_amd64_toIR.c
index 28c37f092211..a7ea5951aac7 100644
--- a/VEX/priv/guest_amd64_toIR.c
+++ b/VEX/priv/guest_amd64_toIR.c
@@ -27026,6 +27026,14 @@ Long dis_ESC_0F__VEX (
          if (epartIsReg(modrm)) {
             /* fall through, awaiting test case */
             /* dst: lo half copied, hi half zeroed */
+            UInt rE = eregOfRexRM(pfx,modrm);
+            putXMMRegLane64( rE, 0, getXMMRegLane64( rG, 0 ));
+            /* zero bits 255:64 */
+            putXMMRegLane64( rG, 1, mkU64(0) );
+            putYMMRegLane128( rG, 1, mkV128(0) );
+            DIP("vmovq %s,%s\n", nameXMMReg(rG), nameXMMReg(rE));
+            delta += 1;
+            goto decode_success;
          } else {
             addr = disAMode ( &alen, vbi, pfx, delta, dis_buf, 0 );
             storeLE( mkexpr(addr), getXMMRegLane64( rG, 0 ));

But obviously we need some real test cases.
Comment 9 Sam James 2024-06-28 17:39:08 UTC
> But obviously we need some real test cases.

I was confused by what you meant, then I checked the See Also'd bugs and saw there's no testcases there except for an artificial one :|
Comment 10 Mark Wielaard 2024-06-30 17:00:07 UTC
Created attachment 171207 [details]
Implement VMOVQ xmm1, xmm2/m64

Added testcase, which showed we cleared the wrong register (oops). Fixed that in the implementation.
Comment 11 Sam James 2024-06-30 17:40:49 UTC
Thank you very much - that works great!
Comment 12 Mark Wielaard 2024-06-30 18:23:13 UTC
commit 10a22445d747817932692b1c1ee3faa726121cb4
Author: Mark Wielaard <mark@klomp.org>
Date:   Sun Jun 30 20:17:32 2024 +0200

    Implement VMOVQ xmm1, xmm2/m64
    
    We implemented the memory variant already, but not the reg variant.
    Add a separate avx-vmovq testcase, because avx-1 is already really big.
    
    https://bugs.kde.org/show_bug.cgi?id=391148
    https://bugs.kde.org/show_bug.cgi?id=417572
    https://bugs.kde.org/show_bug.cgi?id=489088