public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] Extend struct r_debug to support multiple namespaces
@ 2021-08-30 17:38 H.J. Lu
  2021-08-30 17:38 ` [PATCH v6 1/2] Add declare_object_symbol_alias for assembly codes [BZ #28128] H.J. Lu
  2021-08-30 17:38 ` [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces H.J. Lu
  0 siblings, 2 replies; 8+ messages in thread
From: H.J. Lu @ 2021-08-30 17:38 UTC (permalink / raw)
  To: libc-alpha; +Cc: gdb, libc-coord, Florian Weimer, Daniel Walker

Changes in the v6 patch:

1. Add comments for dl-debug-compat-symbols.[o|os] usage. 

Changes in the v5 patch:

1. Use struct r_debug as the base for struct r_debug_extended.
2. Keep _dl_debug_initialize return type.

Changes in the v4 patch:

1. Improve the empty namespace removal from the namespace linked list.
2. Check r_version == 0 for the unused namespace.

Changes in the v3 patch:

1. Remove the empty namespace from the namespace linked list.
2. Properly add the new namespace to the linked list.

Changes in the v2 patch:

1. Bump r_version to 2.
2. Don't add DT_DEBUGSZ.
3. Add struct r_debug_extended to extend struct r_debug.
4. Don't update the r_state field in the copy of _r_debug in executable
since it is not consumed by the program.

---
Glibc does not provide an interface for debugger to access libraries
loaded in multiple namespaces via dlmopen.

The current rtld-debugger interface is described in the file:

elf/rtld-debugger-interface.txt

under the "Standard debugger interface" heading.  This interface only
provides access to the first link-map (LM_ID_BASE).

1. Bump r_version to 2.  This triggers the GDB bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=28236

2. Add struct r_debug_extended to extend struct r_debug into a linked-list,
where each element correlates to an unique namespace.
3. Remove the empty namespace from the namespace linked list and clear
its r_version field.
4. Add the new namespace, whose r_version field is 0, to the namespace
linked list.
5. Initialize the r_debug_extended structure only if its r_version field
is 0 and update its r_map field if it is NULL.
6. Add a hidden symbol, _r_debug_extended, for struct r_debug_extended.
7. Provide the compatibility symbol, _r_debug, with size of struct r_debug,
as an alise of _r_debug_extended, for programs which reference _r_debug.
Glibc does not provide an interface for debugger to access libraries
loaded in multiple namespaces via dlmopen.

The current rtld-debugger interface is described in the file:

elf/rtld-debugger-interface.txt

under the "Standard debugger interface" heading.  This interface only
provides access to the first link-map (LM_ID_BASE).

1. Bump r_version to 2.  This triggers the GDB bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=28236

2. Add struct r_debug_extended to extend struct r_debug into a linked-list,
where each element correlates to an unique namespace.
3. Remove the empty namespace from the namespace linked list and clear
its r_version field.
4. Add the new namespace, whose r_version field is 0, to the namespace
linked list.
5. Initialize the r_debug_extended structure only if its r_version field
is 0 and update its r_map field if it is NULL.
6. Add a hidden symbol, _r_debug_extended, for struct r_debug_extended.
7. Provide the compatibility symbol, _r_debug, with size of struct r_debug,
as an alise of _r_debug_extended, for programs which reference _r_debug.

H.J. Lu (2):
  Add declare_object_symbol_alias for assembly codes [BZ #28128]
  Extend struct r_debug to support multiple namespaces

 NEWS                            |  9 ++++-
 csu/Makefile                    |  3 ++
 csu/rtld-sizes.sym              |  4 ++
 elf/Makefile                    | 24 +++++++++++-
 elf/dl-close.c                  | 21 ++++++++++
 elf/dl-debug-symbols-gen.c      | 24 ++++++++++++
 elf/dl-debug-symbols.S          | 37 ++++++++++++++++++
 elf/dl-debug.c                  | 46 ++++++++++++----------
 elf/dl-reloc-static-pie.c       |  2 +-
 elf/link.h                      | 36 ++++++++++++-----
 elf/rtld-debugger-interface.txt | 14 +++++++
 elf/rtld.c                      |  2 +-
 elf/tst-dlmopen4.c              | 68 +++++++++++++++++++++++++++++++++
 include/libc-symbols.h          | 12 +++---
 include/link.h                  |  4 ++
 sysdeps/generic/ldsodefs.h      |  8 ++--
 16 files changed, 270 insertions(+), 44 deletions(-)
 create mode 100644 csu/rtld-sizes.sym
 create mode 100644 elf/dl-debug-symbols-gen.c
 create mode 100644 elf/dl-debug-symbols.S
 create mode 100644 elf/tst-dlmopen4.c

-- 
2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/2] Add declare_object_symbol_alias for assembly codes [BZ #28128]
  2021-08-30 17:38 [PATCH v6 0/2] Extend struct r_debug to support multiple namespaces H.J. Lu
@ 2021-08-30 17:38 ` H.J. Lu
  2021-08-30 17:38 ` [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces H.J. Lu
  1 sibling, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2021-08-30 17:38 UTC (permalink / raw)
  To: libc-alpha; +Cc: gdb, libc-coord, Florian Weimer, Daniel Walker

There are 2 problems in:

 #define declare_symbol_alias(symbol, original, type, size) \
  declare_symbol_alias_1 (symbol, original, type, size)
 #ifdef __ASSEMBLER__
 # define declare_symbol_alias_1(symbol, original, type, size) \
   strong_alias (original, symbol); \
   .type C_SYMBOL_NAME (symbol), %##type; \
   .size C_SYMBOL_NAME (symbol), size

1. .type and .size are substituted by arguments.
2. %##type is expanded to "% type" due to the GCC bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101613

But assembler doesn't support "% type".

Workaround BZ #28128 by

1. Don't define declare_symbol_alias for assembly codes.
2. Define declare_object_symbol_alias for assembly codes.
---
 include/libc-symbols.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index d41ecf4384..1678071d77 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -324,14 +324,16 @@ for linking")
    This is only necessary when defining something in assembly, or playing
    funny alias games where the size should be other than what the compiler
    thinks it is.  */
-#define declare_symbol_alias(symbol, original, type, size) \
-  declare_symbol_alias_1 (symbol, original, type, size)
 #ifdef __ASSEMBLER__
-# define declare_symbol_alias_1(symbol, original, type, size) \
+# define declare_object_symbol_alias(symbol, original, size) \
+  declare_object_symbol_alias_1 (symbol, original, size)
+# define declare_object_symbol_alias_1(symbol, original, s_size) \
    strong_alias (original, symbol); \
-   .type C_SYMBOL_NAME (symbol), %##type; \
-   .size C_SYMBOL_NAME (symbol), size
+   .type C_SYMBOL_NAME (symbol), %object; \
+   .size C_SYMBOL_NAME (symbol), s_size
 #else /* Not __ASSEMBLER__.  */
+# define declare_symbol_alias(symbol, original, type, size) \
+  declare_symbol_alias_1 (symbol, original, type, size)
 # define declare_symbol_alias_1(symbol, original, type, size) \
    asm (".globl " __SYMBOL_PREFIX #symbol \
 	"\n\t" declare_symbol_alias_1_alias (symbol, original) \
-- 
2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-08-30 17:38 [PATCH v6 0/2] Extend struct r_debug to support multiple namespaces H.J. Lu
  2021-08-30 17:38 ` [PATCH v6 1/2] Add declare_object_symbol_alias for assembly codes [BZ #28128] H.J. Lu
@ 2021-08-30 17:38 ` H.J. Lu
  2021-09-06  9:39   ` Florian Weimer
  1 sibling, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2021-08-30 17:38 UTC (permalink / raw)
  To: libc-alpha; +Cc: gdb, libc-coord, Florian Weimer, Daniel Walker

Glibc does not provide an interface for debugger to access libraries
loaded in multiple namespaces via dlmopen.

The current rtld-debugger interface is described in the file:

elf/rtld-debugger-interface.txt

under the "Standard debugger interface" heading.  This interface only
provides access to the first link-map (LM_ID_BASE).

1. Bump r_version to 2.  This triggers the GDB bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=28236

2. Add struct r_debug_extended to extend struct r_debug into a linked-list,
where each element correlates to an unique namespace.
3. Remove the empty namespace from the namespace linked list and clear
its r_version field.
4. Add the new namespace, whose r_version field is 0, to the namespace
linked list.
5. Initialize the r_debug_extended structure only if its r_version field
is 0 and update its r_map field if it is NULL.
6. Add a hidden symbol, _r_debug_extended, for struct r_debug_extended.
7. Provide the compatibility symbol, _r_debug, with size of struct r_debug,
as an alise of _r_debug_extended, for programs which reference _r_debug.
---
 NEWS                            |  9 ++++-
 csu/Makefile                    |  3 ++
 csu/rtld-sizes.sym              |  4 ++
 elf/Makefile                    | 24 +++++++++++-
 elf/dl-close.c                  | 21 ++++++++++
 elf/dl-debug-symbols-gen.c      | 24 ++++++++++++
 elf/dl-debug-symbols.S          | 37 ++++++++++++++++++
 elf/dl-debug.c                  | 46 ++++++++++++----------
 elf/dl-reloc-static-pie.c       |  2 +-
 elf/link.h                      | 36 ++++++++++++-----
 elf/rtld-debugger-interface.txt | 14 +++++++
 elf/rtld.c                      |  2 +-
 elf/tst-dlmopen4.c              | 68 +++++++++++++++++++++++++++++++++
 include/link.h                  |  4 ++
 sysdeps/generic/ldsodefs.h      |  8 ++--
 15 files changed, 263 insertions(+), 39 deletions(-)
 create mode 100644 csu/rtld-sizes.sym
 create mode 100644 elf/dl-debug-symbols-gen.c
 create mode 100644 elf/dl-debug-symbols.S
 create mode 100644 elf/tst-dlmopen4.c

diff --git a/NEWS b/NEWS
index 79c895e382..9e59cde737 100644
--- a/NEWS
+++ b/NEWS
@@ -9,11 +9,16 @@ Version 2.35
 
 Major new features:
 
-  [Add new features here]
+* Bump r_version in the debugger interface to 2 and add a new field,
+  r_next, support multiple namespaces.
 
 Deprecated and removed features, and other changes affecting compatibility:
 
-  [Add deprecations, removals and changes affecting compatibility here]
+* The r_version update in the debugger interface makes the glibc binary
+  incompatible with GDB binaries built without the following commits:
+
+  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
+  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface
 
 Changes to build and runtime requirements:
 
diff --git a/csu/Makefile b/csu/Makefile
index 3054329cea..e2390e4a7d 100644
--- a/csu/Makefile
+++ b/csu/Makefile
@@ -88,6 +88,9 @@ endif
 before-compile += $(objpfx)abi-tag.h
 generated += abi-tag.h
 
+# Put it here to generate it earlier.
+gen-as-const-headers += rtld-sizes.sym
+
 # These are the special initializer/finalizer files.  They are always the
 # first and last file in the link.  crti.o ... crtn.o define the global
 # "functions" _init and _fini to run the .init and .fini sections.
diff --git a/csu/rtld-sizes.sym b/csu/rtld-sizes.sym
new file mode 100644
index 0000000000..40dd8edaec
--- /dev/null
+++ b/csu/rtld-sizes.sym
@@ -0,0 +1,4 @@
+#include <link.h>
+
+--
+COMPAT_R_DEBUG_SIZE	sizeof (struct r_debug)
diff --git a/elf/Makefile b/elf/Makefile
index 9f3fadc37e..d0d226fe9a 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -35,7 +35,8 @@ dl-routines	= $(addprefix dl-,load lookup object reloc deps \
 				  execstack open close trampoline \
 				  exception sort-maps lookup-direct \
 				  call-libc-early-init write \
-				  thread_gscope_wait tls_init_tp)
+				  thread_gscope_wait tls_init_tp \
+				  debug-symbols)
 ifeq (yes,$(use-ldconfig))
 dl-routines += dl-cache
 endif
@@ -203,7 +204,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-tls16 tst-tls17 tst-tls18 tst-tls19 tst-tls-dlinfo \
 	 tst-align tst-align2 \
 	 tst-dlmodcount tst-dlopenrpath tst-deep1 \
-	 tst-dlmopen1 tst-dlmopen3 \
+	 tst-dlmopen1 tst-dlmopen3 tst-dlmopen4 \
 	 unload3 unload4 unload5 unload6 unload7 unload8 tst-global1 order2 \
 	 tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
 	 tst-addr1 tst-thrlock \
@@ -674,6 +675,23 @@ LC_ALL=C sed $(ldd-rewrite) < $< \
 endef
 endif
 
+ifeq ($(build-shared),yes)
+# dl-debug-compat-symbols.os and dl-debug-compat-symbols.o are assembly
+# codes included in dl-debug-symbols.S.
+generated += dl-debug-compat-symbols.os dl-debug-compat-symbols.o
+
+libof-dl-debug-compat-symbols = rtld
+
+$(objpfx)dl-debug-compat-symbols.os: dl-debug-symbols-gen.c
+	$(compile-command.c) -S
+
+$(objpfx)dl-debug-compat-symbols.o: dl-debug-symbols-gen.c
+	$(compile-command.c) -S
+
+$(objpfx)dl-debug-symbols.os: $(objpfx)dl-debug-compat-symbols.os
+$(objpfx)dl-debug-symbols.o: $(objpfx)dl-debug-compat-symbols.o
+endif
+
 $(objpfx)ldd: ldd.bash.in $(common-objpfx)soversions.mk \
 	      $(common-objpfx)config.make
 	$(gen-ldd)
@@ -1244,6 +1262,8 @@ $(objpfx)tst-dlmopen2.out: $(objpfx)tst-dlmopen1mod.so
 
 $(objpfx)tst-dlmopen3.out: $(objpfx)tst-dlmopen1mod.so
 
+$(objpfx)tst-dlmopen4.out: $(objpfx)tst-dlmopen1mod.so
+
 $(objpfx)tst-audit1.out: $(objpfx)tst-auditmod1.so
 tst-audit1-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so
 
diff --git a/elf/dl-close.c b/elf/dl-close.c
index f39001cab9..43873d2543 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -709,6 +709,27 @@ _dl_close_worker (struct link_map *map, bool force)
 	      assert (nsid != LM_ID_BASE);
 	      ns->_ns_loaded = imap->l_next;
 
+	      if (ns->_ns_loaded == NULL)
+		{
+		  /* Remove the empty namespace from the namespace linked
+		     list.  */
+		  struct r_debug_extended **pp, *p;
+
+		  for (pp = &_r_debug_extended.r_next;
+		       (p = *pp) != NULL;
+		       pp = &p->r_next)
+		    if (p == &ns->_ns_debug)
+		      {
+			/* Remove the empty namespace.  */
+			*pp = p->r_next;
+
+			/* Clear r_version to indicate that it is
+			   unused.  */
+			p->base.r_version = 0;
+			break;
+		      }
+		}
+
 	      /* Update the pointer to the head of the list
 		 we leave for debuggers to examine.  */
 	      r->r_map = (void *) ns->_ns_loaded;
diff --git a/elf/dl-debug-symbols-gen.c b/elf/dl-debug-symbols-gen.c
new file mode 100644
index 0000000000..2406260bcb
--- /dev/null
+++ b/elf/dl-debug-symbols-gen.c
@@ -0,0 +1,24 @@
+/* Generate the _r_debug_extended symbol used to communicate dynamic
+   linker state to the debugger at runtime.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+
+/* This structure communicates dl state to the debugger.  The debugger
+   finds it via the DT_DEBUG entry in the dynamic section.  */
+struct r_debug_extended _r_debug_extended;
diff --git a/elf/dl-debug-symbols.S b/elf/dl-debug-symbols.S
new file mode 100644
index 0000000000..902b9d4a3c
--- /dev/null
+++ b/elf/dl-debug-symbols.S
@@ -0,0 +1,37 @@
+/* Define symbols used to communicate dynamic linker state to the
+   debugger at runtime.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <rtld-sizes.h>
+
+/* Define 2 symbols, _r_debug_extended and _r_debug, which is an alias
+   of _r_debug_extended, but with the size of struct r_debug and is
+   provided as a compatibility symbol.  Since in assembly codes, the
+   symbol alias with a different size must be placed after the original
+   symbol and we can't control their orders generated by compiler, we
+   place them directly in assembly codes.  dl-debug-compat-symbols.[o|os]
+   generated from dl-debug-symbols-gen.c defines _r_debug_extended.  */
+
+#ifdef SHARED
+# include "dl-debug-compat-symbols.os"
+#else
+# include "dl-debug-compat-symbols.o"
+#endif
+
+declare_object_symbol_alias (_r_debug, _r_debug_extended,
+			     COMPAT_R_DEBUG_SIZE);
diff --git a/elf/dl-debug.c b/elf/dl-debug.c
index 2cd5f09753..81dd40960a 100644
--- a/elf/dl-debug.c
+++ b/elf/dl-debug.c
@@ -30,37 +30,43 @@ extern const int verify_link_map_members[(VERIFY_MEMBER (l_addr)
 					  && VERIFY_MEMBER (l_prev))
 					 ? 1 : -1];
 
-/* This structure communicates dl state to the debugger.  The debugger
-   normally finds it via the DT_DEBUG entry in the dynamic section, but in
-   a statically-linked program there is no dynamic section for the debugger
-   to examine and it looks for this particular symbol name.  */
-struct r_debug _r_debug;
-
-
-/* Initialize _r_debug if it has not already been done.  The argument is
-   the run-time load address of the dynamic linker, to be put in
-   _r_debug.r_ldbase.  Returns the address of _r_debug.  */
+/* Initialize _r_debug_extended if it has not already been done.  The
+   argument is the run-time load address of the dynamic linker, to be
+   put in _r_debug_extended.r_ldbase.  Returns the address of
+   _r_debug_extended.  */
 
 struct r_debug *
 _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
 {
-  struct r_debug *r;
+  struct r_debug_extended *r;
 
   if (ns == LM_ID_BASE)
-    r = &_r_debug;
+    r = &_r_debug_extended;
   else
-    r = &GL(dl_ns)[ns]._ns_debug;
+    {
+      r = &GL(dl_ns)[ns]._ns_debug;
+      if (DL_NNS > 1 && r->base.r_version == 0)
+	{
+	  /* Add the new namespace to the linked list.  */
+	  r->r_next = _r_debug_extended.r_next;
+	  _r_debug_extended.r_next = r;
+	}
+    }
 
-  if (r->r_map == NULL || ldbase != 0)
+  if (r->base.r_version == 0)
     {
-      /* Tell the debugger where to find the map of loaded objects.  */
-      r->r_version = 1	/* R_DEBUG_VERSION XXX */;
-      r->r_ldbase = ldbase ?: _r_debug.r_ldbase;
-      r->r_map = (void *) GL(dl_ns)[ns]._ns_loaded;
-      r->r_brk = (ElfW(Addr)) &_dl_debug_state;
+      /* Tell the debugger where to find the map of loaded objects.
+	 Bump r_version to 2 for the r_next field.  */
+      r->base.r_version = 2;
+      r->base.r_ldbase = ldbase ?: _r_debug_extended.base.r_ldbase;
+      r->base.r_brk = (ElfW(Addr)) &_dl_debug_state;
+      r->r_next = NULL;
     }
 
-  return r;
+  if (r->base.r_map == NULL)
+    r->base.r_map = (void *) GL(dl_ns)[ns]._ns_loaded;
+
+  return &r->base;
 }
 
 
diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index d5bd2f31e9..289651b341 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -51,7 +51,7 @@ _dl_relocate_static_pie (void)
   ELF_DYNAMIC_RELOCATE (main_map, 0, 0, 0);
   main_map->l_relocated = 1;
 
-  /* Initialize _r_debug.  */
+  /* Initialize _r_debug_extended.  */
   struct r_debug *r = _dl_debug_initialize (0, LM_ID_BASE);
   r->r_state = RT_CONSISTENT;
 
diff --git a/elf/link.h b/elf/link.h
index ff3a85c847..a297318236 100644
--- a/elf/link.h
+++ b/elf/link.h
@@ -34,14 +34,13 @@
 #include <bits/elfclass.h>		/* Defines __ELF_NATIVE_CLASS.  */
 #include <bits/link.h>
 
-/* Rendezvous structure used by the run-time dynamic linker to communicate
-   details of shared object loading to the debugger.  If the executable's
-   dynamic section has a DT_DEBUG element, the run-time linker sets that
-   element's value to the address where this structure can be found.  */
+/* The legacy rendezvous structure used by the run-time dynamic linker to
+   communicate details of shared object loading to the debugger.  */
 
 struct r_debug
   {
-    int r_version;		/* Version number for this protocol.  */
+    /* Version number for this protocol.  It should be greater than 0.  */
+    int r_version;
 
     struct link_map *r_map;	/* Head of the chain of loaded objects.  */
 
@@ -63,16 +62,35 @@ struct r_debug
     ElfW(Addr) r_ldbase;	/* Base address the linker is loaded at.  */
   };
 
-/* This is the instance of that structure used by the dynamic linker.  */
+/* This is the compatibility symbol of that structure provided by the
+   dynamic linker.  */
 extern struct r_debug _r_debug;
 
+/* The extended rendezvous structure used by the run-time dynamic linker
+   to communicate details of shared object loading to the debugger.  If
+   the executable's dynamic section has a DT_DEBUG element, the run-time
+   linker sets that element's value to the address where this structure
+   can be found.  */
+
+struct r_debug_extended
+  {
+    struct r_debug base;
+
+    /* The following field is added by r_version == 2.  */
+
+    /* Link to the next r_debug_extended structure.  Each r_debug_extended
+       structure represents a different namespace.  The first
+       r_debug_extended structure is for the default namespace.  */
+    struct r_debug_extended *r_next;
+  };
+
 /* This symbol refers to the "dynamic structure" in the `.dynamic' section
    of whatever module refers to `_DYNAMIC'.  So, to find its own
-   `struct r_debug', a program could do:
+   `struct r_debug_extended', a program could do:
      for (dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn)
        if (dyn->d_tag == DT_DEBUG)
-	 r_debug = (struct r_debug *) dyn->d_un.d_ptr;
-   */
+	 r_debug_extended = (struct r_debug_extended *) dyn->d_un.d_ptr;
+ */
 extern ElfW(Dyn) _DYNAMIC[];
 
 /* Structure describing a loaded shared object.  The `l_next' and `l_prev'
diff --git a/elf/rtld-debugger-interface.txt b/elf/rtld-debugger-interface.txt
index 61bc99e4b0..f6aaa28706 100644
--- a/elf/rtld-debugger-interface.txt
+++ b/elf/rtld-debugger-interface.txt
@@ -9,6 +9,9 @@ structure can be found.
 
 The r_debug structure contains (amongst others) the following fields:
 
+  int r_version:
+    Version number for this protocol.  It should be greater than 0.
+
   struct link_map *r_map:
     A linked list of loaded objects.
 
@@ -32,6 +35,17 @@ but there is no way for the debugger to discover whether any of the
 objects in the link-map have been relocated or not.
 
 
+Extension to the r_debug structure
+==================================
+
+The r_debug_extended structure is an extension of the r_debug interface.
+If r_version is 2, one additional field is available:
+
+  struct r_debug_extended *r_next;
+    Link to the next r_debug_extended structure.  Each r_debug_extended
+    structure represents a different namespace.  The first r_debug_extended
+    structure is for the default namespace.
+
 Probe-based debugger interface
 ==============================
 
diff --git a/elf/rtld.c b/elf/rtld.c
index 878e6480f4..7f80378fb4 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1660,7 +1660,7 @@ dl_main (const ElfW(Phdr) *phdr,
      objects.  */
   call_init_paths (&state);
 
-  /* Initialize _r_debug.  */
+  /* Initialize _r_debug_extended.  */
   struct r_debug *r = _dl_debug_initialize (GL(dl_rtld_map).l_addr,
 					    LM_ID_BASE);
   r->r_state = RT_CONSISTENT;
diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
new file mode 100644
index 0000000000..7a6c502e8c
--- /dev/null
+++ b/elf/tst-dlmopen4.c
@@ -0,0 +1,68 @@
+/* Test struct r_debug_extended via DT_DEBUG.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <link.h>
+#include <stdlib.h>
+#include <string.h>
+#include <gnu/lib-names.h>
+#include <support/xdlfcn.h>
+#include <support/check.h>
+#include <support/test-driver.h>
+
+#ifndef ELF_MACHINE_GET_R_DEBUG
+# define ELF_MACHINE_GET_R_DEBUG(d) \
+    (__extension__ ({ 						\
+      struct r_debug_extended *debug;				\
+      if ((d)->d_tag == DT_DEBUG)				\
+	debug = (struct r_debug_extended *) (d)->d_un.d_ptr;	\
+      else							\
+	debug = NULL;						\
+      debug; }))
+#endif
+
+static int
+do_test (void)
+{
+  void *h = xdlmopen (LM_ID_NEWLM, "$ORIGIN/tst-dlmopen1mod.so",
+		      RTLD_LAZY);
+
+  int status = EXIT_FAILURE;
+  ElfW(Dyn) *d;
+  for (d = _DYNAMIC; d->d_tag != DT_NULL; ++d)
+    {
+      struct r_debug_extended *debug = ELF_MACHINE_GET_R_DEBUG (d);
+      if (debug != NULL)
+	{
+	  TEST_VERIFY_EXIT (debug->base.r_version == 2);
+	  TEST_VERIFY_EXIT (debug->r_next != NULL);
+	  TEST_VERIFY_EXIT (debug->r_next->r_next == NULL);
+	  TEST_VERIFY_EXIT (debug->r_next->base.r_map != NULL);
+	  TEST_VERIFY_EXIT (debug->r_next->base.r_map->l_name != NULL);
+	  const char *name = basename (debug->r_next->base.r_map->l_name);
+	  TEST_VERIFY_EXIT (strcmp (name, "tst-dlmopen1mod.so") == 0);
+	  status = EXIT_SUCCESS;
+	}
+    }
+
+  xdlclose (h);
+
+  return status;
+}
+
+#include <support/test-driver.c>
diff --git a/include/link.h b/include/link.h
index 4af16cb596..7b8250db36 100644
--- a/include/link.h
+++ b/include/link.h
@@ -353,6 +353,10 @@ struct auditstate
 };
 
 
+/* This is the hidden instance of struct r_debug_extended used by the
+   dynamic linker.  */
+extern struct r_debug_extended _r_debug_extended attribute_hidden;
+
 #if __ELF_NATIVE_CLASS == 32
 # define symbind symbind32
 #elif __ELF_NATIVE_CLASS == 64
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 9c15259236..813f8659a1 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -355,7 +355,7 @@ struct rtld_global
       void (*free) (void *);
     } _ns_unique_sym_table;
     /* Keep track of changes to each namespace' list.  */
-    struct r_debug _ns_debug;
+    struct r_debug_extended _ns_debug;
   } _dl_ns[DL_NNS];
   /* One higher than index of last used namespace.  */
   EXTERN size_t _dl_nns;
@@ -1093,9 +1093,9 @@ extern void _dl_sort_maps (struct link_map **maps, unsigned int nmaps,
 extern void _dl_debug_state (void);
 rtld_hidden_proto (_dl_debug_state)
 
-/* Initialize `struct r_debug' if it has not already been done.  The
-   argument is the run-time load address of the dynamic linker, to be put
-   in the `r_ldbase' member.  Returns the address of the structure.  */
+/* Initialize `struct r_debug_extended' if it has not already been done.
+   The argument is the run-time load address of the dynamic linker, to
+   be put in the `r_ldbase' member.  Returns the address of the structure.  */
 extern struct r_debug *_dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
      attribute_hidden;
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-08-30 17:38 ` [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces H.J. Lu
@ 2021-09-06  9:39   ` Florian Weimer
  2021-09-06 13:19     ` H.J. Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2021-09-06  9:39 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha, gdb, libc-coord, Daniel Walker

* H. J. Lu:

> +* The r_version update in the debugger interface makes the glibc binary
> +  incompatible with GDB binaries built without the following commits:
> +
> +  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
> +  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface

Does this incompatibility happen even if audit modules and dlmopen are
not used?

> +ifeq ($(build-shared),yes)
> +# dl-debug-compat-symbols.os and dl-debug-compat-symbols.o are assembly
> +# codes included in dl-debug-symbols.S.
> +generated += dl-debug-compat-symbols.os dl-debug-compat-symbols.o
> +
> +libof-dl-debug-compat-symbols = rtld
> +
> +$(objpfx)dl-debug-compat-symbols.os: dl-debug-symbols-gen.c
> +	$(compile-command.c) -S
> +
> +$(objpfx)dl-debug-compat-symbols.o: dl-debug-symbols-gen.c
> +	$(compile-command.c) -S
> +
> +$(objpfx)dl-debug-symbols.os: $(objpfx)dl-debug-compat-symbols.os
> +$(objpfx)dl-debug-symbols.o: $(objpfx)dl-debug-compat-symbols.o
> +endif

This puts the assember output from the compiler through the
preprocessor.  That seems to be brittle.  I think you would have to
preprocess the manually written fragment separately.

However, I think we are overdesigning things here.  The following in
dl-debug-symbols-gen.c should work (and the file should have a different
name then):

/* Alias _r_debug to a prefix of _r_debug_extended.  */
asm (".set _r_debug, _r_debug_extended\n\t"
     ".type _r_debug, %object\n\t"
     ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING);
#if __WORDSIZE == 64
_Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)");
asm (".size _r_debug, 40");
#else
_Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)");
asm (".size _r_debug, 20");
#endif

It's not exactly pretty, but at least it's obvious what is going on.
(Extended asm with input operands is not supported outside of functions.)

And it's not that the size of struct _r_debug is going to change.

> diff --git a/elf/dl-close.c b/elf/dl-close.c
> index f39001cab9..43873d2543 100644
> --- a/elf/dl-close.c
> +++ b/elf/dl-close.c
> @@ -709,6 +709,27 @@ _dl_close_worker (struct link_map *map, bool force)
>  	      assert (nsid != LM_ID_BASE);
>  	      ns->_ns_loaded = imap->l_next;
>  
> +	      if (ns->_ns_loaded == NULL)
> +		{
> +		  /* Remove the empty namespace from the namespace linked
> +		     list.  */
> +		  struct r_debug_extended **pp, *p;
> +
> +		  for (pp = &_r_debug_extended.r_next;
> +		       (p = *pp) != NULL;
> +		       pp = &p->r_next)
> +		    if (p == &ns->_ns_debug)
> +		      {
> +			/* Remove the empty namespace.  */
> +			*pp = p->r_next;
> +
> +			/* Clear r_version to indicate that it is
> +			   unused.  */
> +			p->base.r_version = 0;
> +			break;
> +		      }
> +		}

Is this necessary?  It makes concurrent access to the list harder and
does not save any memory.

> diff --git a/elf/rtld-debugger-interface.txt b/elf/rtld-debugger-interface.txt
> index 61bc99e4b0..f6aaa28706 100644
> --- a/elf/rtld-debugger-interface.txt
> +++ b/elf/rtld-debugger-interface.txt
> @@ -9,6 +9,9 @@ structure can be found.
>  
>  The r_debug structure contains (amongst others) the following fields:
>  
> +  int r_version:
> +    Version number for this protocol.  It should be greater than 0.
> +

It seems to me that r_version starts out as 0 and is initialized later.
Maybe this should be described here, and tested in the test case.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-09-06  9:39   ` Florian Weimer
@ 2021-09-06 13:19     ` H.J. Lu
  2021-09-06 14:24       ` Florian Weimer
  0 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2021-09-06 13:19 UTC (permalink / raw)
  To: Florian Weimer; +Cc: GNU C Library, GDB, libc-coord, Daniel Walker

On Mon, Sep 6, 2021 at 2:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > +* The r_version update in the debugger interface makes the glibc binary
> > +  incompatible with GDB binaries built without the following commits:
> > +
> > +  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
> > +  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface
>
> Does this incompatibility happen even if audit modules and dlmopen are
> not used?

Yes.

> > +ifeq ($(build-shared),yes)
> > +# dl-debug-compat-symbols.os and dl-debug-compat-symbols.o are assembly
> > +# codes included in dl-debug-symbols.S.
> > +generated += dl-debug-compat-symbols.os dl-debug-compat-symbols.o
> > +
> > +libof-dl-debug-compat-symbols = rtld
> > +
> > +$(objpfx)dl-debug-compat-symbols.os: dl-debug-symbols-gen.c
> > +     $(compile-command.c) -S
> > +
> > +$(objpfx)dl-debug-compat-symbols.o: dl-debug-symbols-gen.c
> > +     $(compile-command.c) -S
> > +
> > +$(objpfx)dl-debug-symbols.os: $(objpfx)dl-debug-compat-symbols.os
> > +$(objpfx)dl-debug-symbols.o: $(objpfx)dl-debug-compat-symbols.o
> > +endif
>
> This puts the assember output from the compiler through the
> preprocessor.  That seems to be brittle.  I think you would have to
> preprocess the manually written fragment separately.
>
> However, I think we are overdesigning things here.  The following in
> dl-debug-symbols-gen.c should work (and the file should have a different
> name then):
>
> /* Alias _r_debug to a prefix of _r_debug_extended.  */
> asm (".set _r_debug, _r_debug_extended\n\t"
>      ".type _r_debug, %object\n\t"
>      ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING);
> #if __WORDSIZE == 64
> _Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)");
> asm (".size _r_debug, 40");
> #else
> _Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)");
> asm (".size _r_debug, 20");
> #endif
>
> It's not exactly pretty, but at least it's obvious what is going on.
> (Extended asm with input operands is not supported outside of functions.)

This was the first thing I tried and it didn't work:

[hjl@gnu-cfl-2 tmp]$ cat foo.s
.set  _r_debug, _r_debug_extended
.globl _r_debug
.type _r_debug, %object
.size _r_debug, 40
.data
.type _r_debug_extended, %object
.size _r_debug_extended, 48
.globl _r_debug_extended
_r_debug_extended:
.zero 48
[hjl@gnu-cfl-2 tmp]$ gcc -c foo.s
[hjl@gnu-cfl-2 tmp]$ readelf -sW foo.o | grep _r_debug
     1: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug
     2: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug_extended
[hjl@gnu-cfl-2 tmp]$

> And it's not that the size of struct _r_debug is going to change.
>
> > diff --git a/elf/dl-close.c b/elf/dl-close.c
> > index f39001cab9..43873d2543 100644
> > --- a/elf/dl-close.c
> > +++ b/elf/dl-close.c
> > @@ -709,6 +709,27 @@ _dl_close_worker (struct link_map *map, bool force)
> >             assert (nsid != LM_ID_BASE);
> >             ns->_ns_loaded = imap->l_next;
> >
> > +           if (ns->_ns_loaded == NULL)
> > +             {
> > +               /* Remove the empty namespace from the namespace linked
> > +                  list.  */
> > +               struct r_debug_extended **pp, *p;
> > +
> > +               for (pp = &_r_debug_extended.r_next;
> > +                    (p = *pp) != NULL;
> > +                    pp = &p->r_next)
> > +                 if (p == &ns->_ns_debug)
> > +                   {
> > +                     /* Remove the empty namespace.  */
> > +                     *pp = p->r_next;
> > +
> > +                     /* Clear r_version to indicate that it is
> > +                        unused.  */
> > +                     p->base.r_version = 0;
> > +                     break;
> > +                   }
> > +             }
>
> Is this necessary?  It makes concurrent access to the list harder and

When _dl_close_worker is called, it holds GL(dl_load_lock).  Why does
this change make concurrent access harder?

> does not save any memory.

_dl_debug_initialize can be called multiple times with the same
namespace.  Only the first time we need to initialize the namespace.
I use base.r_version == 0 to check if I need to initialize the namespace
and put it on the linked list.

>
> > diff --git a/elf/rtld-debugger-interface.txt b/elf/rtld-debugger-interface.txt
> > index 61bc99e4b0..f6aaa28706 100644
> > --- a/elf/rtld-debugger-interface.txt
> > +++ b/elf/rtld-debugger-interface.txt
> > @@ -9,6 +9,9 @@ structure can be found.
> >
> >  The r_debug structure contains (amongst others) the following fields:
> >
> > +  int r_version:
> > +    Version number for this protocol.  It should be greater than 0.
> > +
>
> It seems to me that r_version starts out as 0 and is initialized later.
> Maybe this should be described here, and tested in the test case.

r_version is never zero (initialized by _dl_debug_initialize) when the
namespace is in use.  The unused namespace isn't accessible from
user programs.

> Thanks,
> Florian
>

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-09-06 13:19     ` H.J. Lu
@ 2021-09-06 14:24       ` Florian Weimer
  2021-09-06 14:31         ` H.J. Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2021-09-06 14:24 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library, GDB, libc-coord, Daniel Walker

* H. J. Lu:

> On Mon, Sep 6, 2021 at 2:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > +* The r_version update in the debugger interface makes the glibc binary
>> > +  incompatible with GDB binaries built without the following commits:
>> > +
>> > +  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
>> > +  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface
>>
>> Does this incompatibility happen even if audit modules and dlmopen are
>> not used?
>
> Yes.

Why?  Can't we keep r_version at 1 in this case?

>> This puts the assember output from the compiler through the
>> preprocessor.  That seems to be brittle.  I think you would have to
>> preprocess the manually written fragment separately.
>>
>> However, I think we are overdesigning things here.  The following in
>> dl-debug-symbols-gen.c should work (and the file should have a different
>> name then):
>>
>> /* Alias _r_debug to a prefix of _r_debug_extended.  */
>> asm (".set _r_debug, _r_debug_extended\n\t"
>>      ".type _r_debug, %object\n\t"
>>      ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING);
>> #if __WORDSIZE == 64
>> _Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)");
>> asm (".size _r_debug, 40");
>> #else
>> _Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)");
>> asm (".size _r_debug, 20");
>> #endif
>>
>> It's not exactly pretty, but at least it's obvious what is going on.
>> (Extended asm with input operands is not supported outside of functions.)
>
> This was the first thing I tried and it didn't work:
>
> [hjl@gnu-cfl-2 tmp]$ cat foo.s
> .set  _r_debug, _r_debug_extended
> .globl _r_debug
> .type _r_debug, %object
> .size _r_debug, 40
> .data
> .type _r_debug_extended, %object
> .size _r_debug_extended, 48
> .globl _r_debug_extended
> _r_debug_extended:
> .zero 48
> [hjl@gnu-cfl-2 tmp]$ gcc -c foo.s
> [hjl@gnu-cfl-2 tmp]$ readelf -sW foo.o | grep _r_debug
>      1: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug
>      2: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug_extended
> [hjl@gnu-cfl-2 tmp]$

Huh.  Does this mean this depends on the symbol definition order in the
assembler file?

I really hate the post-processing of compiler output.  This isn't GHC. 8->

Can we write a portable assembler file instead?

Nick Clifton has written down some guidelines:

  Tips for writing portable assembler with GNU Assembler (GAS)
  <https://developers.redhat.com/blog/2021/02/26/tips-for-writing-portable-assembler-with-gnu-assembler-gas>

There's no initializer, so all we need to know is size and alignment.

>> Is this necessary?  It makes concurrent access to the list harder and
>
> When _dl_close_worker is called, it holds GL(dl_load_lock).  Why does
> this change make concurrent access harder?

Something else might want to read the list directly, by starting with
DT_DEBUG.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-09-06 14:24       ` Florian Weimer
@ 2021-09-06 14:31         ` H.J. Lu
  2021-09-07 15:18           ` H.J. Lu
  0 siblings, 1 reply; 8+ messages in thread
From: H.J. Lu @ 2021-09-06 14:31 UTC (permalink / raw)
  To: Florian Weimer; +Cc: GNU C Library, GDB, libc-coord, Daniel Walker

On Mon, Sep 6, 2021 at 7:24 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Mon, Sep 6, 2021 at 2:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > +* The r_version update in the debugger interface makes the glibc binary
> >> > +  incompatible with GDB binaries built without the following commits:
> >> > +
> >> > +  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
> >> > +  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface
> >>
> >> Does this incompatibility happen even if audit modules and dlmopen are
> >> not used?
> >
> > Yes.
>
> Why?  Can't we keep r_version at 1 in this case?

r_version is checked in GDB whenever DT_DEBUG is used to access
the rtld debug interface, independent of audit modules and dlmopen.

We can bump r_version only if a non-default namespace is used.

> >> This puts the assember output from the compiler through the
> >> preprocessor.  That seems to be brittle.  I think you would have to
> >> preprocess the manually written fragment separately.
> >>
> >> However, I think we are overdesigning things here.  The following in
> >> dl-debug-symbols-gen.c should work (and the file should have a different
> >> name then):
> >>
> >> /* Alias _r_debug to a prefix of _r_debug_extended.  */
> >> asm (".set _r_debug, _r_debug_extended\n\t"
> >>      ".type _r_debug, %object\n\t"
> >>      ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING);
> >> #if __WORDSIZE == 64
> >> _Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)");
> >> asm (".size _r_debug, 40");
> >> #else
> >> _Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)");
> >> asm (".size _r_debug, 20");
> >> #endif
> >>
> >> It's not exactly pretty, but at least it's obvious what is going on.
> >> (Extended asm with input operands is not supported outside of functions.)
> >
> > This was the first thing I tried and it didn't work:
> >
> > [hjl@gnu-cfl-2 tmp]$ cat foo.s
> > .set  _r_debug, _r_debug_extended
> > .globl _r_debug
> > .type _r_debug, %object
> > .size _r_debug, 40
> > .data
> > .type _r_debug_extended, %object
> > .size _r_debug_extended, 48
> > .globl _r_debug_extended
> > _r_debug_extended:
> > .zero 48
> > [hjl@gnu-cfl-2 tmp]$ gcc -c foo.s
> > [hjl@gnu-cfl-2 tmp]$ readelf -sW foo.o | grep _r_debug
> >      1: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug
> >      2: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug_extended
> > [hjl@gnu-cfl-2 tmp]$
>
> Huh.  Does this mean this depends on the symbol definition order in the
> assembler file?
>
> I really hate the post-processing of compiler output.  This isn't GHC. 8->
>
> Can we write a portable assembler file instead?
>
> Nick Clifton has written down some guidelines:
>
>   Tips for writing portable assembler with GNU Assembler (GAS)
>   <https://developers.redhat.com/blog/2021/02/26/tips-for-writing-portable-assembler-with-gnu-assembler-gas>
>
> There's no initializer, so all we need to know is size and alignment.

