i#6698 instr_t ISA mode: move from flags to its own field

Moves instruction-related ISA mode from instr_t flags field to a new dr_isa_mode_t field in instr_t and updates setter and getter functions for instr_t ISA mode. Fixes #6698
DynamoRIO · Mar 8, 2024 · c4b9371 · c4b9371
1 parent 79b00af
commit c4b9371
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 55 deletions.
diff --git a/core/ir/aarch64/instr.c b/core/ir/aarch64/instr.c
@@ -35,18 +35,23 @@
 #include "instr.h"
 #include "decode.h"
 
+#include "encode_api.h"
 #include "opcode_names.h"
 
 bool
 instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
 {
-    return (mode == DR_ISA_ARM_A64);
+    if (mode != DR_ISA_ARM_A64) {
+        return false;
+    }
+    instr->isa_mode = DR_ISA_ARM_A64;
+    return true;
 }
 
 dr_isa_mode_t
 instr_get_isa_mode(instr_t *instr)
 {
-    return DR_ISA_ARM_A64;
+    return instr->isa_mode;
 }
 
 int

diff --git a/core/ir/arm/instr.c b/core/ir/arm/instr.c
@@ -39,19 +39,17 @@
 bool
 instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
 {
-    if (mode == DR_ISA_ARM_THUMB)
-        instr->flags |= INSTR_THUMB_MODE;
-    else if (mode == DR_ISA_ARM_A32)
-        instr->flags &= ~INSTR_THUMB_MODE;
-    else
+    if ((mode != DR_ISA_ARM_THUMB) && (mode != DR_ISA_ARM_A32)) {
         return false;
+    }
+    instr->isa_mode = mode;
     return true;
 }
 
 dr_isa_mode_t
 instr_get_isa_mode(instr_t *instr)
 {
-    return TEST(INSTR_THUMB_MODE, instr->flags) ? DR_ISA_ARM_THUMB : DR_ISA_ARM_A32;
+    return instr->isa_mode;
 }
 
 int

diff --git a/core/ir/instr.h b/core/ir/instr.h
@@ -202,19 +202,8 @@ enum {
     INSTR_DO_NOT_EMIT = 0x10000000,
     /* PR 251479: re-relativization support: is instr->rip_rel_pos valid? */
     INSTR_RIP_REL_VALID = 0x20000000,
-#ifdef X86
-    /* PR 278329: each instr stores its own mode */
-    INSTR_X86_MODE = 0x40000000,
-#elif defined(ARM)
-    /* We assume we don't need to distinguish A64 from A32 as you cannot swap
-     * between them in user mode.  Thus we only need one flag.
-     * XXX: we might want more power for drdecode, though the global isa_mode
-     * should be sufficient there.
-     */
-    INSTR_THUMB_MODE = 0x40000000,
-#endif
     /* PR 267260: distinguish our own mangling from client-added instrs */
-    INSTR_OUR_MANGLING = 0x80000000,
+    INSTR_OUR_MANGLING = 0x40000000,
 };
 
 #define DR_TUPLE_TYPE_BITS 4

diff --git a/core/ir/instr_api.h b/core/ir/instr_api.h
@@ -274,6 +274,11 @@ struct _instr_t {
     /* flags contains the constants defined above */
     uint flags;
 
+    /* Instruction ISA mode used to distinguish between 32/64-bit
+     * architectures for encoding purposes.
+     */
+    dr_isa_mode_t isa_mode;
+
     /* hints for encoding this instr in a specific way, holds dr_encoding_hint_type_t */
     uint encoding_hints;
 
@@ -339,7 +344,7 @@ struct _instr_t {
 
     /* Used to hold the relative offset within an instruction list when encoding. */
     size_t offset;
-};     /* instr_t */
+}; /* instr_t */
 #endif /* DR_FAST_IR */
 
 /**

diff --git a/core/ir/riscv64/instr.c b/core/ir/riscv64/instr.c
@@ -36,13 +36,17 @@
 bool
 instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
 {
-    return (mode == DR_ISA_RV64IMAFDC);
+    if (mode != DR_ISA_RV64IMAFDC) {
+        return false;
+    }
+    instr->isa_mode = DR_ISA_RV64IMAFDC;
+    return true;
 }
 
 dr_isa_mode_t
 instr_get_isa_mode(instr_t *instr)
 {
-    return DR_ISA_RV64IMAFDC;
+    return instr->isa_mode;
 }
 
 int

diff --git a/core/ir/x86/instr.c b/core/ir/x86/instr.c
@@ -40,8 +40,31 @@
 #include "instr.h"
 #include "decode.h"
 #include "decode_private.h"
+#include "encode_api.h"
 #include "instr_create_shared.h"
 
+bool
+instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
+{
+#ifdef X64
+    if ((mode != DR_ISA_IA32) && (mode != DR_ISA_AMD64)) {
+        return false;
+    }
+#else
+    if (mode != DR_ISA_IA32) {
+        return false;
+    }
+#endif
+    instr->isa_mode = mode;
+    return true;
+}
+
+dr_isa_mode_t
+instr_get_isa_mode(instr_t *instr)
+{
+    return instr->isa_mode;
+}
+
 #ifdef X64
 /*
  * Each instruction stores whether it should be interpreted in 32-bit
@@ -50,10 +73,11 @@
 void
 instr_set_x86_mode(instr_t *instr, bool x86)
 {
-    if (x86)
-        instr->flags |= INSTR_X86_MODE;
-    else
-        instr->flags &= ~INSTR_X86_MODE;
+    if (x86) {
+        instr->isa_mode = DR_ISA_IA32;
+    } else {
+        instr->isa_mode = DR_ISA_AMD64;
+    }
 }
 
 /*
@@ -63,37 +87,10 @@ instr_set_x86_mode(instr_t *instr, bool x86)
 bool
 instr_get_x86_mode(instr_t *instr)
 {
-    return TEST(INSTR_X86_MODE, instr->flags);
+    return (instr->isa_mode == DR_ISA_IA32);
 }
 #endif
 
-bool
-instr_set_isa_mode(instr_t *instr, dr_isa_mode_t mode)
-{
-#ifdef X64
-    if (mode == DR_ISA_IA32)
-        instr_set_x86_mode(instr, true);
-    else if (mode == DR_ISA_AMD64)
-        instr_set_x86_mode(instr, false);
-    else
-        return false;
-#else
-    if (mode != DR_ISA_IA32)
-        return false;
-#endif
-    return true;
-}
-
-dr_isa_mode_t
-instr_get_isa_mode(instr_t *instr)
-{
-#ifdef X64
-    return TEST(INSTR_X86_MODE, instr->flags) ? DR_ISA_IA32 : DR_ISA_AMD64;
-#else
-    return DR_ISA_IA32;
-#endif
-}
-
 int
 instr_length_arch(dcontext_t *dcontext, instr_t *instr)
 {