From 0907b3ea044e1593b1a4ff6c0dda6ca9d3643a0b Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Fri, 7 Mar 2025 09:25:33 +0100 Subject: [PATCH] [gdb/tdep] Refactor amd64_get_unused_input_int_reg, part 1 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. --- gdb/amd64-tdep.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 3c75f2fa22d..542455e4cd7 100644 --- 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); -- 2.39.5