Yes, I can do that.

> >> Is this necessary?  It makes concurrent access to the list harder and
> >
> > When _dl_close_worker is called, it holds GL(dl_load_lock).  Why does
> > this change make concurrent access harder?
>
> Something else might want to read the list directly, by starting with
> DT_DEBUG.

I can add _dl_debug_get and use _dl_debug_initialize only for initialization.
Will it work?


-- 
H.J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces
  2021-09-06 14:31         ` H.J. Lu
@ 2021-09-07 15:18           ` H.J. Lu
  0 siblings, 0 replies; 8+ messages in thread
From: H.J. Lu @ 2021-09-07 15:18 UTC (permalink / raw)
  To: Florian Weimer; +Cc: GNU C Library, GDB, libc-coord, Daniel Walker

On Mon, Sep 6, 2021 at 7:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Sep 6, 2021 at 7:24 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > > On Mon, Sep 6, 2021 at 2:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >>
> > >> * H. J. Lu:
> > >>
> > >> > +* The r_version update in the debugger interface makes the glibc binary
> > >> > +  incompatible with GDB binaries built without the following commits:
> > >> > +
> > >> > +  c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux
> > >> > +  4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface
> > >>
> > >> Does this incompatibility happen even if audit modules and dlmopen are
> > >> not used?
> > >
> > > Yes.
> >
> > Why?  Can't we keep r_version at 1 in this case?
>
> r_version is checked in GDB whenever DT_DEBUG is used to access
> the rtld debug interface, independent of audit modules and dlmopen.
>
> We can bump r_version only if a non-default namespace is used.

