Re: [PATCHv3] gdb: include NT_I386_TLS note in generated core files

[Eli Zaretskii <eliz@gnu.org>]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 14 Oct 2025 14:54:28 +0100
> 
> In v3:
> 
>   - Fixed doc issue that Eli pointed out.
> 
>   - Rebased to current HEAD, no merged conflicts.
> 
> In v2:
> 
>   - Rebased to current HEAD, resolved merge conflicts related to
>     recent i386 register changes.
> 
>   - Updated test to take account of recent clean_restart changes.
> 
>   - Retested.
> 
> ---
> 
> This commit extends GDB for x86/Linux to include the NT_I386_TLS note
> in generated core files (i.e. created with `generate-core-file` or
> `gcore` command).  This note contains the 3 per-thread TLS related
> GDT (global descriptor table) entries, and is present for i386
> binaries, or those compiled on x86-64 with -m32.
> 
> The approach I have taken to achieve this, is to make the 3 GDT
> entries available within 3 new registers.  I added these registers to
> the org.gnu.gdb.i386.linux target description feature, as this feature
> seemed perfectly named.  As the new registers are optional I don't see
> any harm in extending this existing feature.  I did consider adding a
> new feature with `tls` in the name, but this seemed excessive given
> the existing feature.
> 
> Which GDT entries are used for TLS varies between i386 and x86-64
> running in 32-bit mode.  As such the registers are named with suffixes
> 0, 1, and 2, and it is left to GDB or gdbserver, to find the correct
> GDT entries (based on the precise target) and place the contents into
> these registers.
> 
> With this done, adding the relevant regset is sufficient to get the
> tls contents emitted as a core file note.  Support for emitting the
> note into the generated core file relies on some BFD changes which
> were made in an earlier commit:
> 
>   commit ea6ec00ff4520895735e4913cb90c933c7296f04
>   Date:   Fri Jul 25 19:51:58 2025 +0100
> 
>       bfd: support for NT_386_TLS notes
> 
> The three new registers are readable and writable.  Writing to one of
> the new registers will update the relevant kernel GDT entry.
> 
> Each TLS GDT is represented by a 'struct user_desc' (see 'man 2
> get_thread_area' for details), the first 4 bytes of each 'user_desc'
> is the 'entry_number' field, this is the index of the GDT within the
> kernel, and cannot be modified.  Attempts to write to this region of
> the register will be ignored, but will not give an error.
> 
> I did consider not including this part of the user_desc within the
> register value, but this becomes difficult when we consider remote
> targets, GDB would then need to figure out what these indexes were so
> that the core file note could be generated.  Sure, we probably could
> figure the correct index values out, but I figure, why bother, we can
> just pass them through in the register and know for certain that we
> have the correct values.
> 
> For testing, there's a new test that covers the basic functionality,
> including read/write access to the new registers, and checking that
> the NT_386_TLS note is added to the core file, and that the note
> contents can be read by GDB.
> 
> I also manually tested opening a core file generated from an old
> GDB (so no NT_386_TLS notes) using a GDB with this patch.  This works
> fine, the new tls registers are not created as the NT_GDB_TDESC
> note (the target description) doesn't include the new registers.
> 
> Out of interest I also patched an old version of GDB to avoid creating
> the NT_GDB_TDESC, and created a core file.  This core file contained
> neither the NT_386_TLS nor NT_GDB_TDESC.  When opening this core file
> with a patched GDB, the new registers do show up, but their contents
> are given as <unavailable>, which is exactly what we'd expect, GDB
> builds a target description based on the architecture, the
> architecture says these registers should exist, but they are missing
> from the core file, hence, <unavailable>.
> 
> I also tested using a patched GDB with an old version of gdbserver,
> the new registers don't show up as the old gdbserver doesn't send them
> in its target description.  And a core file created using the gcore
> command in such a setup leaves no NT_386_TLS notes added, which is
> what we'd expect.
> 
> And I also tested a new gdbserver running with an old version of GDB.
> As the new tls registers are now mentioned in the target description,
> then obviously, the old GDB does see the registers, and present them
> to the user, however GDB doesn't know how to use these registers to
> create a NT_386_TLS, so that note isn't added to any core files.
> Also, while a new GDB places the tls registers into the 'system'
> group, an old GDB doesn't do this, so the registers end up in the
> 'general' group by default.  This means they show up within 'info
> registers' output.  This isn't ideal, but there's not much that can be
> done about this.
> 
> Overall, I feel the combinations of old and new tools has been tested,
> and the behaviours are what we'd want or expect.
> 
> I'm tagging this commit with PR gdb/15591, even though this patch
> isn't directly related.  That bug is for improving GDB's testing of
> TLS support in core files.  The test in this commit does do some very
> simple reading of a TLS variable, but there's only two threads, and
> one TLS variable, so it's not extensive.  Additionally, the test in
> this commit is x86 only, so this should not be considered a full
> resolution to that bug.  But still, it's something.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15591
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/NEWS                                      |   4 +
>  gdb/amd64-linux-nat.c                         |  26 +-
>  gdb/doc/gdb.texinfo                           |  11 +-
>  gdb/features/i386/32bit-linux.c               |   7 +
>  gdb/features/i386/32bit-linux.xml             |   5 +
>  gdb/i386-linux-nat.c                          |  21 ++
>  gdb/i386-linux-tdep.c                         | 171 +++++++++-
>  gdb/i386-linux-tdep.h                         |  15 +
>  gdb/i386-tdep.h                               |   5 +
>  gdb/nat/amd64-linux.h                         |  29 ++
>  gdb/nat/i386-linux.h                          |  10 +
>  gdb/nat/x86-linux.c                           |  44 +++
>  gdb/nat/x86-linux.h                           |  15 +
>  gdb/testsuite/gdb.arch/i386-linux-tls-regs.c  |  74 +++++
>  .../gdb.arch/i386-linux-tls-regs.exp          | 314 ++++++++++++++++++
>  gdb/x86-linux-nat.c                           |  67 ++++
>  gdb/x86-linux-nat.h                           |  17 +
>  gdbserver/linux-x86-low.cc                    |  99 ++++++
>  18 files changed, 928 insertions(+), 6 deletions(-)
>  create mode 100644 gdb/nat/amd64-linux.h
>  create mode 100644 gdb/testsuite/gdb.arch/i386-linux-tls-regs.c
>  create mode 100644 gdb/testsuite/gdb.arch/i386-linux-tls-regs.exp

Thanks, the documentation parts are okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>