[DynInst_API:] [PATCH RFC] Issue a diagnostic error if x86_64 lacks LAHF/SAHF

[Josh Stone <jistone@xxxxxxxxxx>]

It was noted in Dyninst 7.0 known bugs that x86_64 CPUs which lacked
LAHF/SAHF would not be able to run instrumentation.  This limitation
still holds in 8.x, even though it's no longer mentioned.

There was already one cpuid check for xmm capability.  It's also
possible to check lahf_lm in cpuid 80000001H ECX.  This commit moves to
using more generic cpuid support from compilers, and then issues a
diagnostic error message if lahf_lm is missing.
---

RFC: I used the __cpuid() compiler intrinsic for Windows, and cpuid.h
__get_cpuid() elsewhere.  I know Linux GCC provides that header, but I'm
not sure about other platforms/compilers that dyninst cares about.

Opinions are also welcome on the whole concept of issuing this message.
It doesn't actually stop anything from proceeding, but at least should
give a clue if things blow up as expected.

---
 dyninstAPI/CMakeLists.txt  |  4 ---
 dyninstAPI/src/cpuid-x86.S | 18 -------------
 dyninstAPI/src/emit-x86.C  | 11 ++++++++
 dyninstAPI/src/inst-x86.C  | 64 +++++++++++++++++++++++-----------------------
 dyninstAPI/src/inst-x86.h  |  6 +++++
 5 files changed, 49 insertions(+), 54 deletions(-)
 delete mode 100644 dyninstAPI/src/cpuid-x86.S

diff --git a/dyninstAPI/CMakeLists.txt b/dyninstAPI/CMakeLists.txt
index e5e4e10ff875..6b56c9d192b4 100644
--- a/dyninstAPI/CMakeLists.txt
+++ b/dyninstAPI/CMakeLists.txt
@@ -161,10 +161,6 @@ endif()
 
 set_source_files_properties(${SRC_LIST} PROPERTIES LANGUAGE CXX)
 add_definitions(-DBPATCH_DLL_BUILD)
-if (PLATFORM MATCHES i386 AND UNIX)
-set (SRC_LIST ${SRC_LIST} src/cpuid-x86.S)
-set_source_files_properties(src/cpuid-x86.S PROPERTIES LANGUAGE C)
-endif ()
 
 add_library (dyninstAPI ${SRC_LIST})
 add_library (dyninstAPI_static STATIC ${SRC_LIST})
