Re: [PATCH v3] gdb: Introduce user-friendly namespace identifier for "info shared"

[Guinevere Larsen <guinevere@redhat.com>]

On 4/5/25 5:17 PM, Kevin Buettner wrote:
> On Thu, 27 Mar 2025 14:13:13 -0300
> Guinevere Larsen <guinevere@redhat.com> wrote:
>
>> This commit is the first step in improving the user experience around
>> multiple namespace support. It introduces a user-friendly identifier for
>> namespaces, in the format [[<number>]], that will keep consistent between
>> dlmopen and dlclose calls. The plan is for this identifier to be usable
>> in expressions like `print [[1]]::var` to find a specific instance of a
>> symbol, and so the identifier must not be a valid C++ or Ada namespace
>> identifier, otherwise disambiguation becomes a problem. Support for
>> those expressions has not been implemented yet, it is only mentioned to
>> explain why the identifier looks like this.
> This syntax seems okay to me.  (I don't have a better suggestion.)
>
>> This syntax was chosen based on the C attributes, since nothing in GDB
>> uses a similar syntax that could confuse users. Other syntax options
>> that were explored were "#<number>" and "@<number>". The former was
>> abandoned because when printing a frame, the frame number is also
>> printed with #<number>, so in a lot of the context in which that the
>> identifier would show up, it appears in a confusing way. The latter
>> clashes with the array printing syntax, and I believe that the having
>> "@N::foo" working completely differently to "foo@2" would also lead to a
>> bad user experience.
> Thanks for explaining the other options considered.
>
>> The namespace identifiers are stored via a vector inside svr4_info
>> object. The vector stores the address of the r_debug objects used by
>> glibc to identify each namespace, and the user-friendly ID is the index
>> of the r_debug in the vector. This commit also introduces a set storing
>> the indices of active namespaces. The glibc I used to develop this patch
>> (glibc 2.40 on Fedora 41) doesn't allow an SO to be loaded into a
>> deactivated namespace, and requesting a new namespace when a namespace
>> was previously closed will reuse that namespace. Because of how this is
>> implemented, this patch lets GDB easily track the exact namespace IDs
>> that the inferior will see.
>>
>> Finally, two new solib_ops function pointers were added, find_solib_ns
>> and num_active_namespaces, to allow code outside of solib-svr4 to find
>> and use the namespace identifiers and the number of namespaces,
>> respectively. As a sanity check, the command `info sharedlibrary` has
>> been changed to display the namespace identifier when the inferior has
>> more than one active namespace. With this final change, a couple of tests
>> had to be tweaked to handle the possible new column, and a new test has
>> been created to make sure that the column appears and disappears as
>> needed, and that GDB can track the value of the LMID for namespaces.
>>
>> ---
>> Changes for v3:
>> * Changed the namespace identifier display from #N to [[N]]
>>
>> Changes for v2:
>> * made it so the new column in "info shared" only shows up when multiple
>>    namespaces are active
>> * changes NEWS and docs to reflect that
>> * Added a test for this functionality
> Your solib related changes and the new tests look good to me.  The only
> (slight) misgiving I have is related to identifier being printed in
> the NS column...
>
> (gdb) info sharedlibrary
>  From                To                  NS    Syms Read   Shared Object Library
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7ea2000  0x00007ffff7f90000  [[0]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff7caf000  0x00007ffff7ea2000  [[0]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fbb000  0x00007ffff7fbf000  [[1]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff7b8f000  0x00007ffff7c7d000  [[1]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff799c000  0x00007ffff7b8f000  [[1]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7fb7000  0x00007ffff7fbb000  [[2]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff78ae000  0x00007ffff799c000  [[2]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff76bb000  0x00007ffff78ae000  [[2]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
> 0x00007ffff7fb3000  0x00007ffff7fb7000  [[3]] Yes         /mesquite2/sourceware-git/f42-review/bld/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
> 0x00007ffff75cd000  0x00007ffff76bb000  [[3]] Yes (*)     /lib64/libm.so.6
> 0x00007ffff73da000  0x00007ffff75cd000  [[3]] Yes (*)     /lib64/libc.so.6
> 0x00007ffff7fc7000  0x00007ffff7fff000  [[1]] Yes         /lib64/ld-linux-x86-64.so.2
>
>
> I'm wondering if it might be better to print just (e.g.) "1" instead
> of "[[1]]".  The argument in favor of the way you're doing it is that
> it may be a reminder of the syntax that'll (eventually) be used to
> specify linker namespaces in expressions.  Since it is kind of odd
> and may be difficult to remember, especially since this is such
> a niche feature, I think it's okay.
>
> I think this patch should go in so that further progress can be made
> on linker namespace support.  Quibbles regarding whether the NS
> column should have the square brackets or not can be addressed in
> later patches if necessary.

I agree it makes sense to push this and leave final discussion of how to 
print the namespace, but I'll leave my thoughts here.

It was a conscious decision to always display the linker namespace with 
the same look as how the user should input it. That's because I believe 
that, as much as its reasonable, we should try to make it possible for a 
user to figure out how to use commands without requiring that they look 
into the documentation. So if a user ends up dealing with an inferior 
with multiple namespaces, how to interact with that part of the inferior 
should be apparent from the moment the user realizes this is happening. 
In other words, we probably should print the full syntax every time.

Additionally, if we print a stop location like "1::foo", a user could be 
confused about whether that refers to a c++ namespace named 1, or the 
linker namespace, so it is clear to me that we should always print 
"[[1]]:foo". Keeping the NS column as printing the [[]] will also help 
the user read that identifier and keep things consistent throughout GDB.

>
> The approval give below assumes that you've fixed the testcase as
> mentioned in reply.  (I ran into this and added your fix to my local
> sources for testing.  FWIW, I tested on x86_64, aarch64, and riscv.)
>
> Approved-by: Kevin Buettner <kevinb@redhat.com>
>
Thanks for the approval. I've pushed it

-- 
Cheers,
Guinevere Larsen
She/Her/Hers