Bug 256387

Vince, is this one that you already have a fix for?

It looks like 0xD4 0xA is the "aam" (ascii-adjust for multiplication) BCD instruction, which is 32-bit only.  My big list of missing instructions is currently 64-bit only, but I could probably be convinced to put one together for 32-bit as well.

This would help us solve a many year old problem for our customers.  I would
greatly appreciate it if you would start a 32-bit list!  :-) :-) :-)

Shawn


On Wed, Nov 10, 2010 at 8:28 AM, Vince Weaver <vince@csl.cornell.edu> wrote:

> https://bugs.kde.org/show_bug.cgi?id=256387
>
>
>
>
>
> --- Comment #2 from Vince Weaver <vince csl cornell edu>  2010-11-10
> 16:28:48 ---
> It looks like 0xD4 0xA is the "aam" (ascii-adjust for multiplication) BCD
> instruction, which is 32-bit only.  My big list of missing instructions is
> currently 64-bit only, but I could probably be convinced to put one
> together
> for 32-bit as well.
>
> --
> Configure bugmail: https://bugs.kde.org/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You reported the bug.
>

Created attachment 53327 [details]
implements aad and aam instructions.

*** Bug 167700 has been marked as a duplicate of this bug. ***

Shawn, does Vince's patch work for you?

I wasn't aware the patch was complete.  I tried applying the patch to the 3.6.0 source which I used to make the binary.  Here's what I get:

***

valgrind-3.6.0> patch -i aad_aam.patch 
patching file aad_aam.vgtest
patching file aad_aam.c
patching file Makefile.am
Hunk #1 FAILED at 21.
Hunk #2 FAILED at 62.
2 out of 2 hunks FAILED -- saving rejects to file Makefile.am.rej
can't find file to patch at input line 152
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: priv/guest_x86_defs.h
|===================================================================
|--- VEX/priv/guest_x86_defs.h	(revision 2069)
|+++ VEX/priv/guest_x86_defs.h	(working copy)
--------------------------
File to patch: 

***

I ctrl-c out at the "File to patch:" prompt.

Do I need to get a different source tree to apply the patch to?  Or am I using the wrong command to apply the patch?

Thanks,
Shawn

