diff mbox

[uclibc-ng-devel] ld.so cleanup

Message ID 20170626183634.GZ26922@waldemar-brodkorb.de
State Rejected
Headers show

Commit Message

Waldemar Brodkorb June 26, 2017, 6:36 p.m. UTC
Hi,

I would suggest following patch, which removes the special
communication between ld.so and gdb.
While debugging a ld.so issue with gdb 8.0 on alpha, I recognized
that the debugger stalled when trying to execute this code.
It thought this code was added to help debugging ld.so, but it seems
nowadays it prevents to do so. May be bitrotted?

Does anyone found it useful while debugging ld.so with gdb in the
past?

best regards
 Waldemar

Comments

Max Filippov June 28, 2017, 7:18 p.m. UTC | #1
Hi Waldemar,

On Mon, Jun 26, 2017 at 11:36 AM, Waldemar Brodkorb <wbx@uclibc-ng.org> wrote:
> I would suggest following patch, which removes the special
> communication between ld.so and gdb.
> While debugging a ld.so issue with gdb 8.0 on alpha, I recognized
> that the debugger stalled when trying to execute this code.
> It thought this code was added to help debugging ld.so, but it seems
> nowadays it prevents to do so. May be bitrotted?
>
> Does anyone found it useful while debugging ld.so with gdb in the
> past?

AFAIU this code is responsible for notifying gdb of loading/unloading
shared objects, not just for ld.so debugging. It makes gdb commands
like 'catch load' work, which I occasionally find useful.
Waldemar Brodkorb June 29, 2017, 9:11 a.m. UTC | #2
Hi Max,
Max Filippov wrote,

> Hi Waldemar,
> 
> On Mon, Jun 26, 2017 at 11:36 AM, Waldemar Brodkorb <wbx@uclibc-ng.org> wrote:
> > I would suggest following patch, which removes the special
> > communication between ld.so and gdb.
> > While debugging a ld.so issue with gdb 8.0 on alpha, I recognized
> > that the debugger stalled when trying to execute this code.
> > It thought this code was added to help debugging ld.so, but it seems
> > nowadays it prevents to do so. May be bitrotted?
> >
> > Does anyone found it useful while debugging ld.so with gdb in the
> > past?
> 
> AFAIU this code is responsible for notifying gdb of loading/unloading
> shared objects, not just for ld.so debugging. It makes gdb commands
> like 'catch load' work, which I occasionally find useful.

Okay, thanks for your feedback. I think I will just discard the
patch and locally hack out the calls for now.
Maybe when alpha ld.so is working and the failure still exist,
I add another option to disable these calls via make menuconfig.

best regards
 Waldemar
diff mbox

Patch

From 20f3fafe7bb675844e93336efb96a23457dd4773 Mon Sep 17 00:00:00 2001
From: Waldemar Brodkorb <wbx@openadk.org>
Date: Fri, 23 Jun 2017 23:06:03 +0200
Subject: [PATCH] ld.so: remove GDB communication support

While analyzing a problem with GDB 8.0 under Qemu for Alpha
a deadlock occured. It seems the special GDb communication
code is no longer working as designed.

Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
---
 ldso/include/dl-elf.h         | 14 +++-----------
 ldso/ldso/avr32/dl-sysdep.h   |  2 +-
 ldso/ldso/c6x/dl-sysdep.h     |  2 +-
 ldso/ldso/dl-elf.c            |  8 ++++----
 ldso/ldso/dl-startup.c        |  4 ++--
 ldso/ldso/ldso.c              | 33 ++-------------------------------
 ldso/ldso/mips/dl-sysdep.h    |  4 +---
 ldso/ldso/powerpc/dl-sysdep.h |  2 +-
 ldso/ldso/xtensa/dl-sysdep.h  |  2 +-
 ldso/libdl/libdl.c            | 26 --------------------------
 10 files changed, 16 insertions(+), 81 deletions(-)

