Re: [patch+7.4] Fix gdbserver qXfer:libraries-svr4 regression in special cases

[Pedro Alves <palves at redhat dot com>]

On 03/03/2012 06:27 PM, Jan Kratochvil wrote:

> in a special case of running gdbserver IIRC by Daniel Jacobowitz before (I did
> not try to find that mail) there is a 7.4 regression by the
> qXfer:libraries-svr4 latency optimization.
> 
> +# In some cases gdbserver cannot find the DT_DEBUG library list on its own and
> +# it needs to fall back to GDB solib-svr4.c look up via symbols from the main
> +# executable.
> +# gdbserver :1234 ld-linux-x86-64.so.2 /bin/prog
> +# gdb /bin/prog
> +# (gdb) target remote ...
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu in non-extended
> gdbserver mode.
> 
> I had even coded this fallback before for the cases where the new gdbserver
> code fails but later decided in such case it is better to fix gdbserver
> instead.  I did not realize there may be cases where gdbserver just cannot
> find the list while GDB can.
> 
> OTOH sure there may be a way of figuring out the shared library list in such
> a case by gdbserver itself.  I did not investigate the details.  Still
> I believe the fallback cases are negligible enough to need to care about
> optimizing them.


GDBserver can ask GDB the address of _r_debug, using the qSymbols mechanism,
so it doesn't look that much trouble.  WDYT of the alternative patch below?
It fixes the testcase too.

While looking at this, I wonder if MIPS isn't broken too?

solib-svr4.c has:

static CORE_ADDR
elf_locate_base (void)
{
  struct minimal_symbol *msymbol;
  CORE_ADDR dyn_ptr;

  /* Look for DT_MIPS_RLD_MAP first.  MIPS executables use this
     instead of DT_DEBUG, although they sometimes contain an unused
     DT_DEBUG.  */
  if (scan_dyntag (DT_MIPS_RLD_MAP, exec_bfd, &dyn_ptr)
      || scan_dyntag_auxv (DT_MIPS_RLD_MAP, &dyn_ptr))


> +  /* We failed to find DT_DEBUG.  Such situation will not change for this
> +     inferior - do not retry it.  Report it to GDB as E01, see for the reasons
> +     at the GDB solib-svr4.c side.  */
> +  if (priv->r_debug == (CORE_ADDR) -1)
> +    return -1;
> +
>    if (priv->r_debug == (CORE_ADDR) -1 || priv->r_debug == 0)

        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If we return early, then this check looks dead now.

2012-03-05  Pedro Alves  <palves@redhat.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-low.c (linux_look_up_symbols): Look up svr4 DSO handling
	related symbols.
	(svr4_look_up_symbols): New.
	(get_r_debug): Rename to ...
	(get_r_debug_dynamic): ... this, and adjust comments.
	(linux_qxfer_libraries_svr4): Fallback &_r_debug from symbols if
	DT_DEBUG isn't found.
---

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f062575..1697e5a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -175,6 +175,7 @@ static int finish_step_over (struct lwp_info *lwp);
 static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
 static int kill_lwp (unsigned long lwpid, int signo);
 static void linux_enable_event_reporting (int pid);
+static void svr4_look_up_symbols (void);
 
 /* True if the low target can hardware single-step.  Such targets
    don't need a BREAKPOINT_REINSERT_ADDR callback.  */
@@ -4522,6 +4523,8 @@ linux_look_up_symbols (void)
 #ifdef USE_THREAD_DB
   struct process_info *proc = current_process ();
 
+  svr4_look_up_symbols ();
+
   if (proc->private->thread_db != NULL)
     return;
 
@@ -5144,6 +5147,22 @@ linux_get_min_fast_tracepoint_insn_len (void)
   return (*the_low_target.get_min_fast_tracepoint_insn_len) ();
 }
 
+/* Cache all future symbols that svr4 handling might request.  We can
+   not request symbols at arbitrary states in the remote protocol,
+   only when the client tells us that new symbols are available.  */
+
+static void
+svr4_look_up_symbols (void)
+{
+  struct process_info *proc = current_process ();
+  struct process_info_private *priv = proc->private;
+
+  if (priv->r_debug_from_syms != 0)
+    return;
+
+  look_up_one_symbol ("_r_debug", &priv->r_debug_from_syms, 1);
+}
+
 /* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
 
 static int
@@ -5275,11 +5294,12 @@ get_dynamic (const int pid, const int is_elf64)
   return 0;
 }
 
-/* Return &_r_debug in the inferior, or -1 if not present.  Return value
-   can be 0 if the inferior does not yet have the library list initialized.  */
+/* Return &_r_debug in the inferior (via PT_DYNAMIC), or -1 if not
+   present.  Return value can be 0 if the inferior does not yet have
+   the library list initialized.  */
 
 static CORE_ADDR
-get_r_debug (const int pid, const int is_elf64)
+get_r_debug_dynamic (const int pid, const int is_elf64)
 {
   CORE_ADDR dynamic_memaddr;
   const int dyn_size = is_elf64 ? sizeof (Elf64_Dyn) : sizeof (Elf32_Dyn);
@@ -5397,8 +5417,18 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
   is_elf64 = elf_64_file_p (filename);
   lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets;
 
+  /* We'll only try this once per inferior.  */
   if (priv->r_debug == 0)
-    priv->r_debug = get_r_debug (pid, is_elf64);
+    {
+      priv->r_debug = get_r_debug_dynamic (pid, is_elf64);
+
+      /* If there's no DT_DEBUG, r_debug will now be -1.  */
+    }
+
+  /* This may be a static executable.  Look for the symbol
+     conventionally named _r_debug, as a last resort.  */
+  if (priv->r_debug == (CORE_ADDR) -1 && priv->r_debug_from_syms != 0)
+    priv->r_debug = priv->r_debug_from_syms;
 
   if (priv->r_debug == (CORE_ADDR) -1 || priv->r_debug == 0)
     {
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 677d261..d6f169e 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -57,8 +57,14 @@ struct process_info_private
      thread_db, and it is active.  */
   struct thread_db *thread_db;
 
-  /* &_r_debug.  0 if not yet determined.  -1 if no PT_DYNAMIC in Phdrs.  */
+  /* &_r_debug.  0 if not yet determined.  -1 if no PT_DYNAMIC in
+     Phdrs, and not found by querying GDB's symbols for _r_debug's
+     address either.  */
   CORE_ADDR r_debug;
+
+  /* Fallback &_r_debug, from qSymbols, for static executables.  0 if
+     not asked/found yet.  */
+  CORE_ADDR r_debug_from_syms;
 };
 
 struct lwp_info;