Bug 249991

Summary: Valgrind incorrectly declares AESKEYGENASSIST support since VEX r2011
Product: [Developer tools] valgrind Reporter: Alexander Potapenko <glider>
Component: vexAssignee: Julian Seward <jseward>
Status: RESOLVED FIXED    
Severity: normal CC: timurrrr
Priority: NOR    
Version First Reported In: 3.6 SVN   
Target Milestone: ---   
Platform: Unlisted Binaries   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description Alexander Potapenko 2010-09-03 12:27:29 UTC 
Starting with Valgrind r11269, VEX r2011 Valgrind fails Chromium base_unittests with the following error message:

====================================
$ ./memcheck-11269.sh ./base_unittests --gtest_filter=*Encr*
...
Note: Google Test filter = *Encr*
[==========] Running 2 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 2 tests from EncryptorTest
[ RUN      ] EncryptorTest.EncryptDecrypt
vex amd64->IR: unhandled instruction bytes: 0x66 0xF 0x3A 0xDF 0xD3 0x1
====================================

, while everything is ok for Valgrind r11268, VEX r2010.

The unhandled instruction is AESKEYGENASSIST (see http://software.intel.com/zh-cn/blogs/wordpress/wp-content/uploads/2010/05/aes-instructions-set_wp.pdf)
However, this instruction is not supported on that CPU: I've compiled the following code:
====================
int main() {
  char arg[16];
  asm volatile("movupd 0(%0), %%xmm3\n"
               "movupd 0(%0), %%xmm2\n"
               "nop\nnop\nnop\nnop\n"
               "nop\nnop\nnop\nnop\n"
               :: "r"(arg)
               );
  return 0;
}
====================
And replaced six NOPs in the resulting binary with "66 0F 3A DF D3 01"

The binary fails to run on both Valgrind versions I've tried, and also dies with "Illegal instruction" natively.

Looks like the Chromium test exploits AESKEYGENASSIST conditionally under Valgrind, but not natively.

According to Intel spec (see the link above): "AES extensions is supported if CPUID.01H:ECX.AES[bit 25] = 1."

The following code:
====================
#include <stdio.h>
int main() {
  unsigned int result = -1;
  asm volatile(
        "mov $0x1, %%eax\n"
        "cpuid\n"
        "mov $0x02000000, %%eax\n"
        "and %%eax, %%ecx\n"
        "mov %%ecx, %0\n": "=r"(result));
  printf("%x\n", result);
  return 0;
}
====================

prints "0" for the native run and for VEX r2010, but for VEX r2011 the result is "2000000"
If the Intel document is correct, Valgrind shouldn't set this bit until it fully supports the AES extension.
Comment 1 Alexander Potapenko 2010-09-03 12:34:20 UTC 
Here are the contents of /proc/cpuid:


$ cat /proc/cpuinfo 
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 26
model name	: Intel(R) Xeon(R) CPU           X5550  @ 2.67GHz
stepping	: 5
cpu MHz		: 2660.000
cache size	: 8192 KB
fpu		: yes
fpu_exception	: yes
cpuid level	: 11
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good pni ssse3 cx16 sse4_1 sse4_2 popcnt lahf_lm ida
bogomips	: 5320.00
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management:

processor	: 1
vendor_id	: GenuineIntel
cpu family	: 6
model		: 26
model name	: Intel(R) Xeon(R) CPU           X5550  @ 2.67GHz
stepping	: 5
cpu MHz		: 2660.000
cache size	: 8192 KB
fpu		: yes
fpu_exception	: yes
cpuid level	: 11
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good pni ssse3 cx16 sse4_1 sse4_2 popcnt lahf_lm ida
bogomips	: 5319.64
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management:

processor	: 2
vendor_id	: GenuineIntel
cpu family	: 6
model		: 26
model name	: Intel(R) Xeon(R) CPU           X5550  @ 2.67GHz
stepping	: 5
cpu MHz		: 2660.000
cache size	: 8192 KB
fpu		: yes
fpu_exception	: yes
cpuid level	: 11
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good pni ssse3 cx16 sse4_1 sse4_2 popcnt lahf_lm ida
bogomips	: 5319.74
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management:

processor	: 3
vendor_id	: GenuineIntel
cpu family	: 6
model		: 26
model name	: Intel(R) Xeon(R) CPU           X5550  @ 2.67GHz
stepping	: 5
cpu MHz		: 2660.000
cache size	: 8192 KB
fpu		: yes
fpu_exception	: yes
cpuid level	: 11
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good pni ssse3 cx16 sse4_1 sse4_2 popcnt lahf_lm ida
bogomips	: 5323.29
clflush size	: 64
cache_alignment	: 64
address sizes	: 40 bits physical, 48 bits virtual
power management:
Comment 2 Julian Seward 2010-09-28 22:53:32 UTC 
Alexander, can you try this?

Index: priv/guest_amd64_helpers.c
===================================================================
--- priv/guest_amd64_helpers.c  (revision 2051)
+++ priv/guest_amd64_helpers.c  (working copy)
@@ -2065,7 +2065,8 @@
          SET_ABCD(0x0000000b, 0x756e6547, 0x6c65746e, 0x49656e69);
          break;
       case 0x00000001:
-         SET_ABCD(0x00020652, 0x00100800, 0x0298e3ff, 0xbfebfbff);
+         SET_ABCD(0x00020652, 0x00100800, 0x0298e3ff & ~(1<<25),
+                                          0xbfebfbff);
          break;
       case 0x00000002:
          SET_ABCD(0x55035a01, 0x00f0b2e3, 0x00000000, 0x09ca212c);
Comment 3 Alexander Potapenko 2010-09-29 14:27:26 UTC 
(In reply to comment #2)

Yes, this fixes the bug, thank you!

BTW, cpuid (http://www.etallen.com/cpuid.html) should help in testing such patches.
Comment 4 Julian Seward 2010-09-30 12:52:54 UTC 
Fixed, r2055.