Re: [PATCH 11/11] Add TLS NEWS entry and document 'set force-internal-tls-address-lookup' command

[Eli Zaretskii <eliz@gnu.org>]

> From: Kevin Buettner <kevinb@redhat.com>
> Cc: Kevin Buettner <kevinb@redhat.com>
> Date: Wed,  9 Oct 2024 19:16:14 -0700
> 
> ---
>  gdb/NEWS            | 20 ++++++++++++++++++++
>  gdb/doc/gdb.texinfo | 17 +++++++++++++++++
>  2 files changed, 37 insertions(+)

Thanks.

A general comment: I think the documentation changes should include
the explanation of the "TLS" acronym and its meaning in the context of
debugging.  Without saying that much, this piece of documentation
makes very little sense, if at all.

Another aspect that is missing is how the new commands affect
user-level features in GDB.  What does it mean to "force TLS lookup",
and how does it matter whether the TLS look up is internal or via
libthread_db, when a program uses thread-local storage?  More
generally, what is "TLS address look up" and how it affects debugging
multithreaded programs?  Note that the current text in the manual
mentions libthread_db as a "helper library" for debugging
multithreaded programs, but doesn't go into details of what that
"help" includes, leaving that vague and unexplained.  This addition
builds on that description, but it talks about a very specific aspect
of supporting threads, and does that without explaining its role.  The
previous text about libthread_db doesn't help in any way, so we must
add some details here, IMO.

Some more specific comments follow:

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 42668cbc057..dcb24eb5248 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -63,6 +63,26 @@
>       the return value from the latest "stepOut" command, when
>       appropriate.
>  
> +* GDB-internal TLS support
> +
> +  ** Linux targets for the x86_64, aarch64, ppc64, s390x, and riscv
> +     architectures now have GDB-internal support for TLS address
> +     lookup in addition to that traditionally provided by the
> +     libthread_db library.  This internal support works for programs
> +     linked against either the GLIBC or MUSL C libraries.  For
> +     programs linked against MUSL, this new internal support provides
> +     new debug functionality, allowing access to TLS variables, due to
> +     the fact that MUSL does not implement the libthread_db library. 
> +     Internal TLS support is also useful in cross-debugging
> +     situations, debugging statically linked binaries, and debugging
> +     programs linked against GLIBC 2.33 and earlier, but which are not
> +     linked against libpthread.
> +
> +  ** The command 'set force-internal-tls-address-lookup on' may be
> +     used to force the internal TLS lookup mechanisms to be used.
> +     Otherwise, TLS lookup via libthread_db will still be preferred,
> +     when available.

Do we support TLS on any platform but Linux?

> +@kindex set force-internal-tls-address-lookup
> +@kindex show force-internal-tls-address-lookup
> +@cindex internal TLS lookup
> +Turns on or off forced use of @value{GDBN}-internal TLS (Thread Local
> +Storage) address lookup code.  Use @code{on} to enable and @code{0} to
> +disable.

Please add here some index entry that uses "thread-local storage".

Also, "TLS" should be in @acronym (here and elsewhere in the patch).

> +When disabled, @value{GDBN} will attempt to use a helper
> +@code{libthread_db} library if possible, but will fall back to use of
> +its own internal TLS address lookup mechanisms if necessary.
> +
> +When enabled, @value{GDBN} will only use the @value{GDBN}'s internal
> +TLS address lookup mechanisms, if they exist.
> +
> +This command is only available on targets for which internal TLS
> +address lookup support exists.

I think we should tell here that this setting has effect only for
targets that use libthread_db.