Fixed.

> > >> This puts the assember output from the compiler through the
> > >> preprocessor.  That seems to be brittle.  I think you would have to
> > >> preprocess the manually written fragment separately.
> > >>
> > >> However, I think we are overdesigning things here.  The following in
> > >> dl-debug-symbols-gen.c should work (and the file should have a different
> > >> name then):
> > >>
> > >> /* Alias _r_debug to a prefix of _r_debug_extended.  */
> > >> asm (".set _r_debug, _r_debug_extended\n\t"
> > >>      ".type _r_debug, %object\n\t"
> > >>      ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING);
> > >> #if __WORDSIZE == 64
> > >> _Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)");
> > >> asm (".size _r_debug, 40");
> > >> #else
> > >> _Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)");
> > >> asm (".size _r_debug, 20");
> > >> #endif
> > >>
> > >> It's not exactly pretty, but at least it's obvious what is going on.
> > >> (Extended asm with input operands is not supported outside of functions.)
> > >
> > > This was the first thing I tried and it didn't work:
> > >
> > > [hjl@gnu-cfl-2 tmp]$ cat foo.s
> > > .set  _r_debug, _r_debug_extended
> > > .globl _r_debug
> > > .type _r_debug, %object
> > > .size _r_debug, 40
> > > .data
> > > .type _r_debug_extended, %object
> > > .size _r_debug_extended, 48
> > > .globl _r_debug_extended
> > > _r_debug_extended:
> > > .zero 48
> > > [hjl@gnu-cfl-2 tmp]$ gcc -c foo.s
> > > [hjl@gnu-cfl-2 tmp]$ readelf -sW foo.o | grep _r_debug
> > >      1: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug
> > >      2: 0000000000000000    48 OBJECT  GLOBAL DEFAULT    2 _r_debug_extended
> > > [hjl@gnu-cfl-2 tmp]$
> >
> > Huh.  Does this mean this depends on the symbol definition order in the
> > assembler file?
> >
> > I really hate the post-processing of compiler output.  This isn't GHC. 8->
> >
> > Can we write a portable assembler file instead?
> >
> > Nick Clifton has written down some guidelines:
> >
> >   Tips for writing portable assembler with GNU Assembler (GAS)
> >   <https://developers.redhat.com/blog/2021/02/26/tips-for-writing-portable-assembler-with-gnu-assembler-gas>
> >
> > There's no initializer, so all we need to know is size and alignment.
>
> Yes, I can do that.

