Re: [RFC PATCH 0/3] implement dlmopen hooks for gdb

[Carlos O'Donell via Libc-alpha <libc-alpha@sourceware.org>]

On 6/26/20 3:32 PM, Daniel Walker via Libc-alpha wrote:
> Cisco System, Inc. has a need to have dlmopen support in gdb, which
> required glibc changes. I think it was known when glibc implemented
> dlmopen that gdb would not work with it.
> 
> Since 2015 Cisco has had these patches in our inventor to fix issues in
> glibc which prevented this type of gdb usage.
> 
> This RFC is mainly to get guidance on this implementation. We have some
> individuals who have signed the copyright assignment for glibc, and we
> will submit these (or different patches) formally thru those channels if
> no one has issues with the implementation.
> 
> Also included in this are a couple of fixes which went along with the
> original implementation.
> 
> Please provide any comments you might have.
> 
> Conan C Huang (3):
>   Segfault when dlopen with RTLD_GLOBAL in dlmopened library
>   glibc: dlopen RTLD_NOLOAD optimization
>   add r_debug multiple namespaces support
> 
>  elf/dl-close.c |  7 ++++++-
>  elf/dl-debug.c | 13 ++++++++++---
>  elf/dl-open.c  |  8 +++++++-
>  elf/link.h     |  4 ++++
>  4 files changed, 27 insertions(+), 5 deletions(-)
> 

Thanks for looking at this. It is something the community would
absolutely like to see. I'll comment quickly to provide direction.

Florian Weimer, Pedro Alves, and I were talking about this as
recently as April where we tried to agree to just adding a
_r_debug_dlmopen with a new ABI for the debugger to use.

Your proposed solution of bumping the version is unacceptable,
and was last rejected by Roland McGrath. The problem is that
when you bump the version the current 

It is easier from
a backwards compatibility perspective to add a new _r_debug_dlmopen
and use that instead.

gdb checks for r_version != 1 and issues a warning, but keeps going:

6952           if (linux_read_memory (priv->r_debug + lmo->r_version_offset,
6953                                  (unsigned char *) &r_version,
6954                                  sizeof (r_version)) != 0
6955               || r_version != 1)
                      ^^^^^^^^^^^^^^
6956             {
6957               warning ("unexpected r_debug version %d", r_version);
6958             }

This is bad precedent that other software might have hard checks
for r_version != 1 stop operating correclty.

I suggest reviewing these threads:
https://sourceware.org/legacy-ml/libc-alpha/2012-11/msg00182.html
https://sourceware.org/legacy-ml/libc-alpha/2012-12/msg00278.html
https://sourceware.org/legacy-ml/libc-alpha/2013-01/msg00045.html

An alternative suggested in 2012 was to add a new DT_* entry to point
to the extended debug information e.g. DT_DEBUG_EXTENDED, and so avoid
needing ld.so for lookup of _r_debug_dlmopen.  Gary Benson also suggests
versioning the new structure, but being very clear what a "version bump"
means, in that we compatible add elements to the end after each version
change. So all consumers would check _r_debug_dlmopen.r_version > 1 to
know they had at least v1 elements.

And for reference from Solaris:
https://docs.oracle.com/cd/E23824_01/html/819-0690/chapter6-1247.html#chapter6-15
I'd want to avoid having to run code to get at these objects, since
experience has shown this is always going to cause problems. Having
an entirely data-driven approach would be preferable, but locks us
into an ABI that we have to be able to bump.

-- 
Cheers,
Carlos.