diff --git a/dyninstAPI/src/cpuid-x86.S b/dyninstAPI/src/cpuid-x86.S
deleted file mode 100644
index 912b69036193..000000000000
--- a/dyninstAPI/src/cpuid-x86.S
+++ /dev/null
@@ -1,18 +0,0 @@
-	.text
-.globl cpuidCall
-	.type	cpuidCall, @function
-cpuidCall:
-	pushl   %ebp
-	movl    %esp, %ebp
-	subl    $8, %esp
-	movl    %ebx, 4(%esp)
-	movl    $1, %eax
-	cpuid
-	movl	%edx, %eax
-	movl    4(%esp), %ebx
-	movl    %ebp, %esp
-	popl    %ebp	
-	ret
-	.size	cpuidCall, .-cpuidCall
-	.section	.note.GNU-stack,"",@progbits
-	.ident	"GCC: (GNU) 3.3.3"
diff --git a/dyninstAPI/src/emit-x86.C b/dyninstAPI/src/emit-x86.C
index 00e1a3f27156..ca7fc7f19c83 100644
--- a/dyninstAPI/src/emit-x86.C
+++ b/dyninstAPI/src/emit-x86.C
@@ -2328,6 +2328,17 @@ bool shouldSaveReg(registerSlot *reg, baseTramp *inst, bool saveFlags)
 
 void EmitterAMD64::emitStackAlign(int offset, codeGen &gen)
 {
+   static bool checked_lahf = false;
+   if (!checked_lahf) {
+      /* Bit 0 of ECX from CPUID 80000001H tells whether LAHF/SAHF are
+       * available in 64-bit mode.  (Early steppings lacked this.)  */
+      unsigned int eax, ebx, ecx, edx;
+      get_cpuid(0x80000001, eax, ebx, ecx, edx);
+      if (!(ecx & 1U))
+         bperr("This processor doesn't support LAHF/SAHF in 64-bit mode.");
+      checked_lahf = true;
+   }
+
    int off = offset + 8 + AMD64_STACK_ALIGNMENT;
    int saveSlot1 =    0 + AMD64_STACK_ALIGNMENT;
    int saveSlot2 =    8 + AMD64_STACK_ALIGNMENT;
diff --git a/dyninstAPI/src/inst-x86.C b/dyninstAPI/src/inst-x86.C
index fa25e528ce71..db84c54fb544 100644
--- a/dyninstAPI/src/inst-x86.C
+++ b/dyninstAPI/src/inst-x86.C
@@ -67,11 +67,33 @@
 #include <sstream>
 #include <assert.h>
 
+
+#if !defined(os_windows)
+#include <cpuid.h>
+#endif
+
+void get_cpuid(unsigned int level,
+               unsigned int& eax,
+               unsigned int& ebx,
+               unsigned int& ecx,
+               unsigned int& edx)
+{
+#if defined(os_windows)
+  int cpuInfo[4];
+  __cpuid(cpuInfo, level);
+  eax = cpuInfo[0];
+  ebx = cpuInfo[1];
+  ecx = cpuInfo[2];
+  edx = cpuInfo[3];
+#else
+  __get_cpuid(level, &eax, &ebx, &ecx, &edx);
+#endif
+}
+
+
 class ExpandInstruction;
 class InsertNops;
 
-extern "C" int cpuidCall();
-
 /****************************************************************************/
 /****************************************************************************/
 /****************************************************************************/
@@ -530,43 +552,21 @@ void registerSpace::initialize()
 #endif
 }
 
-/* This makes a call to the cpuid instruction, which returns an int where each bit is 
-   a feature.  Bit 24 contains whether fxsave is possible, meaning that xmm registers
-   are saved. */
-#if defined(os_windows)
-int cpuidCall() {
-    DWORD result = 0;
-// Note: mov <target> <source>, so backwards from what gnu uses
-	_asm {
-		push ebx
-		mov eax, 1
-		cpuid
-		pop ebx
-		mov result, edx
-    }
-    return result;
-}
-#endif
 
+bool xmmCapable()
+{
 #if !defined(x86_64_unknown_linux2_4)              \
  && !(defined(os_freebsd) && defined(arch_x86_64)) \
  && !defined(os_vxworks)
-bool xmmCapable()
-{
-  int features = cpuidCall();
-  char * ptr = (char *)&features;
-  ptr += 3;
-  if (0x1 & (*ptr))
-    return true;
-  else
-    return false;
-}
+   /* Bit 24 of EDX from CPUID 1 tells whether fxsave is possible,
+    * meaning that xmm registers should be saved. */
+  unsigned int eax, ebx, ecx, edx;
+  get_cpuid(1, eax, ebx, ecx, edx);
+  return edx & (1U << 24);
 #else
-bool xmmCapable()
-{
   return true;
-}
 #endif
+}
 
 bool baseTramp::generateSaves(codeGen& gen, registerSpace*) {
    return gen.codeEmitter()->emitBTSaves(this, gen);
diff --git a/dyninstAPI/src/inst-x86.h b/dyninstAPI/src/inst-x86.h
index 9c0ec27c46dd..ad4363d3b880 100644
--- a/dyninstAPI/src/inst-x86.h
+++ b/dyninstAPI/src/inst-x86.h
@@ -148,6 +148,12 @@
 
 class codeGen;
 
+void get_cpuid(unsigned int level,
+               unsigned int& eax,
+               unsigned int& ebx,
+               unsigned int& ecx,
+               unsigned int& edx);
+
 // Define access method for saved register (GPR)
 #define GET_GPR(x, insn) emitMovRMToReg(REGNUM_EAX, REGNUM_EBP, SAVED_EAX_OFFSET-(x*4), insn)
 
-- 
1.9.3