diff --git a/ldso/include/dl-elf.h b/ldso/include/dl-elf.h
index a827b8d..c8e890c 100644
--- a/ldso/include/dl-elf.h
+++ b/ldso/include/dl-elf.h
@@ -128,25 +128,17 @@  extern void _dl_protect_relro (struct elf_resolve *l);
 #endif
 
 extern unsigned int _dl_parse_dynamic_info(ElfW(Dyn) *dpnt, unsigned long dynamic_info[],
-                                           void *debug_addr, DL_LOADADDR_TYPE load_off);
+                                           DL_LOADADDR_TYPE load_off);
 
 static __always_inline
 unsigned int __dl_parse_dynamic_info(ElfW(Dyn) *dpnt, unsigned long dynamic_info[],
-                                     void *debug_addr, DL_LOADADDR_TYPE load_off)
+                                     DL_LOADADDR_TYPE load_off)
 {
 	unsigned int rtld_flags = 0;
 
 	for (; dpnt->d_tag; dpnt++) {
 		if (dpnt->d_tag < DT_NUM) {
 			dynamic_info[dpnt->d_tag] = dpnt->d_un.d_val;
-#ifndef __mips__
-			/* we disable for mips because normally this page is readonly
-			 * and modifying the value here needlessly dirties a page.
-			 * see this post for more info:
-			 * http://uclibc.org/lists/uclibc/2006-April/015224.html */
-			if (dpnt->d_tag == DT_DEBUG)
-				dpnt->d_un.d_val = (unsigned long)debug_addr;
-#endif
 			if (dpnt->d_tag == DT_BIND_NOW)
 				dynamic_info[DT_BIND_NOW] = 1;
 			if (dpnt->d_tag == DT_FLAGS &&
@@ -190,7 +182,7 @@  unsigned int __dl_parse_dynamic_info(ElfW(Dyn) *dpnt, unsigned long dynamic_info
 		}
 #ifdef ARCH_DYNAMIC_INFO
 		else {
-			ARCH_DYNAMIC_INFO(dpnt, dynamic_info, debug_addr);
+			ARCH_DYNAMIC_INFO(dpnt, dynamic_info);
 		}
 #endif
 	}
diff --git a/ldso/ldso/avr32/dl-sysdep.h b/ldso/ldso/avr32/dl-sysdep.h
index a422127..fec5881 100644
--- a/ldso/ldso/avr32/dl-sysdep.h
+++ b/ldso/ldso/avr32/dl-sysdep.h
@@ -15,7 +15,7 @@ 
 #define ARCH_NUM 1
 #define DT_AVR32_GOTSZ_IDX	(DT_NUM + OS_NUM)
 
-#define ARCH_DYNAMIC_INFO(dpnt, dynamic, debug_addr)			\
+#define ARCH_DYNAMIC_INFO(dpnt, dynamic)				\
 	do {								\
 		if (dpnt->d_tag == DT_AVR32_GOTSZ)			\
 			dynamic[DT_AVR32_GOTSZ_IDX] = dpnt->d_un.d_val;	\
diff --git a/ldso/ldso/c6x/dl-sysdep.h b/ldso/ldso/c6x/dl-sysdep.h
index c2e91d2..563b5a3 100644
--- a/ldso/ldso/c6x/dl-sysdep.h
+++ b/ldso/ldso/c6x/dl-sysdep.h
@@ -219,7 +219,7 @@  elf_machine_relative (DL_LOADADDR_TYPE load_off, const Elf32_Addr rel_addr,
 #define DT_DSBT_SIZE_IDX	(DT_NUM + OS_NUM + 1)
 #define DT_DSBT_INDEX_IDX	(DT_NUM + OS_NUM + 2)
 
-#define ARCH_DYNAMIC_INFO(dpnt,  dynamic, debug_addr) \
+#define ARCH_DYNAMIC_INFO(dpnt,  dynamic) \
 do { \
 if (dpnt->d_tag == DT_C6000_DSBT_BASE) \
      dynamic[DT_DSBT_BASE_IDX] = dpnt->d_un.d_val; \
diff --git a/ldso/ldso/dl-elf.c b/ldso/ldso/dl-elf.c
index 016d468..1fe8a49 100644
--- a/ldso/ldso/dl-elf.c
+++ b/ldso/ldso/dl-elf.c
@@ -775,7 +775,7 @@  struct elf_resolve *_dl_load_elf_shared_library(unsigned int rflags,
 
 	dpnt = (ElfW(Dyn) *) dynamic_addr;
 	_dl_memset(dynamic_info, 0, sizeof(dynamic_info));
-	rtld_flags = _dl_parse_dynamic_info(dpnt, dynamic_info, NULL, lib_loadaddr);
+	rtld_flags = _dl_parse_dynamic_info(dpnt, dynamic_info, lib_loadaddr);
 	/* If the TEXTREL is set, this means that we need to make the pages
 	   writable before we perform relocations.  Do this now. They get set
 	   back again later. */
@@ -803,7 +803,7 @@  struct elf_resolve *_dl_load_elf_shared_library(unsigned int rflags,
 						       ppnt);
 				/* This has invalidated all pointers into the previously readonly segment.
 				   Update any them to point into the remapped segment.  */
-				_dl_parse_dynamic_info(dpnt, dynamic_info, NULL, lib_loadaddr);
+				_dl_parse_dynamic_info(dpnt, dynamic_info, lib_loadaddr);
 #endif
 			}
 		}
@@ -1181,7 +1181,7 @@  char *_dl_strdup(const char *string)
 #endif
 
 unsigned int _dl_parse_dynamic_info(ElfW(Dyn) *dpnt, unsigned long dynamic_info[],
-                                    void *debug_addr, DL_LOADADDR_TYPE load_off)
+                                    DL_LOADADDR_TYPE load_off)
 {
-	return __dl_parse_dynamic_info(dpnt, dynamic_info, debug_addr, load_off);
+	return __dl_parse_dynamic_info(dpnt, dynamic_info, load_off);
 }
diff --git a/ldso/ldso/dl-startup.c b/ldso/ldso/dl-startup.c
index 1b559a8..f7e89b3 100644
--- a/ldso/ldso/dl-startup.c
+++ b/ldso/ldso/dl-startup.c
@@ -228,9 +228,9 @@  DL_START(unsigned long args)
 	tpnt->dynamic_addr = dpnt;
 #if defined(NO_FUNCS_BEFORE_BOOTSTRAP)
 	/* Some architectures cannot call functions here, must inline */
-	__dl_parse_dynamic_info(dpnt, tpnt->dynamic_info, NULL, load_addr);
+	__dl_parse_dynamic_info(dpnt, tpnt->dynamic_info, load_addr);
 #else
-	_dl_parse_dynamic_info(dpnt, tpnt->dynamic_info, NULL, load_addr);
+	_dl_parse_dynamic_info(dpnt, tpnt->dynamic_info, load_addr);
 #endif
 
 	/*
diff --git a/ldso/ldso/ldso.c b/ldso/ldso/ldso.c
index 0257d1f..e0b9c2e 100644
--- a/ldso/ldso/ldso.c
+++ b/ldso/ldso/ldso.c
@@ -53,7 +53,6 @@  char *_dl_preload              = NULL;	/* Things to be loaded before the libs */
 #endif
 int _dl_errno                  = 0;	/* We can't use the real errno in ldso */
 size_t _dl_pagesize            = 0;	/* Store the page size for use later */
-struct r_debug *_dl_debug_addr = NULL;	/* Used to communicate with the gdb debugger */
 void *(*_dl_malloc_function) (size_t size) = NULL;
 void (*_dl_free_function) (void *p) = NULL;
 
@@ -431,7 +430,6 @@  void *_dl_get_ready_to_run(struct elf_resolve *tpnt, DL_LOADADDR_TYPE load_addr,
 	struct elf_resolve *ldso_tpnt = NULL;
 	struct elf_resolve app_tpnt_tmp;
 	struct elf_resolve *app_tpnt = &app_tpnt_tmp;
-	struct r_debug *debug_addr;
 	unsigned long *lpnt;
 	unsigned long *_dl_envp;		/* The environment address */
 	ElfW(Addr) relro_addr = 0;
@@ -608,12 +606,11 @@  of this helper program; chances are you did not intend to run this program.\n\
 		 * This is used by gdb to locate the chain of shared libraries that are
 		 * currently loaded.
 		 */
-		debug_addr = _dl_zalloc(sizeof(struct r_debug));
 		ppnt = (ElfW(Phdr) *)app_tpnt->ppnt;
 		for (i = 0; i < app_tpnt->n_phent; i++, ppnt++) {
 			if (ppnt->p_type == PT_DYNAMIC) {
 				dpnt = (ElfW(Dyn) *) DL_RELOC_ADDR(app_tpnt->loadaddr, ppnt->p_vaddr);
-				_dl_parse_dynamic_info(dpnt, app_tpnt->dynamic_info, debug_addr, app_tpnt->loadaddr);
+				_dl_parse_dynamic_info(dpnt, app_tpnt->dynamic_info, app_tpnt->loadaddr);
 			}
 		}
 
@@ -645,12 +642,6 @@  of this helper program; chances are you did not intend to run this program.\n\
 					"app_tpnt->loadaddr=%x\n", DL_LOADADDR_BASE(app_tpnt->loadaddr));
 	}
 
-	/*
-	 * This is used by gdb to locate the chain of shared libraries that are
-	 * currently loaded.
-	 */
-	debug_addr = _dl_zalloc(sizeof(struct r_debug));
-
 	ppnt = (ElfW(Phdr) *) auxvt[AT_PHDR].a_un.a_val;
 	for (i = 0; i < auxvt[AT_PHNUM].a_un.a_val; i++, ppnt++) {
 		if (ppnt->p_type == PT_GNU_RELRO) {
@@ -662,7 +653,7 @@  of this helper program; chances are you did not intend to run this program.\n\
 		}
 		if (ppnt->p_type == PT_DYNAMIC) {
 			dpnt = (ElfW(Dyn) *) DL_RELOC_ADDR(app_tpnt->loadaddr, ppnt->p_vaddr);
-			_dl_parse_dynamic_info(dpnt, app_tpnt->dynamic_info, debug_addr, app_tpnt->loadaddr);
+			_dl_parse_dynamic_info(dpnt, app_tpnt->dynamic_info, app_tpnt->loadaddr);
 #ifndef __FORCE_SHAREABLE_TEXT_SEGMENTS__
 			/* Ugly, ugly.  We need to call mprotect to change the
 			 * protection of the text pages so that we can do the
@@ -846,19 +837,6 @@  of this helper program; chances are you did not intend to run this program.\n\
 #endif
 
 	ldso_mapaddr = (ElfW(Addr)) auxvt[AT_BASE].a_un.a_val;
-	/*
-	 * OK, fix one more thing - set up debug_addr so it will point
-	 * to our chain.  Later we may need to fill in more fields, but this
-	 * should be enough for now.
-	 */
-	debug_addr->r_map = (struct link_map *) _dl_loaded_modules;
-	debug_addr->r_version = 1;
-	debug_addr->r_ldbase = (ElfW(Addr))
-		DL_LOADADDR_BASE(DL_GET_RUN_ADDR(load_addr, ldso_mapaddr));
-	debug_addr->r_brk = (unsigned long) &_dl_debug_state;
-	_dl_debug_addr = debug_addr;
-
-	/* Do not notify the debugger until the interpreter is in the list */
 
 	/* OK, we now have the application in the list, and we have some
 	 * basic stuff in place.  Now search through the list for other shared
@@ -1383,9 +1361,6 @@  of this helper program; chances are you did not intend to run this program.\n\
 
 	}
 #endif
-	/* Notify the debugger we have added some objects. */
-	_dl_debug_addr->r_state = RT_ADD;
-	_dl_debug_state();
 
 	/* Run pre-initialization functions for the executable.  */
 	_dl_run_array_forward(_dl_loaded_modules->dynamic_info[DT_PREINIT_ARRAY],
@@ -1434,10 +1409,6 @@  of this helper program; chances are you did not intend to run this program.\n\
 
 #endif
 
-	/* Notify the debugger that all objects are now mapped in.  */
-	_dl_debug_addr->r_state = RT_CONSISTENT;
-	_dl_debug_state();
-
 #ifdef __LDSO_STANDALONE_SUPPORT__
 	if (_start == (void *) auxvt[AT_ENTRY].a_un.a_val)
 		return (void *) app_tpnt->l_entry;
diff --git a/ldso/ldso/mips/dl-sysdep.h b/ldso/ldso/mips/dl-sysdep.h
index 9dcc69b..90846f9 100644
--- a/ldso/ldso/mips/dl-sysdep.h
+++ b/ldso/ldso/mips/dl-sysdep.h
@@ -97,7 +97,7 @@  typedef struct
 #define DT_MIPS_SYMTABNO_IDX	(DT_NUM + OS_NUM +2)
 #define DT_MIPS_PLTGOT_IDX	(DT_NUM + OS_NUM +3)
 
-#define ARCH_DYNAMIC_INFO(dpnt,  dynamic, debug_addr) \
+#define ARCH_DYNAMIC_INFO(dpnt,  dynamic) \
 do { \
 if (dpnt->d_tag == DT_MIPS_GOTSYM) \
      dynamic[DT_MIPS_GOTSYM_IDX] = dpnt->d_un.d_val; \
@@ -107,8 +107,6 @@  else if (dpnt->d_tag == DT_MIPS_SYMTABNO) \
      dynamic[DT_MIPS_SYMTABNO_IDX] = dpnt->d_un.d_val; \
 else if (dpnt->d_tag == DT_MIPS_PLTGOT) \
      dynamic[DT_MIPS_PLTGOT_IDX] = dpnt->d_un.d_val; \
-else if ((dpnt->d_tag == DT_MIPS_RLD_MAP) && (dpnt->d_un.d_ptr)) \
-     *(ElfW(Addr) *)(dpnt->d_un.d_ptr) =  (ElfW(Addr)) debug_addr; \
 } while (0)
 
 #define ARCH_SKIP_RELOC(type_class, sym) \
diff --git a/ldso/ldso/powerpc/dl-sysdep.h b/ldso/ldso/powerpc/dl-sysdep.h
index a665d4e..776bca1 100644
--- a/ldso/ldso/powerpc/dl-sysdep.h
+++ b/ldso/ldso/powerpc/dl-sysdep.h
@@ -177,7 +177,7 @@  elf_machine_relative (Elf32_Addr load_off, const Elf32_Addr rel_addr,
 #define ARCH_NUM 1
 #define DT_PPC_GOT_IDX	(DT_NUM + OS_NUM)
 
-#define ARCH_DYNAMIC_INFO(dpnt,  dynamic, debug_addr) \
+#define ARCH_DYNAMIC_INFO(dpnt,  dynamic) \
 do { \
 if (dpnt->d_tag == DT_PPC_GOT) \
      dynamic[DT_PPC_GOT_IDX] = dpnt->d_un.d_ptr; \
diff --git a/ldso/ldso/xtensa/dl-sysdep.h b/ldso/ldso/xtensa/dl-sysdep.h
index d308237..a756f17 100644
--- a/ldso/ldso/xtensa/dl-sysdep.h
+++ b/ldso/ldso/xtensa/dl-sysdep.h
@@ -79,7 +79,7 @@  typedef struct xtensa_got_location_struct {
 
 /* Parse dynamic info */
 #define ARCH_NUM 2
-#define ARCH_DYNAMIC_INFO(dpnt, dynamic, debug_addr) \
+#define ARCH_DYNAMIC_INFO(dpnt, dynamic) \
   do {									\
     if (dpnt->d_tag == DT_XTENSA_GOT_LOC_OFF)				\
       dynamic[DT_XTENSA (GOT_LOC_OFF)] = dpnt->d_un.d_ptr;		\
diff --git a/ldso/libdl/libdl.c b/ldso/libdl/libdl.c
index f5b6e60..89a868f 100644
--- a/ldso/libdl/libdl.c
+++ b/ldso/libdl/libdl.c
@@ -74,7 +74,6 @@  extern struct dyn_elf *_dl_symbol_tables;
 extern struct dyn_elf *_dl_handles;
 extern struct elf_resolve *_dl_loaded_modules;
 extern void _dl_free (void *__ptr);
-extern struct r_debug *_dl_debug_addr;
 extern unsigned long _dl_error_number;
 extern void *(*_dl_malloc_function)(size_t);
 extern void (*_dl_free_function) (void *p);
@@ -114,8 +113,6 @@  char *_dl_library_path         = NULL;         /* Where we look for libraries */
 #endif
 int _dl_errno                  = 0;         /* We can't use the real errno in ldso */
 size_t _dl_pagesize            = PAGE_SIZE; /* Store the page size for use later */
-/* This global variable is also to communicate with debuggers such as gdb. */
-struct r_debug *_dl_debug_addr = NULL;
 
 #include "../ldso/dl-array.c"
 #include "../ldso/dl-debug.c"
@@ -618,18 +615,6 @@  static void *do_dlopen(const char *libname, int flag, ElfW(Addr) from)
 
 #endif
 
-	/* Notify the debugger we have added some objects. */
-	if (_dl_debug_addr) {
-		dl_brk = (void (*)(void)) _dl_debug_addr->r_brk;
-		if (dl_brk != NULL) {
-			_dl_debug_addr->r_state = RT_ADD;
-			(*dl_brk) ();
-
-			_dl_debug_addr->r_state = RT_CONSISTENT;
-			(*dl_brk) ();
-		}
-	}
-
 	/* Run the ctors and setup the dtors */
 	for (i = nlist; i; --i) {
 		tpnt = init_fini_list[i-1];
@@ -1052,17 +1037,6 @@  static int do_dlclose(void *vhandle, int need_fini)
 	}
 #endif
 
-	if (_dl_debug_addr) {
-		dl_brk = (void (*)(void)) _dl_debug_addr->r_brk;
-		if (dl_brk != NULL) {
-			_dl_debug_addr->r_state = RT_DELETE;
-			(*dl_brk) ();
-
-			_dl_debug_addr->r_state = RT_CONSISTENT;
-			(*dl_brk) ();
-		}
-	}
-
 	return 0;
 }
 
-- 
2.1.4