Fixed.

> > >> Is this necessary?  It makes concurrent access to the list harder and
> > >
> > > When _dl_close_worker is called, it holds GL(dl_load_lock).  Why does
> > > this change make concurrent access harder?
> >
> > Something else might want to read the list directly, by starting with
> > DT_DEBUG.
>
> I can add _dl_debug_get and use _dl_debug_initialize only for initialization.
> Will it work?
>

I submitted the v7 patch with:

1. Rewrite dl-debug-symbols.S and remove dl-debug-compat-symbols.c.
2. Bump r_version to 2 only when multiple namespaces are used.
3. Verify r_version == 1 without multiple namespaces.
4. Keep the empty (unused) namespace on the namespace linked list.
5. Use atomic_store_release to update r_version and r_next.
6. Add _dl_debug_update without adding the namespace to the namespace
linked list.

-- 
H.J.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-07 15:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 17:38 [PATCH v6 0/2] Extend struct r_debug to support multiple namespaces H.J. Lu
2021-08-30 17:38 ` [PATCH v6 1/2] Add declare_object_symbol_alias for assembly codes [BZ #28128] H.J. Lu
2021-08-30 17:38 ` [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces H.J. Lu
2021-09-06  9:39   ` Florian Weimer
2021-09-06 13:19     ` H.J. Lu
2021-09-06 14:24       ` Florian Weimer
2021-09-06 14:31         ` H.J. Lu
2021-09-07 15:18           ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).