patch: solib_break from _r_debug.r_brk

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
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  <[hidden email]>

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


solib_break-from-r_brk-201109290930.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Marek Polacek-2
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/
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
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).

Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
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  <[hidden email]>

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

solib_break-from-r_brk-201109291101.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
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

Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Tom Tromey
In reply to this post by Aleksandar Ristovski-2
>>>>> "Aleksandar" == Aleksandar Ristovski <[hidden email]> 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  <[hidden email]>
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
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
On 11-10-03 04:12 PM, Tom Tromey wrote:

>>>>>> "Aleksandar" == Aleksandar Ristovski<[hidden email]>  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<[hidden email]>
> 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


Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Tom Tromey
>>>>> "Aleksandar" == Aleksandar Ristovski <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Aleksandar Ristovski-2
On 11-10-04 12:58 PM, Tom Tromey wrote:

>>>>>> "Aleksandar" == Aleksandar Ristovski<[hidden email]>  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
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Daniel Jacobowitz-2
In reply to this post by Aleksandar Ristovski-2
On Mon, Oct 3, 2011 at 4:37 PM, Aleksandar Ristovski <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: patch: solib_break from _r_debug.r_brk

Jan Kratochvil-2
In reply to this post by Aleksandar Ristovski-2
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