From 3c528c0bf9a497b413f9d9711404f3327b1d2881 Mon Sep 17 00:00:00 2001 From: Guinevere Larsen Date: Thu, 29 May 2025 13:56:54 -0300 Subject: [PATCH] Backport amd64 decoding fixes Resolves: RHEL-93580 --- _gdb.spec.Patch.include | 52 +++++ _gdb.spec.patch.include | 13 ++ _patch_order | 13 ++ ...29-vandps-clobbers-registers-1-of-13.patch | 115 ++++++++++ ...9-vandps-clobbers-registers-10-of-13.patch | 61 +++++ ...9-vandps-clobbers-registers-11-of-13.patch | 86 +++++++ ...9-vandps-clobbers-registers-12-of-13.patch | 183 +++++++++++++++ ...9-vandps-clobbers-registers-13-of-13.patch | 212 ++++++++++++++++++ ...29-vandps-clobbers-registers-2-of-13.patch | 66 ++++++ ...29-vandps-clobbers-registers-3-of-13.patch | 100 +++++++++ ...29-vandps-clobbers-registers-4-of-13.patch | 59 +++++ ...29-vandps-clobbers-registers-5-of-13.patch | 137 +++++++++++ ...29-vandps-clobbers-registers-6-of-13.patch | 55 +++++ ...29-vandps-clobbers-registers-7-of-13.patch | 55 +++++ ...29-vandps-clobbers-registers-8-of-13.patch | 112 +++++++++ ...29-vandps-clobbers-registers-9-of-13.patch | 78 +++++++ gdb.spec | 6 +- 17 files changed, 1402 insertions(+), 1 deletion(-) create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch create mode 100644 gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch diff --git a/_gdb.spec.Patch.include b/_gdb.spec.Patch.include index b18fc10..a3c18eb 100644 --- a/_gdb.spec.Patch.include +++ b/_gdb.spec.Patch.include @@ -61,3 +61,55 @@ Patch012: gdb-add-rpm-suggestion-script.patch # (Jens Remus, RHEL-50069) Patch013: gdb-rhel-50069-support-z17.patch +# [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 1 +# (Tom de Vries, RHEL-7329) +Patch014: gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch + +# [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 2 +# (Tom de Vries, RHEL-7329) +Patch015: gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch + +# [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 3 +# (Tom de Vries, RHEL-7329) +Patch016: gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch + +# [gdb/tdep] Factor out amd64_get_used_input_int_regs +# (Tom de Vries, RHEL-7329) +Patch017: gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch + +# [gdb/tdep] Add amd64-insn-decode selftest +# (Tom de Vries, RHEL-7329) +Patch018: gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch + +# [gdb/tdep] Factor out rip_relative_p +# (Tom de Vries, RHEL-7329) +Patch019: gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch + +# [gdb/tdep] Fix rip-relative insn handling in amd64_get_used_input_int_reg +# (Tom de Vries, RHEL-7329) +Patch020: gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch + +# [gdb/tdep] Factor out part of fixup_riprel +# (Tom de Vries, RHEL-7329) +Patch021: gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch + +# [gdb/tdep] Add vex2_to_vex3 +# (Tom de Vries, RHEL-7329) +Patch022: gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch + +# [gdb/tdep] Add vzeroupper and vzeroall in amd64-insn-decode selftest +# (Tom de Vries, RHEL-7329) +Patch023: gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch + +# [gdb/tdep] Make amd64_get_insn_details more regular +# (Tom de Vries, RHEL-7329) +Patch024: gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch + +# [gdb/tdep] Fix vmovdqu decoding +# (Tom de Vries, RHEL-7329) +Patch025: gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch + +# [gdb/tdep] Support REX2 and EVEX prefix +# (Tom de Vries, RHEL-7329) +Patch026: gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch + diff --git a/_gdb.spec.patch.include b/_gdb.spec.patch.include index cb13330..5d59f3c 100644 --- a/_gdb.spec.patch.include +++ b/_gdb.spec.patch.include @@ -11,3 +11,16 @@ %patch -p1 -P011 %patch -p1 -P012 %patch -p1 -P013 +%patch -p1 -P014 +%patch -p1 -P015 +%patch -p1 -P016 +%patch -p1 -P017 +%patch -p1 -P018 +%patch -p1 -P019 +%patch -p1 -P020 +%patch -p1 -P021 +%patch -p1 -P022 +%patch -p1 -P023 +%patch -p1 -P024 +%patch -p1 -P025 +%patch -p1 -P026 diff --git a/_patch_order b/_patch_order index bf9dc18..182771b 100644 --- a/_patch_order +++ b/_patch_order @@ -11,3 +11,16 @@ gdb-rhbz1084404-ppc64-s390x-wrong-prologue-skip-O2-g-3of3.patch gdb-add-index.patch gdb-add-rpm-suggestion-script.patch gdb-rhel-50069-support-z17.patch +gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch +gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch diff --git a/gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch new file mode 100644 index 0000000..f3e9017 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch @@ -0,0 +1,115 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-1-of-13.patch + +;; [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 1 +;; (Tom de Vries, RHEL-7329) + +While reading amd64_get_unused_input_int_reg, I noticed that it first asserts, +then throws an internal_error if no unused register can be found. + +Looking at the documentation of gdbarch_displaced_step_copy_insn, it seems +that a failure can be indicated less abruptly, by returning a nullptr. + +Fix this by: +- returning -1 in case of failure to find an unused register in + amd64_get_unused_input_int_reg, and +- propagating this to amd64_displaced_step_copy_insn. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1211,7 +1211,7 @@ amd64_skip_prefixes (gdb_byte *insn) + In order to not require adding a rex prefix if the insn doesn't already + have one, the result is restricted to RAX ... RDI, sans RSP. + The register numbering of the result follows architecture ordering, +- e.g. RDI = 7. */ ++ e.g. RDI = 7. Return -1 if no register can be found. */ + + static int + amd64_get_unused_input_int_reg (const struct amd64_insn *details) +@@ -1263,7 +1263,6 @@ amd64_get_unused_input_int_reg (const struct amd64_insn *details) + } + + gdb_assert (used_regs_mask < 256); +- gdb_assert (used_regs_mask != 255); + + /* Finally, find a free reg. */ + { +@@ -1274,10 +1273,9 @@ amd64_get_unused_input_int_reg (const struct amd64_insn *details) + if (! (used_regs_mask & (1 << i))) + return i; + } +- +- /* We shouldn't get here. */ +- internal_error (_("unable to find free reg")); + } ++ ++ return -1; + } + + /* Extract the details of INSN that we need. */ +@@ -1361,9 +1359,10 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + 32 bits is not enough to be guaranteed to cover the distance between where + the real instruction is and where its copy is. + Convert the insn to use base+disp addressing. +- We set base = pc + insn_length so we can leave disp unchanged. */ ++ We set base = pc + insn_length so we can leave disp unchanged. ++ Return true if successful, false otherwise. */ + +-static void ++static bool + fixup_riprel (struct gdbarch *gdbarch, + amd64_displaced_step_copy_insn_closure *dsc, + CORE_ADDR from, CORE_ADDR to, struct regcache *regs) +@@ -1384,6 +1383,9 @@ fixup_riprel (struct gdbarch *gdbarch, + Pick one not used in the insn. + NOTE: arch_tmp_regno uses architecture ordering, e.g. RDI = 7. */ + arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details); ++ if (arch_tmp_regno == -1) ++ return false; ++ + tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno); + + /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ +@@ -1418,9 +1420,13 @@ fixup_riprel (struct gdbarch *gdbarch, + displaced_debug_printf ("using temp reg %d, old value %s, new value %s", + dsc->tmp_regno, paddress (gdbarch, dsc->tmp_save), + paddress (gdbarch, rip_base)); ++ return true; + } + +-static void ++/* Fixup the insn in DSC->insn_buf, which was copied from address FROM to TO. ++ Return true if successful, false otherwise. */ ++ ++static bool + fixup_displaced_copy (struct gdbarch *gdbarch, + amd64_displaced_step_copy_insn_closure *dsc, + CORE_ADDR from, CORE_ADDR to, struct regcache *regs) +@@ -1435,9 +1441,11 @@ fixup_displaced_copy (struct gdbarch *gdbarch, + { + /* The insn uses rip-relative addressing. + Deal with it. */ +- fixup_riprel (gdbarch, dsc, from, to, regs); ++ return fixup_riprel (gdbarch, dsc, from, to, regs); + } + } ++ ++ return true; + } + + displaced_step_copy_insn_closure_up +@@ -1475,7 +1483,8 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, + + /* Modify the insn to cope with the address where it will be executed from. + In particular, handle any rip-relative addressing. */ +- fixup_displaced_copy (gdbarch, dsc.get (), from, to, regs); ++ if (!fixup_displaced_copy (gdbarch, dsc.get (), from, to, regs)) ++ return nullptr; + + write_memory (to, buf, len); + diff --git a/gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch new file mode 100644 index 0000000..97e5f47 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch @@ -0,0 +1,61 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-10-of-13.patch + +;; [gdb/tdep] Add vzeroupper and vzeroall in amd64-insn-decode selftest +;; (Tom de Vries, RHEL-7329) + +After I posted a tentative patch for PR31952, Alexander Monakov pointed out +that the patch broke instruction decoding for instructions vzeroall and +vzeroupper. + +Add selftests for these two instructions in amd64-insn-decode, both using +vex2 and vex3 prefixes. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -3512,6 +3512,40 @@ test_amd64_get_insn_details (void) + gdb::byte_vector updated_insn = { 0x48, 0x8d, 0xb9, 0x1e, 0x00, 0x00, 0x00 }; + fixup_riprel (details, insn.data (), ECX_REG_NUM); + SELF_CHECK (insn == updated_insn); ++ ++ gdb::byte_vector vex2, vex3; ++ ++ /* INSN: vzeroall, vex2 prefix. */ ++ vex2 = { 0xc5, 0xfc, 0x77 }; ++ amd64_get_insn_details (vex2.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.opcode_offset == 2); ++ SELF_CHECK (details.modrm_offset == -1); ++ ++ /* INSN: vzeroall, vex3 prefix. */ ++ vex2_to_vex3 (vex2, vex3); ++ amd64_get_insn_details (vex3.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 3); ++ SELF_CHECK (details.modrm_offset == -1); ++ ++ /* INSN: vzeroupper, vex2 prefix. */ ++ vex2 = { 0xc5, 0xf8, 0x77 }; ++ amd64_get_insn_details (vex2.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.opcode_offset == 2); ++ SELF_CHECK (details.modrm_offset == -1); ++ ++ /* INSN: vzeroupper, vex3 prefix. */ ++ vex2_to_vex3 (vex2, vex3); ++ amd64_get_insn_details (vex3.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 3); ++ SELF_CHECK (details.modrm_offset == -1); + } + + static void diff --git a/gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch new file mode 100644 index 0000000..55d3b91 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch @@ -0,0 +1,86 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-11-of-13.patch + +;; [gdb/tdep] Make amd64_get_insn_details more regular +;; (Tom de Vries, RHEL-7329) + +In amd64_get_insn_details, I found this code with a comment explaining why +enc_prefix_offset is not set: +... + else if (vex2_prefix_p (*insn)) + { + /* Don't record the offset in this case because this prefix has + no REX.B equivalent. */ + insn += 2; + } +... +which I didn't understand until I looked at the only use of enc_prefix_offset, +in fixup_riprel: +... + /* REX.B should be unset (VEX.!B set) as we were using rip-relative + addressing, but ensure it's unset (set for VEX) anyway, tmp_regno + is not r8-r15. */ + if (insn_details->enc_prefix_offset != -1) + { + gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset]; + if (rex_prefix_p (pfx[0])) + pfx[0] &= ~REX_B; + else if (vex3_prefix_p (pfx[0])) + pfx[1] |= VEX3_NOT_B; + else + gdb_assert_not_reached ("unhandled prefix"); + } +... + +Fix this by: +- setting enc_prefix_offset for the vex2 case in amd64_get_insn_details, + making the function more regular and easier to understand, and +- handling the vex2 case in the "enc_prefix_offset != -1" clause in + fixup_riprel. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1328,8 +1328,7 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + } + else if (vex2_prefix_p (*insn)) + { +- /* Don't record the offset in this case because this prefix has +- no REX.B equivalent. */ ++ details->enc_prefix_offset = insn - start; + insn += 2; + } + else if (vex3_prefix_p (*insn)) +@@ -1395,6 +1394,10 @@ fixup_riprel (const struct amd64_insn &details, gdb_byte *insn, + gdb_byte *pfx = &insn[details.enc_prefix_offset]; + if (rex_prefix_p (pfx[0])) + pfx[0] &= ~REX_B; ++ else if (vex2_prefix_p (pfx[0])) ++ { ++ /* VEX.!B is set implicitly. */ ++ } + else if (vex3_prefix_p (pfx[0])) + pfx[1] |= VEX3_NOT_B; + else +@@ -3519,7 +3522,7 @@ test_amd64_get_insn_details (void) + vex2 = { 0xc5, 0xfc, 0x77 }; + amd64_get_insn_details (vex2.data (), &details); + SELF_CHECK (details.opcode_len == 1); +- SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 2); + SELF_CHECK (details.modrm_offset == -1); + +@@ -3535,7 +3538,7 @@ test_amd64_get_insn_details (void) + vex2 = { 0xc5, 0xf8, 0x77 }; + amd64_get_insn_details (vex2.data (), &details); + SELF_CHECK (details.opcode_len == 1); +- SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 2); + SELF_CHECK (details.modrm_offset == -1); + diff --git a/gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch new file mode 100644 index 0000000..90498bb --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch @@ -0,0 +1,183 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-12-of-13.patch + +;; [gdb/tdep] Fix vmovdqu decoding +;; (Tom de Vries, RHEL-7329) + +PR tdep/31952 reports that displaced stepping over an instruction pointer +relative insn "vmovdqu 0x20(%rip),%ymm1" gives the wrong results. + +This is caused by misclassification of the insn in amd64_get_insn_details, +which results in details.modrm_offset == -1, while the instruction in fact +does have a modrm byte. + +The instruction is encoded as follows: +... + 400557: c5 fe 6f 0d 20 00 00 00 vmovdqu 0x20(%rip),%ymm1 +... +where: +- "0xc5 0xfe" is the vex2 prefix, +- "0x6f" is the opcode, +- "0x0d" is the modrm byte, and +- "0x20 0x00 0x00 0x00" is a 32-bit displacement. + +The problem is related to details.opcode_len, which is 1. + +While it is true that the length of the opcode in the insn (0x6f) is 1 byte, +the vex2 prefix implies that we're encoding an 2-byte opcode beginnning +with 0x0f [1]. + +Consequently, we should be using the twobyte_has_modrm map rather than the +onebyte_has_modrm map. + +Fix this in amd64_get_insn_details, and add a selftest to check this. + +Tested on x86_64-linux. + +Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31952 + +[1] https://en.wikipedia.org/wiki/VEX_prefix + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1336,10 +1336,49 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + details->enc_prefix_offset = insn - start; + insn += 3; + } ++ gdb_byte *prefix = (details->enc_prefix_offset == -1 ++ ? nullptr ++ : &start[details->enc_prefix_offset]); + + details->opcode_offset = insn - start; + +- if (*insn == TWO_BYTE_OPCODE_ESCAPE) ++ if (prefix != nullptr && vex2_prefix_p (*prefix)) ++ { ++ need_modrm = twobyte_has_modrm[*insn]; ++ details->opcode_len = 2; ++ } ++ else if (prefix != nullptr && vex3_prefix_p (*prefix)) ++ { ++ need_modrm = twobyte_has_modrm[*insn]; ++ ++ gdb_byte m = prefix[1] & 0x1f; ++ if (m == 0) ++ { ++ /* Todo: Xeon Phi-specific JKZD/JKNZD. */ ++ return; ++ } ++ else if (m == 1) ++ { ++ /* Escape 0x0f. */ ++ details->opcode_len = 2; ++ } ++ else if (m == 2 || m == 3) ++ { ++ /* Escape 0x0f 0x38 or 0x0f 0x3a. */ ++ details->opcode_len = 3; ++ } ++ else if (m == 7) ++ { ++ /* Todo: URDMSR/UWRMSR instructions. */ ++ return; ++ } ++ else ++ { ++ /* Unknown opcode map. */ ++ return; ++ } ++ } ++ else if (*insn == TWO_BYTE_OPCODE_ESCAPE) + { + /* Two or three-byte opcode. */ + ++insn; +@@ -1513,6 +1552,8 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, + memset (buf + len, 0, fixup_sentinel_space); + + amd64_get_insn_details (buf, details); ++ if (details->opcode_len == -1) ++ return nullptr; + + /* GDB may get control back after the insn after the syscall. + Presumably this is a kernel bug. +@@ -3475,7 +3516,7 @@ static void + test_amd64_get_insn_details (void) + { + struct amd64_insn details; +- gdb::byte_vector insn; ++ gdb::byte_vector insn, tmp; + + /* INSN: add %eax,(%rcx). */ + insn = { 0x01, 0x01 }; +@@ -3521,7 +3562,7 @@ test_amd64_get_insn_details (void) + /* INSN: vzeroall, vex2 prefix. */ + vex2 = { 0xc5, 0xfc, 0x77 }; + amd64_get_insn_details (vex2.data (), &details); +- SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.opcode_len == 2); + SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 2); + SELF_CHECK (details.modrm_offset == -1); +@@ -3529,7 +3570,7 @@ test_amd64_get_insn_details (void) + /* INSN: vzeroall, vex3 prefix. */ + vex2_to_vex3 (vex2, vex3); + amd64_get_insn_details (vex3.data (), &details); +- SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.opcode_len == 2); + SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 3); + SELF_CHECK (details.modrm_offset == -1); +@@ -3537,7 +3578,7 @@ test_amd64_get_insn_details (void) + /* INSN: vzeroupper, vex2 prefix. */ + vex2 = { 0xc5, 0xf8, 0x77 }; + amd64_get_insn_details (vex2.data (), &details); +- SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.opcode_len == 2); + SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 2); + SELF_CHECK (details.modrm_offset == -1); +@@ -3545,10 +3586,40 @@ test_amd64_get_insn_details (void) + /* INSN: vzeroupper, vex3 prefix. */ + vex2_to_vex3 (vex2, vex3); + amd64_get_insn_details (vex3.data (), &details); +- SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.opcode_len == 2); + SELF_CHECK (details.enc_prefix_offset == 0); + SELF_CHECK (details.opcode_offset == 3); + SELF_CHECK (details.modrm_offset == -1); ++ ++ /* INSN: vmovdqu 0x9(%rip),%ymm3, vex2 prefix. */ ++ vex2 = { 0xc5, 0xfe, 0x6f, 0x1d, 0x09, 0x00, 0x00, 0x00 }; ++ amd64_get_insn_details (vex2.data (), &details); ++ SELF_CHECK (details.opcode_len == 2); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 2); ++ SELF_CHECK (details.modrm_offset == 3); ++ ++ /* INSN: vmovdqu 0x9(%rcx),%ymm3, vex2 prefix. */ ++ gdb::byte_vector updated_vex2 ++ = { 0xc5, 0xfe, 0x6f, 0x99, 0x09, 0x00, 0x00, 0x00 }; ++ tmp = vex2; ++ fixup_riprel (details, tmp.data (), ECX_REG_NUM); ++ SELF_CHECK (tmp == updated_vex2); ++ ++ /* INSN: vmovdqu 0x9(%rip),%ymm3, vex3 prefix. */ ++ vex2_to_vex3 (vex2, vex3); ++ amd64_get_insn_details (vex3.data (), &details); ++ SELF_CHECK (details.opcode_len == 2); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 3); ++ SELF_CHECK (details.modrm_offset == 4); ++ ++ /* INSN: vmovdqu 0x9(%rcx),%ymm3, vex3 prefix. */ ++ gdb::byte_vector updated_vex3; ++ vex2_to_vex3 (updated_vex2, updated_vex3); ++ tmp = vex3; ++ fixup_riprel (details, tmp.data (), ECX_REG_NUM); ++ SELF_CHECK (tmp == updated_vex3); + } + + static void diff --git a/gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch new file mode 100644 index 0000000..0d1fc91 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch @@ -0,0 +1,212 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-13-of-13.patch + +;; [gdb/tdep] Support REX2 and EVEX prefix +;; (Tom de Vries, RHEL-7329) + +The following amd64 insn: +... + 0: 67 d5 44 8d 3d 00 00 00 00 lea 0x0(%eip),%r31d +... +uses the REX2 prefix [1], which is currently not supported in +amd64_get_insn_details. + +Add the missing support in amd64_get_insn_details, as well as a corresponding +unit test. + +Likewise for an amd64 insn using an EVEX prefix [2]: +... + 0: 62 f1 7c 48 28 05 00 fc ff ff vmovaps -0x400(%rip),%zmm0 +... + +Tested on x86_64-linux. + +PR tdep/32725 +Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32725 + +[1] https://en.wikipedia.org/wiki/VEX_prefix +[2] https://en.wikipedia.org/wiki/EVEX_prefix + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1158,6 +1158,14 @@ rex_prefix_p (gdb_byte pfx) + return REX_PREFIX_P (pfx); + } + ++/* True if PFX is the start of the 2-byte REX2 prefix. */ ++ ++static bool ++rex2_prefix_p (gdb_byte pfx) ++{ ++ return pfx == REX2_OPCODE; ++} ++ + /* True if PFX is the start of the 2-byte VEX prefix. */ + + static bool +@@ -1174,6 +1182,14 @@ vex3_prefix_p (gdb_byte pfx) + return pfx == 0xc4; + } + ++/* Return true if PFX is the start of the 4-byte EVEX prefix. */ ++ ++static bool ++evex_prefix_p (gdb_byte pfx) ++{ ++ return pfx == 0x62; ++} ++ + /* Skip the legacy instruction prefixes in INSN. + We assume INSN is properly sentineled so we don't have to worry + about falling off the end of the buffer. */ +@@ -1326,6 +1342,11 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + details->enc_prefix_offset = insn - start; + ++insn; + } ++ else if (rex2_prefix_p (*insn)) ++ { ++ details->enc_prefix_offset = insn - start; ++ insn += 2; ++ } + else if (vex2_prefix_p (*insn)) + { + details->enc_prefix_offset = insn - start; +@@ -1336,13 +1357,32 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + details->enc_prefix_offset = insn - start; + insn += 3; + } ++ else if (evex_prefix_p (*insn)) ++ { ++ details->enc_prefix_offset = insn - start; ++ insn += 4; ++ } + gdb_byte *prefix = (details->enc_prefix_offset == -1 + ? nullptr + : &start[details->enc_prefix_offset]); + + details->opcode_offset = insn - start; + +- if (prefix != nullptr && vex2_prefix_p (*prefix)) ++ if (prefix != nullptr && rex2_prefix_p (*prefix)) ++ { ++ bool m = (prefix[1] & (REX2_M << 4)) != 0; ++ if (!m) ++ { ++ need_modrm = onebyte_has_modrm[*insn]; ++ details->opcode_len = 1; ++ } ++ else ++ { ++ need_modrm = twobyte_has_modrm[*insn]; ++ details->opcode_len = 2; ++ } ++ } ++ else if (prefix != nullptr && vex2_prefix_p (*prefix)) + { + need_modrm = twobyte_has_modrm[*insn]; + details->opcode_len = 2; +@@ -1378,6 +1418,27 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + return; + } + } ++ else if (prefix != nullptr && evex_prefix_p (*prefix)) ++ { ++ need_modrm = twobyte_has_modrm[*insn]; ++ ++ gdb_byte m = prefix[1] & 0x7; ++ if (m == 1) ++ { ++ /* Escape 0x0f. */ ++ details->opcode_len = 2; ++ } ++ else if (m == 2 || m == 3) ++ { ++ /* Escape 0x0f 0x38 or 0x0f 0x3a. */ ++ details->opcode_len = 3; ++ } ++ else ++ { ++ /* Unknown opcode map. */ ++ return; ++ } ++ } + else if (*insn == TWO_BYTE_OPCODE_ESCAPE) + { + /* Two or three-byte opcode. */ +@@ -1425,6 +1486,15 @@ fixup_riprel (const struct amd64_insn &details, gdb_byte *insn, + /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ + static constexpr gdb_byte VEX3_NOT_B = 0x20; + ++ /* Position of the B3 and B4 bits in the REX2 prefix (in byte 1). */ ++ static constexpr gdb_byte REX2_B = 0x11; ++ ++ /* Position of the not-B3 bit in the EVEX prefix (in byte 1). */ ++ static constexpr gdb_byte EVEX_NOT_B = VEX3_NOT_B; ++ ++ /* Position of the B4 bit in the EVEX prefix (in byte 1). */ ++ static constexpr gdb_byte EVEX_B = 0x08; ++ + /* REX.B should be unset (VEX.!B set) as we were using rip-relative + addressing, but ensure it's unset (set for VEX) anyway, tmp_regno + is not r8-r15. */ +@@ -1433,12 +1503,19 @@ fixup_riprel (const struct amd64_insn &details, gdb_byte *insn, + gdb_byte *pfx = &insn[details.enc_prefix_offset]; + if (rex_prefix_p (pfx[0])) + pfx[0] &= ~REX_B; ++ else if (rex2_prefix_p (pfx[0])) ++ pfx[1] &= ~REX2_B; + else if (vex2_prefix_p (pfx[0])) + { + /* VEX.!B is set implicitly. */ + } + else if (vex3_prefix_p (pfx[0])) + pfx[1] |= VEX3_NOT_B; ++ else if (evex_prefix_p (pfx[0])) ++ { ++ pfx[1] |= EVEX_NOT_B; ++ pfx[1] &= ~EVEX_B; ++ } + else + gdb_assert_not_reached ("unhandled prefix"); + } +@@ -3620,6 +3697,37 @@ test_amd64_get_insn_details (void) + tmp = vex3; + fixup_riprel (details, tmp.data (), ECX_REG_NUM); + SELF_CHECK (tmp == updated_vex3); ++ ++ /* INSN: lea 0x0(%eip),%r31d, rex2 prefix. */ ++ insn = { 0x67, 0xd5, 0x44, 0x8d, 0x3d, 0x00, 0x00, 0x00, 0x00 }; ++ amd64_get_insn_details (insn.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == 1); ++ SELF_CHECK (details.opcode_offset == 3); ++ SELF_CHECK (details.modrm_offset == 4); ++ /* This is incorrect, r31 is used instead of rdi, but currently that doesn't ++ matter. */ ++ SELF_CHECK (amd64_get_used_input_int_regs (&details, false) ++ == (1 << EDI_REG_NUM)); ++ ++ /* INSN: lea 0x0(%ecx),%r31d, rex2 prefix. */ ++ updated_insn = { 0x67, 0xd5, 0x44, 0x8d, 0xb9, 0x00, 0x00, 0x00, 0x00 }; ++ fixup_riprel (details, insn.data (), ECX_REG_NUM); ++ SELF_CHECK (insn == updated_insn); ++ ++ /* INSN: vmovaps -0x400(%rip),%zmm0, evex prefix. */ ++ insn = { 0x62, 0xf1, 0x7c, 0x48, 0x28, 0x05, 0x00, 0xfc, 0xff, 0xff }; ++ amd64_get_insn_details (insn.data (), &details); ++ SELF_CHECK (details.opcode_len == 2); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 4); ++ SELF_CHECK (details.modrm_offset == 5); ++ ++ /* INSN: vmovaps -0x400(%rcx),%zmm0, evex prefix. */ ++ updated_insn ++ = { 0x62, 0xf1, 0x7c, 0x48, 0x28, 0x81, 0x00, 0xfc, 0xff, 0xff }; ++ fixup_riprel (details, insn.data (), ECX_REG_NUM); ++ SELF_CHECK (insn == updated_insn); + } + + static void diff --git a/gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch new file mode 100644 index 0000000..d9037d3 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch @@ -0,0 +1,66 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-2-of-13.patch + +;; [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 2 +;; (Tom de Vries, RHEL-7329) + +I noticed that amd64_get_unused_input_int_reg uses a signed int for a bit +mask: +... + /* 1 bit for each reg */ + int used_regs_mask = 0; +... + +There's an assert: +... + gdb_assert (used_regs_mask < 256); +... +which is meant to assert on register numbers >= 8, but if for instance +sizeof (used_regs_mask) == 4 and used_regs_mask == (1 << 31), then that is not +caught because of the signedness. + +We could fix this by changing the type to unsigned int, but that only +guarantees 16 bits in the reg mask. Intel CPUs with the APX extension support +32 int registers. + +The implementation of amd64_get_unused_input_int_reg doesn't support analyzing +registers with register number >= 8 yet, but now that we're changing the type, +it seems like a good idea to anticipate this. + +Fix this by using uint32_t. + +Likewise, update the loop over the reg mask: +... + for (i = 0; i < 8; ++i) + { + if (! (used_regs_mask & (1 << i))) + return i; +... +to handle any used_regs_mask value rather than just those for +register number < 8. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1217,7 +1217,7 @@ static int + amd64_get_unused_input_int_reg (const struct amd64_insn *details) + { + /* 1 bit for each reg */ +- int used_regs_mask = 0; ++ uint32_t used_regs_mask = 0; + + /* There can be at most 3 int regs used as inputs in an insn, and we have + 7 to choose from (RAX ... RDI, sans RSP). +@@ -1268,7 +1268,7 @@ amd64_get_unused_input_int_reg (const struct amd64_insn *details) + { + int i; + +- for (i = 0; i < 8; ++i) ++ for (i = 0; i < 32; ++i) + { + if (! (used_regs_mask & (1 << i))) + return i; diff --git a/gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch new file mode 100644 index 0000000..a28e03a --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch @@ -0,0 +1,100 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-3-of-13.patch + +;; [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 3 +;; (Tom de Vries, RHEL-7329) + +While reading amd64_get_unused_input_int_reg, I noticed that it avoids picking +RSP, which has to do with how the result of the only call to it is going to be +used. + +Likewise for picking a register in the RAX ... RDI range. + +Fix this by: +- adding an allowed_regs_mask parameter to amd64_get_unused_input_int_reg, and +- properly documenting the value of the corresponding argument in fixup_riprel. + +No functional changes. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1206,31 +1206,23 @@ amd64_skip_prefixes (gdb_byte *insn) + return insn; + } + +-/* Return an integer register (other than RSP) that is unused as an input +- operand in INSN. +- In order to not require adding a rex prefix if the insn doesn't already +- have one, the result is restricted to RAX ... RDI, sans RSP. +- The register numbering of the result follows architecture ordering, +- e.g. RDI = 7. Return -1 if no register can be found. */ ++/* Return an integer register in ALLOWED_REGS_MASK that is unused as an input ++ operand in INSN. The register numbering of the result follows architecture ++ ordering, e.g. RDI = 7. Return -1 if no register can be found. */ + + static int +-amd64_get_unused_input_int_reg (const struct amd64_insn *details) ++amd64_get_unused_input_int_reg (const struct amd64_insn *details, ++ uint32_t allowed_regs_mask) + { + /* 1 bit for each reg */ + uint32_t used_regs_mask = 0; + +- /* There can be at most 3 int regs used as inputs in an insn, and we have +- 7 to choose from (RAX ... RDI, sans RSP). +- This allows us to take a conservative approach and keep things simple. +- E.g. By avoiding RAX, we don't have to specifically watch for opcodes +- that implicitly specify RAX. */ +- +- /* Avoid RAX. */ ++ /* Assume RAX is used. If not, we'd have to detect opcodes that implicitly ++ use RAX. */ + used_regs_mask |= 1 << EAX_REG_NUM; +- /* Similarly avoid RDX, implicit operand in divides. */ ++ /* Assume RDX is used. If not, we'd have to detect opcodes that implicitly ++ use RDX, like divides. */ + used_regs_mask |= 1 << EDX_REG_NUM; +- /* Avoid RSP. */ +- used_regs_mask |= 1 << ESP_REG_NUM; + + /* If the opcode is one byte long and there's no ModRM byte, + assume the opcode specifies a register. */ +@@ -1270,6 +1262,9 @@ amd64_get_unused_input_int_reg (const struct amd64_insn *details) + + for (i = 0; i < 32; ++i) + { ++ if (! (allowed_regs_mask & (1 << i))) ++ continue; ++ + if (! (used_regs_mask & (1 << i))) + return i; + } +@@ -1379,13 +1374,19 @@ fixup_riprel (struct gdbarch *gdbarch, + dsc->insn_buf.size (), from); + rip_base = from + insn_length; + +- /* We need a register to hold the address. +- Pick one not used in the insn. +- NOTE: arch_tmp_regno uses architecture ordering, e.g. RDI = 7. */ +- arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details); ++ /* We need a register to hold the address. Pick one not used in the insn. ++ In order to not require adding a rex prefix if the insn doesn't already ++ have one, the range is restricted to RAX ... RDI, without RSP. ++ We avoid RSP, because when patched into in the modrm byte, it doesn't ++ indicate the use of the register, but instead the use of a SIB byte. */ ++ uint32_t allowed_regs_mask = 0xff & ~(1 << ESP_REG_NUM); ++ arch_tmp_regno ++ = amd64_get_unused_input_int_reg (insn_details, allowed_regs_mask); + if (arch_tmp_regno == -1) + return false; + ++ /* Convert arch_tmp_regno, which uses architecture ordering (e.g. RDI = 7), ++ to GDB regnum. */ + tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno); + + /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ diff --git a/gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch new file mode 100644 index 0000000..37159e1 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch @@ -0,0 +1,59 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-4-of-13.patch + +;; [gdb/tdep] Factor out amd64_get_used_input_int_regs +;; (Tom de Vries, RHEL-7329) + +The function amd64_get_unused_input_int_reg consists of two parts: +- finding the used int registers in an insn, and +- picking an unused int register. + +Factor out the first part as new function amd64_get_used_input_int_regs. + +No functional changes. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1206,13 +1206,11 @@ amd64_skip_prefixes (gdb_byte *insn) + return insn; + } + +-/* Return an integer register in ALLOWED_REGS_MASK that is unused as an input +- operand in INSN. The register numbering of the result follows architecture +- ordering, e.g. RDI = 7. Return -1 if no register can be found. */ ++/* Return a register mask for the integer registers that are used as an input ++ operand in INSN. */ + +-static int +-amd64_get_unused_input_int_reg (const struct amd64_insn *details, +- uint32_t allowed_regs_mask) ++static uint32_t ++amd64_get_used_input_int_regs (const struct amd64_insn *details) + { + /* 1 bit for each reg */ + uint32_t used_regs_mask = 0; +@@ -1255,6 +1253,19 @@ amd64_get_unused_input_int_reg (const struct amd64_insn *details, + } + + gdb_assert (used_regs_mask < 256); ++ return used_regs_mask; ++} ++ ++/* Return an integer register in ALLOWED_REGS_MASK that is unused as an input ++ operand in INSN. The register numbering of the result follows architecture ++ ordering, e.g. RDI = 7. Return -1 if no register can be found. */ ++ ++static int ++amd64_get_unused_input_int_reg (const struct amd64_insn *details, ++ uint32_t allowed_regs_mask) ++{ ++ /* 1 bit for each reg */ ++ uint32_t used_regs_mask = amd64_get_used_input_int_regs (details); + + /* Finally, find a free reg. */ + { diff --git a/gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch new file mode 100644 index 0000000..10f8046 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch @@ -0,0 +1,137 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-5-of-13.patch + +;; [gdb/tdep] Add amd64-insn-decode selftest +;; (Tom de Vries, RHEL-7329) + +Add a selftest that checks the results of amd64_get_insn_details and related +functions for two basic instructions. + +Add a parameter assumptions to amd64_get_used_input_int_regs, to make sure +that this selftest: +... + /* INSN: add %eax,(%rcx). */ + ... + SELF_CHECK (amd64_get_used_input_int_regs (&details, false) + == ((1 << EAX_REG_NUM) | (1 << ECX_REG_NUM))); +... +passes because it found the "%eax" in the insn, rather than passing because of +this assumption: +... + /* Assume RAX is used. If not, we'd have to detect opcodes that implicitly + use RAX. */ + used_regs_mask |= 1 << EAX_REG_NUM; +... + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -50,6 +50,7 @@ + #include "osabi.h" + #include "x86-tdep.h" + #include "amd64-ravenscar-thread.h" ++#include "gdbsupport/selftest.h" + + /* Note that the AMD64 architecture was previously known as x86-64. + The latter is (forever) engraved into the canonical system name as +@@ -1207,20 +1208,25 @@ amd64_skip_prefixes (gdb_byte *insn) + } + + /* Return a register mask for the integer registers that are used as an input +- operand in INSN. */ ++ operand in INSN. If !ASSUMPTIONS, only return the registers we actually ++ found, for the benefit of self tests. */ + + static uint32_t +-amd64_get_used_input_int_regs (const struct amd64_insn *details) ++amd64_get_used_input_int_regs (const struct amd64_insn *details, ++ bool assumptions = true) + { + /* 1 bit for each reg */ + uint32_t used_regs_mask = 0; + +- /* Assume RAX is used. If not, we'd have to detect opcodes that implicitly +- use RAX. */ +- used_regs_mask |= 1 << EAX_REG_NUM; +- /* Assume RDX is used. If not, we'd have to detect opcodes that implicitly +- use RDX, like divides. */ +- used_regs_mask |= 1 << EDX_REG_NUM; ++ if (assumptions) ++ { ++ /* Assume RAX is used. If not, we'd have to detect opcodes that implicitly ++ use RAX. */ ++ used_regs_mask |= 1 << EAX_REG_NUM; ++ /* Assume RDX is used. If not, we'd have to detect opcodes that implicitly ++ use RDX, like divides. */ ++ used_regs_mask |= 1 << EDX_REG_NUM; ++ } + + /* If the opcode is one byte long and there's no ModRM byte, + assume the opcode specifies a register. */ +@@ -3395,6 +3401,51 @@ amd64_target_description (uint64_t xcr0, bool segments) + return *tdesc; + } + ++#if GDB_SELF_TEST ++ ++namespace selftests { ++ ++/* Test amd64_get_insn_details. */ ++ ++static void ++test_amd64_get_insn_details (void) ++{ ++ struct amd64_insn details; ++ gdb::byte_vector insn; ++ ++ /* INSN: add %eax,(%rcx). */ ++ insn = { 0x01, 0x01 }; ++ amd64_get_insn_details (insn.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.opcode_offset == 0); ++ SELF_CHECK (details.modrm_offset == 1); ++ SELF_CHECK (amd64_get_used_input_int_regs (&details, false) ++ == ((1 << EAX_REG_NUM) | (1 << ECX_REG_NUM))); ++ SELF_CHECK (rip_relative_offset (&details) == 0); ++ ++ /* INSN: push %rax. This exercises the "opcode specifies register" case in ++ amd64_get_used_input_int_regs. */ ++ insn = { 0x50 }; ++ amd64_get_insn_details (insn.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == -1); ++ SELF_CHECK (details.opcode_offset == 0); ++ SELF_CHECK (details.modrm_offset == -1); ++ SELF_CHECK (amd64_get_used_input_int_regs (&details, false) ++ == ((1 << EAX_REG_NUM))); ++ SELF_CHECK (rip_relative_offset (&details) == 0); ++} ++ ++static void ++amd64_insn_decode (void) ++{ ++ test_amd64_get_insn_details (); ++} ++ ++} // namespace selftests ++#endif /* GDB_SELF_TEST */ ++ + void _initialize_amd64_tdep (); + void + _initialize_amd64_tdep () +@@ -3403,6 +3454,10 @@ _initialize_amd64_tdep () + amd64_none_init_abi); + gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32, GDB_OSABI_NONE, + amd64_x32_none_init_abi); ++#if GDB_SELF_TEST ++ selftests::register_test ("amd64-insn-decode", ++ selftests::amd64_insn_decode); ++#endif + } + + diff --git a/gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch new file mode 100644 index 0000000..9bb4b87 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch @@ -0,0 +1,55 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-6-of-13.patch + +;; [gdb/tdep] Factor out rip_relative_p +;; (Tom de Vries, RHEL-7329) + +Factor out rip_relative_p, and rewrite it to use MODRM_MOD_FIELD and +MODRM_RM_FIELD. + +No functional changes. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1207,6 +1207,18 @@ amd64_skip_prefixes (gdb_byte *insn) + return insn; + } + ++/* Return true if the MODRM byte of an insn indicates that the insn is ++ rip-relative. */ ++ ++static bool ++rip_relative_p (gdb_byte modrm) ++{ ++ gdb_byte mod = MODRM_MOD_FIELD (modrm); ++ gdb_byte rm = MODRM_RM_FIELD (modrm); ++ ++ return mod == 0 && rm == 0x05; ++} ++ + /* Return a register mask for the integer registers that are used as an input + operand in INSN. If !ASSUMPTIONS, only return the registers we actually + found, for the benefit of self tests. */ +@@ -1455,7 +1467,7 @@ fixup_displaced_copy (struct gdbarch *gdbarch, + { + gdb_byte modrm = details->raw_insn[details->modrm_offset]; + +- if ((modrm & 0xc7) == 0x05) ++ if (rip_relative_p (modrm)) + { + /* The insn uses rip-relative addressing. + Deal with it. */ +@@ -1789,7 +1801,7 @@ rip_relative_offset (struct amd64_insn *insn) + { + gdb_byte modrm = insn->raw_insn[insn->modrm_offset]; + +- if ((modrm & 0xc7) == 0x05) ++ if (rip_relative_p (modrm)) + { + /* The displacement is found right after the ModRM byte. */ + return insn->modrm_offset + 1; diff --git a/gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch new file mode 100644 index 0000000..9a04e2e --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch @@ -0,0 +1,55 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-7-of-13.patch + +;; [gdb/tdep] Fix rip-relative insn handling in amd64_get_used_input_int_reg +;; (Tom de Vries, RHEL-7329) + +I wanted to add a unit test for an an rip-relative amd64 insn, so I did: +... +$ gcc -fPIE hello.c +... +and used an rip-relative insn from main: +... + 4005db: 48 8d 3d 1e 00 00 00 lea 0x1e(%rip),%rdi +... + +While writing the unit test, I found that amd64_get_used_input_int_reg returns +rbp as input register. + +Fix this by using rip_relative_p in amd64_get_used_input_int_reg to handle +this case. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1264,7 +1264,7 @@ amd64_get_used_input_int_regs (const struct amd64_insn *details, + used_regs_mask |= 1 << base; + used_regs_mask |= 1 << idx; + } +- else ++ else if (!rip_relative_p (modrm)) + { + used_regs_mask |= 1 << rm; + } +@@ -3447,6 +3447,17 @@ test_amd64_get_insn_details (void) + SELF_CHECK (amd64_get_used_input_int_regs (&details, false) + == ((1 << EAX_REG_NUM))); + SELF_CHECK (rip_relative_offset (&details) == 0); ++ ++ /* INSN: lea 0x1e(%rip),%rdi, rex prefix. */ ++ insn = { 0x48, 0x8d, 0x3d, 0x1e, 0x00, 0x00, 0x00 }; ++ amd64_get_insn_details (insn.data (), &details); ++ SELF_CHECK (details.opcode_len == 1); ++ SELF_CHECK (details.enc_prefix_offset == 0); ++ SELF_CHECK (details.opcode_offset == 1); ++ SELF_CHECK (details.modrm_offset == 2); ++ SELF_CHECK (amd64_get_used_input_int_regs (&details, false) ++ == (1 << EDI_REG_NUM)); ++ SELF_CHECK (rip_relative_offset (&details) == 3); + } + + static void diff --git a/gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch new file mode 100644 index 0000000..4b75b0e --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch @@ -0,0 +1,112 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-8-of-13.patch + +;; [gdb/tdep] Factor out part of fixup_riprel +;; (Tom de Vries, RHEL-7329) + +Factor out the part of fixup_riprel that patches the insn, and use it in a +unit test. + +Tested on x86_64-linux. + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -1377,6 +1377,36 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details) + } + } + ++/* Convert a %rip-relative INSN to use BASEREG+disp addressing, leaving ++ displacement unchanged. */ ++ ++static void ++fixup_riprel (const struct amd64_insn &details, gdb_byte *insn, ++ int basereg) ++{ ++ /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ ++ static constexpr gdb_byte VEX3_NOT_B = 0x20; ++ ++ /* REX.B should be unset (VEX.!B set) as we were using rip-relative ++ addressing, but ensure it's unset (set for VEX) anyway, tmp_regno ++ is not r8-r15. */ ++ if (details.enc_prefix_offset != -1) ++ { ++ gdb_byte *pfx = &insn[details.enc_prefix_offset]; ++ if (rex_prefix_p (pfx[0])) ++ pfx[0] &= ~REX_B; ++ else if (vex3_prefix_p (pfx[0])) ++ pfx[1] |= VEX3_NOT_B; ++ else ++ gdb_assert_not_reached ("unhandled prefix"); ++ } ++ ++ int modrm_offset = details.modrm_offset; ++ /* Convert the ModRM field to be base+disp. */ ++ insn[modrm_offset] &= ~0xc7; ++ insn[modrm_offset] |= 0x80 + basereg; ++} ++ + /* Update %rip-relative addressing in INSN. + + %rip-relative addressing only uses a 32-bit displacement. +@@ -1392,7 +1422,6 @@ fixup_riprel (struct gdbarch *gdbarch, + CORE_ADDR from, CORE_ADDR to, struct regcache *regs) + { + const struct amd64_insn *insn_details = &dsc->insn_details; +- int modrm_offset = insn_details->modrm_offset; + CORE_ADDR rip_base; + int insn_length; + int arch_tmp_regno, tmp_regno; +@@ -1414,36 +1443,17 @@ fixup_riprel (struct gdbarch *gdbarch, + if (arch_tmp_regno == -1) + return false; + ++ fixup_riprel (dsc->insn_details, dsc->insn_buf.data (), arch_tmp_regno); ++ + /* Convert arch_tmp_regno, which uses architecture ordering (e.g. RDI = 7), + to GDB regnum. */ + tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno); + +- /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1). */ +- static constexpr gdb_byte VEX3_NOT_B = 0x20; +- +- /* REX.B should be unset (VEX.!B set) as we were using rip-relative +- addressing, but ensure it's unset (set for VEX) anyway, tmp_regno +- is not r8-r15. */ +- if (insn_details->enc_prefix_offset != -1) +- { +- gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset]; +- if (rex_prefix_p (pfx[0])) +- pfx[0] &= ~REX_B; +- else if (vex3_prefix_p (pfx[0])) +- pfx[1] |= VEX3_NOT_B; +- else +- gdb_assert_not_reached ("unhandled prefix"); +- } +- + regcache_cooked_read_unsigned (regs, tmp_regno, &orig_value); + dsc->tmp_regno = tmp_regno; + dsc->tmp_save = orig_value; + dsc->tmp_used = 1; + +- /* Convert the ModRM field to be base+disp. */ +- dsc->insn_buf[modrm_offset] &= ~0xc7; +- dsc->insn_buf[modrm_offset] |= 0x80 + arch_tmp_regno; +- + regcache_cooked_write_unsigned (regs, tmp_regno, rip_base); + + displaced_debug_printf ("%%rip-relative addressing used."); +@@ -3458,6 +3468,11 @@ test_amd64_get_insn_details (void) + SELF_CHECK (amd64_get_used_input_int_regs (&details, false) + == (1 << EDI_REG_NUM)); + SELF_CHECK (rip_relative_offset (&details) == 3); ++ ++ /* INSN: lea 0x1e(%ecx),%rdi, rex prefix. */ ++ gdb::byte_vector updated_insn = { 0x48, 0x8d, 0xb9, 0x1e, 0x00, 0x00, 0x00 }; ++ fixup_riprel (details, insn.data (), ECX_REG_NUM); ++ SELF_CHECK (insn == updated_insn); + } + + static void diff --git a/gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch b/gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch new file mode 100644 index 0000000..3c61905 --- /dev/null +++ b/gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch @@ -0,0 +1,78 @@ +From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 +From: Tom de Vries +Date: Fri, 7 Mar 2025 09:25:33 +0100 +Subject: gdb-rhel-7329-vandps-clobbers-registers-9-of-13.patch + +;; [gdb/tdep] Add vex2_to_vex3 +;; (Tom de Vries, RHEL-7329) + +I noticed here [1] that the vex2 prefix is essentially a special case of the +vex3 prefix, meaning it's possible to rewrite any insn with a vex2 prefix into +an equivalent one with a vex3 prefix. + +Add function vex2_to_vex3 that does precisely that, in the selftests +namespace. + +Add a selftest that exercises this function. + +Tested on x86_64-linux. + +[1] https://en.wikipedia.org/wiki/VEX_prefix + +diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c +--- a/gdb/amd64-tdep.c ++++ b/gdb/amd64-tdep.c +@@ -3427,6 +3427,45 @@ amd64_target_description (uint64_t xcr0, bool segments) + + namespace selftests { + ++/* Recode a vex2 instruction into a vex3 instruction. */ ++ ++static void ++vex2_to_vex3 (gdb::byte_vector &vex2, gdb::byte_vector &vex3) ++{ ++ gdb_assert (vex2.size () >= 2); ++ gdb_assert (vex2[0] == 0xc5); ++ ++ unsigned char r = vex2[1] >> 7; ++ unsigned char b = 0x1; ++ unsigned char x = 0x1; ++ unsigned char m = 0x1; ++ unsigned char w = 0x0; ++ ++ vex3.resize (3); ++ vex3[0] = 0xc4; ++ vex3[1] = (r << 7) | (x << 6) | (b << 5) | m; ++ vex3[2] = (vex2[1] & ~0x80) | (w << 7); ++ ++ std::copy (vex2.begin () + 2, vex2.end (), ++ std::back_inserter (vex3)); ++} ++ ++/* Test vex2 to vex3. */ ++ ++static void ++test_vex2_to_vex3 (void) ++{ ++ /* INSN: vzeroall, vex2 prefix. */ ++ gdb::byte_vector vex2 = { 0xc5, 0xfc, 0x77 }; ++ ++ gdb::byte_vector vex3; ++ vex2_to_vex3 (vex2, vex3); ++ ++ /* INSN: vzeroall, vex3 prefix. */ ++ gdb::byte_vector vex3_ref = { 0xc4, 0xe1, 0x7c, 0x77 }; ++ SELF_CHECK (vex3 == vex3_ref); ++} ++ + /* Test amd64_get_insn_details. */ + + static void +@@ -3478,6 +3517,7 @@ test_amd64_get_insn_details (void) + static void + amd64_insn_decode (void) + { ++ test_vex2_to_vex3 (); + test_amd64_get_insn_details (); + } + diff --git a/gdb.spec b/gdb.spec index 7852495..30dd3bc 100644 --- a/gdb.spec +++ b/gdb.spec @@ -45,7 +45,7 @@ Version: 16.3 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 1%{?dist} +Release: 2%{?dist} License: GPL-3.0-or-later AND BSD-3-Clause AND FSFAP AND LGPL-2.1-or-later AND GPL-2.0-or-later AND LGPL-2.0-or-later AND LicenseRef-Fedora-Public-Domain AND GFDL-1.3-or-later AND LGPL-2.0-or-later WITH GCC-exception-2.0 AND GPL-3.0-or-later WITH GCC-exception-3.1 AND GPL-2.0-or-later WITH GNU-compiler-exception AND MIT # Do not provide URL for snapshots as the file lasts there only for 2 days. @@ -928,6 +928,10 @@ fi # endif scl %changelog +* Thu May 29 2025 Guinevere Larsen - 16.3-2.el9 +- Backport amd64 decoding fixes + Resolves: RHEL-93850 + * Wed May 28 2025 Guinevere Larsen - 16.3-1.el9 - Rebase RHEL 9's GDB from 14.2 to 16.3 Resolves: RHEL-91381