patch: solib_break from _r_debug.r_brk

patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

Hello,

I have a case where gdb can not find solib break function (in our case 
_dl_debug_state) in cases where dynamic linker library is stripped.

This patch adds new fallback method of determining solib_break address 
by using _r_debug symbol and r_brk field from it.

Test suite did not show regressions.


Thank you,

Aleksandar



ChangeLog:

<date>  Aleksandar Ristovski  <aristovski@qnx.com>

     * solib-svr4.c (svr4_fetch_solib_break_from_r_debug): New.
     (enable_break): Use new function.


[-- Attachment #2: solib_break-from-r_brk-201109290930.patch --]
[-- Type: text/x-patch, Size: 2131 bytes --]

Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.154
diff -u -p -r1.154 solib-svr4.c
--- gdb/solib-svr4.c	30 Aug 2011 02:48:05 -0000	1.154
+++ gdb/solib-svr4.c	29 Sep 2011 13:31:00 -0000
@@ -1181,6 +1181,39 @@ cmp_name_and_sec_flags (asymbol *sym, vo
   return (strcmp (sym->name, (const char *) data) == 0
 	  && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0);
 }
+
+/*  Use dynamic linker bfd and try to figure out solib break
+    address using _r_debug.r_brk.  It is expected that at this point
+    it is unrelocated filled with relative address of solib
+    break function.  This case may happen if solib break function
+    is defined as static in the dynamic linker, and dynmic linker
+    library is completely stripped.  */
+
+static CORE_ADDR
+svr4_fetch_solib_break_from_r_debug (bfd *const tmp_bfd,
+				     const CORE_ADDR load_addr)
+{
+  const CORE_ADDR r_debug_sym_addr
+    = gdb_bfd_lookup_symbol (tmp_bfd, cmp_name_and_sec_flags,
+			     (void *) "_r_debug");
+  const struct link_map_offsets *const lmo = svr4_fetch_link_map_offsets ();
+  const int ptrsz = gdbarch_ptr_bit (target_gdbarch);
+  gdb_byte r_brk_addr[ptrsz];
+
+  if (target_read_memory (load_addr + r_debug_sym_addr + lmo->r_brk_offset,
+			  r_brk_addr, ptrsz) == 0)
+    {
+      const enum bfd_endian byte_order
+	= gdbarch_byte_order (target_gdbarch);
+
+      return extract_unsigned_integer (r_brk_addr, ptrsz, byte_order);
+    }
+  return 0;
+}
+
+
+
+
 /* Arrange for dynamic linker to hit breakpoint.
 
    Both the SunOS and the SVR4 dynamic linkers have, as part of their
@@ -1435,6 +1468,11 @@ enable_break (struct svr4_info *info, in
 	    break;
 	}
 
+      /* Failing the above methods of locating debug base, use
+	 _r_debug.r_brk structure.  */
+      if (sym_addr == 0 && load_addr_found)
+	sym_addr = svr4_fetch_solib_break_from_r_debug (tmp_bfd, load_addr);
+
       if (sym_addr != 0)
 	/* Convert 'sym_addr' from a function pointer to an address.
 	   Because we pass tmp_bfd_target instead of the current

Re: patch: solib_break from _r_debug.r_brk

[Marek Polacek]

On 09/29/2011 03:55 PM, Aleksandar Ristovski wrote:
> +    break function.  This case may happen if solib break function
> +    is defined as static in the dynamic linker, and dynmic linker

s/dynmic/dynamic/

Re: patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

On 11-09-29 10:00 AM, Marek Polacek wrote:
> On 09/29/2011 03:55 PM, Aleksandar Ristovski wrote:
>> +    break function.  This case may happen if solib break function
>> +    is defined as static in the dynamic linker, and dynmic linker
>
> s/dynmic/dynamic/
>

Fixed. Thanks. (the rest of the patch remains the same so not re-posting).

Re: patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

I have to re-post the patch afterall.

In recoding it from my internal repository to current HEAD, I replaced 
hard coded pointer size with the wrong function: gdbarch_ptr_bit, but 
what I really wanted is simply target pointer size:

+  const unsigned ptrsz
+    = builtin_type (target_gdbarch)->builtin_func_ptr->length;

Thanks,

Aleksandar


Change log is still the same:

<date>  Aleksandar Ristovski  <aristovski@qnx.com>

         * solib-svr4.c (svr4_fetch_solib_break_from_r_debug): New.
         (enable_break): Use new function.

[-- Attachment #2: solib_break-from-r_brk-201109291101.patch --]
[-- Type: text/x-patch, Size: 2165 bytes --]

Index: gdb/solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.154
diff -u -p -r1.154 solib-svr4.c
--- gdb/solib-svr4.c	30 Aug 2011 02:48:05 -0000	1.154
+++ gdb/solib-svr4.c	29 Sep 2011 15:01:36 -0000
@@ -1181,6 +1181,40 @@ cmp_name_and_sec_flags (asymbol *sym, vo
   return (strcmp (sym->name, (const char *) data) == 0
 	  && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0);
 }
+
+/*  Use dynamic linker bfd and try to figure out solib break
+    address using _r_debug.r_brk.  It is expected that at this point
+    it is unrelocated filled with relative address of solib
+    break function.  This case may happen if solib break function
+    is defined as static in the dynamic linker, and dynamic linker
+    library is completely stripped.  */
+
+static CORE_ADDR
+svr4_fetch_solib_break_from_r_debug (bfd *const tmp_bfd,
+				     const CORE_ADDR load_addr)
+{
+  const CORE_ADDR r_debug_sym_addr
+    = gdb_bfd_lookup_symbol (tmp_bfd, cmp_name_and_sec_flags,
+			     (void *) "_r_debug");
+  const struct link_map_offsets *const lmo = svr4_fetch_link_map_offsets ();
+  const unsigned ptrsz
+    = builtin_type (target_gdbarch)->builtin_func_ptr->length;
+  gdb_byte r_brk_addr[ptrsz];
+
+  if (target_read_memory (load_addr + r_debug_sym_addr + lmo->r_brk_offset,
+			  r_brk_addr, ptrsz) == 0)
+    {
+      const enum bfd_endian byte_order
+	= gdbarch_byte_order (target_gdbarch);
+
+      return extract_unsigned_integer (r_brk_addr, ptrsz, byte_order);
+    }
+  return 0;
+}
+
+
+
+
 /* Arrange for dynamic linker to hit breakpoint.
 
    Both the SunOS and the SVR4 dynamic linkers have, as part of their
@@ -1435,6 +1469,11 @@ enable_break (struct svr4_info *info, in
 	    break;
 	}
 
+      /* Failing the above methods of locating debug base, use
+	 _r_debug.r_brk structure.  */
+      if (sym_addr == 0 && load_addr_found)
+	sym_addr = svr4_fetch_solib_break_from_r_debug (tmp_bfd, load_addr);
+
       if (sym_addr != 0)
 	/* Convert 'sym_addr' from a function pointer to an address.
 	   Because we pass tmp_bfd_target instead of the current

Re: patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

I will have to withdraw this patch.

The reason is that on gnu ld, this would not work since _r_debug is 
initialized differently, causing it to always read zero.

In theory, when attaching to a running process, this patch could still 
yield sensible results, but then I'm not sure it is generic enough to 
warrant inclusion in the official sources.


For completeness, here are two other things I have found further testing 
the patch on gnu/linux:

sym->section for _r_debug will be SEC_ALLOC so cmp_name_and_sec_flags 
would always return 0.

Second issue is that once the symbol is correctly found, fetched address 
would have to be checked for relocation since it could be already 
relocated in case of attaching to a running process.


Due to above, even though the patch works fine for me, I see no point 
pursuing it further for fsf gdb.


Thanks,

Aleksandar

Re: patch: solib_break from _r_debug.r_brk

[Tom Tromey]

>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:

Aleksandar> +  const unsigned ptrsz
Aleksandar> +    = builtin_type (target_gdbarch)->builtin_func_ptr->length;

You should use TYPE_LENGTH here, not ->length.

Aleksandar> <date>  Aleksandar Ristovski  <aristovski@qnx.com>
Aleksandar>         * solib-svr4.c (svr4_fetch_solib_break_from_r_debug): New.
Aleksandar>         (enable_break): Use new function.

I don't really know much about this area; I think it would be better for
someone else to review the patch.  Be sure to ping it weekly.  After a
decent interval without replies I will take a stab at it.

Tom

Re: patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

On 11-10-03 04:12 PM, Tom Tromey wrote:
>>>>>> "Aleksandar" == Aleksandar Ristovski<aristovski@qnx.com>  writes:
>
> Aleksandar>  +  const unsigned ptrsz
> Aleksandar>  +    = builtin_type (target_gdbarch)->builtin_func_ptr->length;
>
> You should use TYPE_LENGTH here, not ->length.
>
> Aleksandar>  <date>   Aleksandar Ristovski<aristovski@qnx.com>
> Aleksandar>          * solib-svr4.c (svr4_fetch_solib_break_from_r_debug): New.
> Aleksandar>          (enable_break): Use new function.
>
> I don't really know much about this area; I think it would be better for
> someone else to review the patch.  Be sure to ping it weekly.  After a
> decent interval without replies I will take a stab at it.
>


Tom, thanks for looking into this, but I have meanwhile done further 
testing on gnu/linux and uncovered that it is not worth pursuing. While 
it works for us, it doesn't on gnu/linux and I am not sure it can be 
made generic enough to defend it: 
http://sourceware.org/ml/gdb-patches/2011-10/msg00043.html

Sorry about that.

---
Aleksandar

Re: patch: solib_break from _r_debug.r_brk

[Tom Tromey]

>>>>> "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:

Aleksandar> Tom, thanks for looking into this, but I have meanwhile done
Aleksandar> further testing on gnu/linux and uncovered that it is not
Aleksandar> worth pursuing. While it works for us, it doesn't on
Aleksandar> gnu/linux and I am not sure it can be made generic enough to
Aleksandar> defend it:
Aleksandar> http://sourceware.org/ml/gdb-patches/2011-10/msg00043.html

Aleksandar> Sorry about that.

No problem.  Somehow I didn't see that message.

I think we can always make things work.  E.g., we could make this
OS-dependent, say via gdbarch.  Whether you want to take the time is
really up to you.

Tom

Re: patch: solib_break from _r_debug.r_brk

[Aleksandar Ristovski]

On 11-10-04 12:58 PM, Tom Tromey wrote:
>>>>>> "Aleksandar" == Aleksandar Ristovski<aristovski@qnx.com>  writes:
>
> Aleksandar>  Tom, thanks for looking into this, but I have meanwhile done
> Aleksandar>  further testing on gnu/linux and uncovered that it is not
> Aleksandar>  worth pursuing. While it works for us, it doesn't on
> Aleksandar>  gnu/linux and I am not sure it can be made generic enough to
> Aleksandar>  defend it:
> Aleksandar>  http://sourceware.org/ml/gdb-patches/2011-10/msg00043.html
>
> Aleksandar>  Sorry about that.
>
> No problem.  Somehow I didn't see that message.
>
> I think we can always make things work.  E.g., we could make this
> OS-dependent, say via gdbarch.  Whether you want to take the time is
> really up to you.
>
> Tom


I plan to revisit it. Thanks for the gdbarch suggestion.

---
Aleksandar

Re: patch: solib_break from _r_debug.r_brk

[Daniel Jacobowitz]

On Mon, Oct 3, 2011 at 4:37 PM, Aleksandar Ristovski <aristovski@qnx.com> wrote:
> Tom, thanks for looking into this, but I have meanwhile done further testing
> on gnu/linux and uncovered that it is not worth pursuing. While it works for
> us, it doesn't on gnu/linux and I am not sure it can be made generic enough
> to defend it: http://sourceware.org/ml/gdb-patches/2011-10/msg00043.html

FWIW (not much), I looked into this area a year ago also.  I thought
I'd posted a patch to use r_debug, but maybe not...  Solaris does
something clever in which unrelocated values are available at link
time, so the debugger can pick them up right away - as long as it can
tell if they've been relocated, I guess.  Linux just fills in zero
when it has the right value.

-- 
Thanks,
Daniel

Re: patch: solib_break from _r_debug.r_brk

[Jan Kratochvil]

On Thu, 29 Sep 2011 15:55:10 +0200, Aleksandar Ristovski wrote:
> I have a case where gdb can not find solib break function (in our
> case _dl_debug_state) in cases where dynamic linker library is
> stripped.

It would be an interesting patch but I do not see it useful on glibc:

$ readelf -Ws /lib64/ld-linux-x86-64.so.2
Symbol table '.dynsym' contains 28 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
    10: 0000000000223260    40 OBJECT  GLOBAL DEFAULT   21 _r_debug@@GLIBC_2.2.5
    18: 000000000000f610     2 FUNC    GLOBAL DEFAULT   11 _dl_debug_state@@GLIBC_PRIVATE

These symbols are the same category so if there is one, there will be both of
them.

Plus apparently `.dynsym' can never be `strip'ped.

So the patch may make sense for QNX and then one should just ensure it does
not break glibc.


Thanks,
Jan