(In reply to comment #7)

It looks complete to me.  I'm a bit surprised it doesn't apply
to 3.6.0 though.  Try a trunk tree

  svn co svn://svn.valgrind.org/valgrind/trunk
  cd trunk
  # apply patch
  ./autogen.sh
  # configure and build as usual.

> valgrind-3.6.0> patch -i aad_aam.patch 

Hmm, time to make friends with "patch --dry-run" ?

(In reply to comment #7)

I can patch it if I enter the valgrind directory and do a:
  patch -p0 < aad_aam.patch

Julian & Vince,

Both of your methods work fine (guess I need to learn how to use patch :-)

But unless I am still doing something wrong it doesn't look like the patch worked.  Here's the output (started with a fresh 3.6.0 tree extracted from original tar ball):

valgrind-3.6.0> ls -l aad_aam.patch 
-rw-rw-rw- 1 shawnc shawnc 9434 Jan 10 16:53 aad_aam.patch
valgrind-3.6.0> sha1sum aad_aam.patch 
3c70df5f2837554afe475e25118fa183f4caf21a  aad_aam.patch
valgrind-3.6.0> head aad_aam.patch 
Index: none/tests/x86/aad_aam.stderr.exp
===================================================================
Index: none/tests/x86/aad_aam.stdout.exp
===================================================================
Index: none/tests/x86/aad_aam.vgtest
===================================================================
--- none/tests/x86/aad_aam.vgtest	(revision 0)
+++ none/tests/x86/aad_aam.vgtest	(revision 0)
@@ -0,0 +1,2 @@
+prog: aad_aam
valgrind-3.6.0> tail aad_aam.patch 
+     /* Set NDEP even though it isn't used.  This makes redundant-PUT
+        elimination of previous stores to this field work better. */
+       stmt( IRStmt_Put( OFFB_CC_NDEP, mkU32(0) ));
+
+       DIP(opc == 0xD4 ? "aam\n" : "aad\n");
+       break;
+
    /* ------------------------ CWD/CDQ -------------------- */
 
    case 0x98: /* CBW */
valgrind-3.6.0> patch -p0 < aad_aam.patch 
patching file none/tests/x86/aad_aam.vgtest
patching file none/tests/x86/aad_aam.c
patching file none/tests/x86/Makefile.am
patching file VEX/priv/guest_x86_defs.h
patching file VEX/priv/guest_x86_helpers.c
patching file VEX/priv/guest_x86_toIR.c
valgrind-3.6.0> ./configure 
...
         Maximum build arch: x86
         Primary build arch: x86
       Secondary build arch: 
                   Build OS: linux
       Primary build target: X86_LINUX
     Secondary build target: 
         Default supp files: exp-ptrcheck.supp xfree-3.supp xfree-4.supp glibc-2.X-drd.supp glibc-2.34567-NPTL-helgrind.supp glibc-2.5.supp 

valgrind-3.6.0> make
...
valgrind-3.6.0> echo $?
0
valgrind-3.6.0> coregrind/valgrind imsrun PROGRAM
==24910== Memcheck, a memory error detector
==24910== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==24910== Using Valgrind-3.7.0.SVN and LibVEX; rerun with -h for copyright info
==24910== Command: imsrun PROGRAM
==24910== 
IMS/Basic 7.5.124 Runtime (I) (15Dec09) (L64)
Copyright 1985-2001 by Information Management Systems Inc.
Copyright 2002-2006 by Source Licensee.
All rights reserved.


vex x86->IR: unhandled instruction bytes: 0xD4 0xA 0x2 0x7
==24910== valgrind: Unrecognised instruction at address 0x805548f.
==24910== Your program just tried to execute an instruction that Valgrind
==24910== did not recognise.  There are two possible reasons for this.
==24910== 1. Your program has a bug and erroneously jumped to a non-code
==24910==    location.  If you are running Memcheck and you just saw a
==24910==    warning about a bad jump, it's probably your program's fault.
==24910== 2. The instruction is legitimate but Valgrind doesn't handle it,
==24910==    i.e. it's Valgrind's fault.  If you think this is the case or
==24910==    you are not sure, please let us know and we'll try to fix it.
==24910== Either way, Valgrind will now raise a SIGILL signal which will
==24910== probably kill your program.
==24910== 
==24910== Process terminating with default action of signal 4 (SIGILL)
==24910==  Illegal opcode at address 0x805548F
==24910==    at 0x805548F: ??? (in /usr/bin/imsrun)
==24910==    by 0x8078A43: ??? (in /usr/bin/imsrun)
==24910==    by 0x804D70B: ??? (in /usr/bin/imsrun)
==24910==    by 0x805080F: ??? (in /usr/bin/imsrun)
==24910==    by 0x8072C81: ??? (in /usr/bin/imsrun)
==24910==    by 0x804B8D1: ??? (in /usr/bin/imsrun)
==24910==    by 0xAC6E8B: (below main) (in /lib/libc-2.5.so)
==24910== 
==24910== HEAP SUMMARY:
==24910==     in use at exit: 322,266 bytes in 14 blocks
==24910==   total heap usage: 28 allocs, 14 frees, 329,043 bytes allocated
==24910== 
==24910== LEAK SUMMARY:
==24910==    definitely lost: 0 bytes in 0 blocks
==24910==    indirectly lost: 0 bytes in 0 blocks
==24910==      possibly lost: 0 bytes in 0 blocks
==24910==    still reachable: 322,266 bytes in 14 blocks
==24910==         suppressed: 0 bytes in 0 blocks
==24910== Rerun with --leak-check=full to see details of leaked memory
==24910== 
==24910== For counts of detected and suppressed errors, rerun with: -v
==24910== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 16 from 8)
Illegal instruction
valgrind-3.6.0>

I forgot to mention that I got the same result from the trunk source.

Hmm.  What happens if you replace the "break;" at the end of the
patch ..

+       DIP(opc == 0xD4 ? "aam\n" : "aad\n");
+       break;

with instead  "goto decode_success;" ?

(In reply to comment #11)
> I forgot to mention that I got the same result from the trunk source.

If you're using the trunk source with the patch, can you do the following:

Apply patch
Run ./autogen.sh
Run ./configure
Run make
Run make check
Run ./vg-in-place on your test.
If that fails, can you run ./vg-in-place ./none/tests/x86/aad_aam
and see if the test case somehow works despite your test failing?

Sorry for the delay guys.  I only work Mon, Wed, Fri and I was slammed Wed.

Here's my results:
~> cd valgrind-trunk
valgrind-trunk> make clean
...
valgrind-trunk> make distclean
valgrind-trunk> svn status
X       VEX

Performing status on external item at 'VEX'
valgrind-trunk> svn update
U    none/tests/amd64/sse4-64.c

Fetching external item into 'VEX'
U    VEX/priv/guest_amd64_toIR.c
Updated external to revision 2075.

Updated to revision 11496.
valgrind-trunk> ./autogen.sh 
running: aclocal
running: autoheader
running: automake -a
running: autoconf
valgrind-trunk> ./configure
...
         Maximum build arch: x86
         Primary build arch: x86
       Secondary build arch: 
                   Build OS: linux
       Primary build target: X86_LINUX
     Secondary build target: 
         Default supp files: exp-ptrcheck.supp xfree-3.supp xfree-4.supp glibc-2.X-drd.supp glibc-2.34567-NPTL-helgrind.supp glibc-2.5.supp 

valgrind-trunk> make check
...
valgrind-trunk> echo $?
0
valgrind-trunk> ./vg-in-place imsrun PROGRAM
==20132== Memcheck, a memory error detector
==20132== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==20132== Using Valgrind-3.7.0.SVN and LibVEX; rerun with -h for copyright info
==20132== Command: imsrun PROGRAM
==20132== 
IMS/Basic 7.5.124 Runtime (I) (15Dec09) (L64)
Copyright 1985-2001 by Information Management Systems Inc.
Copyright 2002-2006 by Source Licensee.
All rights reserved.

vex x86->IR: unhandled instruction bytes: 0xD4 0xA 0x2 0x7
==20132== valgrind: Unrecognised instruction at address 0x805548f.
==20132== Your program just tried to execute an instruction that Valgrind
==20132== did not recognise.  There are two possible reasons for this.
==20132== 1. Your program has a bug and erroneously jumped to a non-code
==20132==    location.  If you are running Memcheck and you just saw a
==20132==    warning about a bad jump, it's probably your program's fault.
==20132== 2. The instruction is legitimate but Valgrind doesn't handle it,
==20132==    i.e. it's Valgrind's fault.  If you think this is the case or
==20132==    you are not sure, please let us know and we'll try to fix it.
==20132== Either way, Valgrind will now raise a SIGILL signal which will
==20132== probably kill your program.
==20132== 
==20132== Process terminating with default action of signal 4 (SIGILL)
==20132==  Illegal opcode at address 0x805548F
==20132==    at 0x805548F: ??? (in /usr/bin/imsrun)
==20132==    by 0x8078A43: ??? (in /usr/bin/imsrun)
==20132==    by 0x804D70B: ??? (in /usr/bin/imsrun)
==20132==    by 0x805080F: ??? (in /usr/bin/imsrun)
==20132==    by 0x8072C81: ??? (in /usr/bin/imsrun)
==20132==    by 0x804B8D1: ??? (in /usr/bin/imsrun)
==20132==    by 0xAC6E8B: (below main) (in /lib/libc-2.5.so)
==20132== 
==20132== HEAP SUMMARY:
==20132==     in use at exit: 322,182 bytes in 12 blocks
==20132==   total heap usage: 18 allocs, 6 frees, 327,706 bytes allocated
==20132== 
==20132== LEAK SUMMARY:
==20132==    definitely lost: 0 bytes in 0 blocks
==20132==    indirectly lost: 0 bytes in 0 blocks
==20132==      possibly lost: 0 bytes in 0 blocks
==20132==    still reachable: 322,182 bytes in 12 blocks
==20132==         suppressed: 0 bytes in 0 blocks
==20132== Rerun with --leak-check=full to see details of leaked memory
==20132== 
==20132== For counts of detected and suppressed errors, rerun with: -v
==20132== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 16 from 8)
./vg-in-place: line 28: 20132 Illegal instruction     VALGRIND_LIB="$vgbasedir/.in_place" VALGRIND_LIB_INNER="$vgbasedir/.in_place" "$vgbasedir/coregrind/valgrind" "$@"
valgrind-trunk> echo $?
132
valgrind-trunk> ./vg-in-place ./none/tests/x86/aad_aam
valgrind: ./none/tests/x86/aad_aam: No such file or directory
valgrind-trunk> ls -l ./none/tests/x86/a*
ls: ./none/tests/x86/a*: No such file or directory
valgrind-trunk> ls -l ./none/tests/x86/*
-rwxrwxrwx 1 shawnc shawnc   7875 Jan 13 11:03 ./none/tests/x86/badseg
-rw-rw-rw- 1 shawnc shawnc    517 Jan 10 16:49 ./none/tests/x86/badseg.c
-rw-rw-rw- 1 shawnc shawnc   4320 Jan 13 11:03 ./none/tests/x86/badseg.o
-rw-rw-rw- 1 shawnc shawnc      2 Jan 10 16:49 ./none/tests/x86/badseg.stderr.exp
...


I used 'svn co svn://svn.valgrind.org/valgrind/trunk valgrind-trunk' to get the source tree.  Maybe the patch hasn't been committed to trunk?

Thanks,
STC

Committed, vex r2078, valgrind r11502 (tests).  Vince, thanks!

Pulled down updated trunk and tried again.  It works!  Thank you!

I'll see if I can talk my boss into making a contribution for your efforts.  :-)

Shawn