From: Andrew Burgess Date: Mon, 1 Sep 2025 10:25:26 +0000 (+0100) Subject: gdb/i386/linux: fix possible register number conflict X-Git-Tag: binutils-2_46~1545 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c87df013cbc2c5072f1ef95626ed493e2b54506c;p=thirdparty%2Fbinutils-gdb.git gdb/i386/linux: fix possible register number conflict I noticed something that seemed really strange with the i386 register numbering. In i386-linux-tdep.h we setup I386_LINUX_ORIG_EAX_REGNUM based on I386_PKRU_REGNUM. However, in i386-tdep.h, enum i386_regnum ends like this: enum i386_regnum { ... I386_ZMM7H_REGNUM = I386_ZMM0H_REGNUM + 7, I386_PKRU_REGNUM, I386_PL3_SSP_REGNUM, I386_FSBASE_REGNUM, I386_GSBASE_REGNUM }; So I386_LINUX_ORIG_EAX_REGNUM will have the same value as I386_PL3_SSP_REGNUM. The I386_PL3_SSP_REGNUM was added in commit: commit 63b862be762e1e6e7ce667c6b4a1a3dd79939bf4 AuthorDate: Fri Mar 29 16:38:50 2019 +0100 CommitDate: Fri Aug 29 17:02:09 2025 +0000 gdb, gdbserver: Add support of Intel shadow stack pointer register. And before that, I386_FSBASE_REGNUM and I386_GSBASE_REGNUM were added in commit: commit 1163a4b7a38a79ebd153dc5ee76ce93877d21dbd AuthorDate: Tue Mar 12 13:39:02 2019 -0700 CommitDate: Tue Mar 12 13:39:02 2019 -0700 Support the fs_base and gs_base registers on i386. So the SSP overlap is new, but the fs/gs base overlap has existed for years, so why did it not cause any problems? I think the explanation is that on i386, the fs/gs base are only used for FreeBSD, all the calls to i386_target_description that pass true for the segments argument are from fbsd files. As a result, its fine if there's numbering overlap between these i386 registers and some Linux specific i386 registers. OK, but what about the new SSP (shadow stack pointer) register? I think in this case we would see problems, if the shadow stack was supported for i386. Here's what the docs say: The ‘org.gnu.gdb.i386.pl3_ssp’ feature is optional. It should describe the user mode register ‘pl3_ssp’ which has 64 bits on amd64, 32 bits on amd64 with 32-bit pointer size (X32) and 32 bits on i386. Following the restriction of the Linux kernel, only GDB for amd64 targets makes use of this feature for now. And indeed, if we look for callers of x86_supply_ssp, which supplies the shadow stack pointer register, this is only called from amd64 specific code, either the native register fetching, or the core file loading. There's no calls from i386 code. And so, again, we have register number overlap, but we avoid any issues by not making use of these registers for i386 linux. Here's my question: Is this super clever design aimed at saving 12 bytes (3 * 4-byte registers) of space in the i386 regcache? Or is this an accident where we happen to have gotten lucky? If it's the first, then I really think there should be some comments explaining what's going on. If it's the second, then maybe we should fix this before it trips us up? This commit takes the second approach by doing the following: 1. In i386-tdep.h move all the *_NUM_REGS constants to be members of 'enum i386_regnum'. The I386_NUM_REGS value can be automatically calculated based off the (current) last enum entry, and the other *_NUM_REGS constants are calculated just as they previously were, but are moved to keep them all together. 2. In i386-linux-tdep.h, I386_LINUX_ORIG_EAX_REGNUM and I386_LINUX_NUM_REGS are moved into a new enum i386_linux_regnum, the name of which is inspired by i386_regnum with the addition of the linux tag. The first entry in this new enum starts from I386_NUM_REGS rather than I386_PKRU_REGNUM. The I386_LINUX_NUM_REGS will be calculated automatically by the compiler. 3. In amd64-linux-nat.c, I extend amd64_linux_gregset32_reg_offset so that it now has entries for the 3 registers that are no longer aliasing, this stops an assert from the end of the file triggering: gdb_assert (ARRAY_SIZE (amd64_linux_gregset32_reg_offset) == amd64_native_gregset32_num_regs); As I386_LINUX_NUM_REGS has now increased by 3. 4. Given (3) I wondered why there was no assert being triggered from the i386 code as i386_linux_gregset_reg_offset, in i386-linux-tdep.c is clearly also wrong now. So, In i386-linux-tdep.c I've added a new assertion at the end of the file. And then I've fixed i386_linux_gregset_reg_offset by adding the 3 new registers. With these changes made I believe that the register number for the $orig_eax register on i386 GNU/Linux targets should no longer be aliasing with the SSP register. For the reasons given above, I don't think this fixes any actual bugs, it's more just a, lets not have unnecessary, and undocumented, register number aliasing. This change is visible using 'maint print registers', check out the register number of $orig_eax before and after, it should now be +3 from where it was (changed from 72 to 75). I did worry briefly about gdbservers that might not support XML target descriptions and instead rely on a fixed GDB register numbering. Though, if I'm honest, I have very little sympathy for such gdbservers these days. Still, they could, potentially be tripped up by this change. However, this is not the first time in recent years that the value of I386_LINUX_ORIG_EAX_REGNUM has changed. This commit also adjusted the register number: commit 51547df62c155231530ca502c485659f3d2b66cb Date: Wed Feb 1 12:22:27 2017 +0100 Add support for Intel PKRU register to GDB and GDBserver. And I'm not aware of any bug reports that came from this, we certainly didn't feel the need to adjust the register number back again. So I'm guessing that this renumbering will also go without issue. Other than that, there should be no user visible changes after this commit. Reviewed-By: Christina Schimpe --- diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c index e35527de2a6..4b23fd9de53 100644 --- a/gdb/amd64-linux-nat.c +++ b/gdb/amd64-linux-nat.c @@ -87,6 +87,8 @@ static int amd64_linux_gregset32_reg_offset[] = -1, -1, -1, -1, -1, -1, -1, -1, /* k0 ... k7 (AVX512) */ -1, -1, -1, -1, -1, -1, -1, -1, /* zmm0 ... zmm7 (AVX512) */ -1, /* PKEYS register PKRU */ + -1, /* SSP register. */ + -1, -1, /* fs/gs base registers. */ ORIG_RAX * 8 /* "orig_eax" */ }; diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 3360e96f86c..34e13912098 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -1044,6 +1044,8 @@ int i386_linux_gregset_reg_offset[] = -1, -1, -1, -1, -1, -1, -1, -1, /* k0 ... k7 (AVX512) */ -1, -1, -1, -1, -1, -1, -1, -1, /* zmm0 ... zmm7 (AVX512) */ -1, /* PKRU register */ + -1, /* SSP register. */ + -1, -1, /* fs/gs base registers. */ 11 * 4, /* "orig_eax" */ }; @@ -1491,6 +1493,9 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) INIT_GDB_FILE (i386_linux_tdep) { + gdb_assert (ARRAY_SIZE (i386_linux_gregset_reg_offset) + == I386_LINUX_NUM_REGS); + gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_LINUX, i386_linux_init_abi); } diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h index 3a0e056413f..8a1a2447776 100644 --- a/gdb/i386-linux-tdep.h +++ b/gdb/i386-linux-tdep.h @@ -21,20 +21,29 @@ #define GDB_I386_LINUX_TDEP_H #include "gdbsupport/x86-xstate.h" +#include "i386-tdep.h" -/* The Linux kernel pretends there is an additional "orig_eax" - register. Since GDB needs access to that register to be able to - properly restart system calls when necessary (see - i386-linux-tdep.c) we need our own versions of a number of - functions that deal with GDB's register cache. */ +/* Additional register numbers for i386 Linux, these are in addition to + the register numbers found in 'enum i386_regnum', see i386-tdep.h. */ -/* Register number for the "orig_eax" pseudo-register. If this - pseudo-register contains a value >= 0 it is interpreted as the - system call number that the kernel is supposed to restart. */ -#define I386_LINUX_ORIG_EAX_REGNUM (I386_PKRU_REGNUM + 1) +enum i386_linux_regnum +{ + /* STOP! The values in this enum are numbered after the values in the + enum i386_regnum. New entries should be placed after the ORIG_EAX + entry. */ -/* Total number of registers for GNU/Linux. */ -#define I386_LINUX_NUM_REGS (I386_LINUX_ORIG_EAX_REGNUM + 1) + /* Register number for the "orig_eax" pseudo-register. GDB needs access + to this register to be able to properly restart system calls when + necessary (see i386-linux-tdep.c). If this pseudo-register contains a + value >= 0 it is interpreted as the system call number that the kernel + is supposed to restart. */ + I386_LINUX_ORIG_EAX_REGNUM = I386_NUM_REGS, + + /* Total number of registers for GNU/Linux. */ + I386_LINUX_NUM_REGS + + /* STOP! Add new entries before I386_LINUX_NUM_REGS. */ +}; /* Read the XSAVE extended state xcr0 value from the ABFD core file. If it appears to be valid, return it and fill LAYOUT with values diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h index 84f1bbde007..cfe0d7383d6 100644 --- a/gdb/i386-tdep.h +++ b/gdb/i386-tdep.h @@ -303,7 +303,14 @@ enum i386_regnum I386_PKRU_REGNUM, I386_PL3_SSP_REGNUM, I386_FSBASE_REGNUM, - I386_GSBASE_REGNUM + I386_GSBASE_REGNUM, + + I386_NUM_REGS, /* Calculated from last *_REGNUM entry. */ + I386_SSE_NUM_REGS = I386_MXCSR_REGNUM + 1, + I386_AVX_NUM_REGS = I386_YMM7H_REGNUM + 1, + I386_AVX512_NUM_REGS = I386_ZMM7H_REGNUM + 1, + I386_PKEYS_NUM_REGS = I386_PKRU_REGNUM + 1 + /* STOP! New *_REGNUM entries should be added before I386_NUM_REGS. */ }; /* Register numbers of RECORD_REGMAP. */ @@ -340,12 +347,6 @@ enum record_i386_regnum #define I386_NUM_GREGS 16 #define I386_NUM_XREGS 9 -#define I386_SSE_NUM_REGS (I386_MXCSR_REGNUM + 1) -#define I386_AVX_NUM_REGS (I386_YMM7H_REGNUM + 1) -#define I386_AVX512_NUM_REGS (I386_ZMM7H_REGNUM + 1) -#define I386_PKEYS_NUM_REGS (I386_PKRU_REGNUM + 1) -#define I386_NUM_REGS (I386_GSBASE_REGNUM + 1) - /* Size of the largest register. */ #define I386_MAX_REGISTER_SIZE 64