public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
* Improved linker-debugger interface
@ 2011-05-25 15:37 Gary Benson
  2011-05-25 18:21 ` Tom Tromey
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Gary Benson @ 2011-05-25 15:37 UTC (permalink / raw)
  To: archer

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

Hi all,

These past few weeks I've been working on the following bug:

  https://bugzilla.redhat.com/show_bug.cgi?id=658851
  aka http://sources.redhat.com/bugzilla/show_bug.cgi?id=2328
  "_dl_debug_state() RT_CONSISTENT called too early"

and trying to figure out a fix that also addresses this:

  https://bugzilla.redhat.com/show_bug.cgi?id=698001
  "improve GDB performance on an application performing
  a lot of object loading."

and this:

  http://sourceware.org/bugzilla/show_bug.cgi?id=11839
  "gdb does not detect calls to dlmopen"

It's taken me a few tries, but I think I finally have an interface
that will work.  This mail is so I can run it past a wider audience
before I start implementing the gdb side in earnest.

The current linker-debugger interface has a structure (r_debug)
containing a list of loaded libraries, and an empty function
(_dl_debug_state) which the linker calls both before and after
modifying this list.  It exists solely for the debugger to set
a breakpoint on.

  - There is one place where glibc calls _dl_debug_state earlier than
    Solaris libc.  This is #658851.  It is unlikely that glibc will
    ever be changed to make it compatible with Solaris libc as this
    could break compatibility with previous glibcs.

  - This interface was presumably invented before dlmopen() was, so
    there's only provision in it for one namespace.  In glibc each
    namespace has it's own r_debug structure, but there is no way for
    the linker to communicate the addresses of the others to the
    debugger.  This is PR 11839.

  - In normal use gdb only needs to stop _after_ the list is modified.
    Because _dl_debug_state is called both before and after, gdb stops
    twice as often as it needs to.  This is #698001, the gist of it at
    any rate.  
    
  - When stop-on-solib-events is set, however, it is either necessary
    or at least useful to stop both before and after library loads.
    I'm not 100% sure on this point, but if nothing else it preserves
    the existing behaviour, and without it it probably becomes a lot
    more difficult to debug linker issues and possibly other stuff too.

The solution I'm proposing is this:

  - Add a new function for gdb to break on, _dl_debug_state_extended,
    to supplement _dl_debug_state.  Everywhere that _dl_debug_state is
    called, _dl_debug_state_extended is also called.  In the case
    where glibc calls _dl_debug_state earlier than Solaris libc we
    move the call to _dl_debug_state_extended to the Solaris location.
    This fixes #658851, with the caveat that some provision must be
    made for halting before STT_GNU_IFUNC relocations.

  - _dl_debug_state_extended contains SystemTap probes, currently two,
    but extendable to as many as we want.  gdb will look for these
    probes, and if it finds them it will set breakpoints on the ones
    it is interested in, falling back to breaking on _dl_debug_state
    if the required probes are not found.  The two current probes are
    r_debug_mod_starting (called before the list is modified) and
    r_debug_mod_complete (called after).  gdb will always stop on
    r_debug_mod_complete, to update its internal lists, but it will
    only stop on r_debug_mod_starting when stop-on-solib-events is
    set.  This halves the number of stops, addressing #698001.

  - All probes have as namespace id and r_debug address arguments,
    allowing gdb to discover namespaces other than the default.  This
    opens the door to fixing PR 11839.

I've attached a patch for glibc that implements its side of the
interface, designed to apply after the SystemTap support that was
added for Fedora 15, so if what I've written doesn't make sense then
maybe at least the code will!

Does this all seem ok?

Cheers,
Gary

-- 
http://gbenson.net/

[-- Attachment #2: glibc.patch --]
[-- Type: text/plain, Size: 5325 bytes --]

diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index d040590..226904e 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -958,6 +958,11 @@ extern void _dl_sort_fini (struct link_map *l, struct link_map **maps,
 extern void _dl_debug_state (void);
 rtld_hidden_proto (_dl_debug_state)
 
+/* The dynamic linker calls this function after having changed any
+   shared object mappings.  */
+extern void _dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
+     internal_function;
+
 /* 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.  */
diff --git a/elf/dl-debug.c b/elf/dl-debug.c
index 2538364..84cc163 100644
--- a/elf/dl-debug.c
+++ b/elf/dl-debug.c
@@ -76,3 +76,19 @@ _dl_debug_state (void)
 {
 }
 rtld_hidden_def (_dl_debug_state)
+
+
+/* This function exists solely to contain SystemTap probes which the
+   debugger can set breakpoints on.  */
+void
+_dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
+{
+  if (r->r_state == RT_CONSISTENT)
+    {
+      LIBC_PROBE (r_debug_mod_complete, 2, ns, r);
+    }
+  else
+    {
+      LIBC_PROBE (r_debug_mod_starting, 2, ns, r);
+    }
+}
diff --git a/elf/rtld.c b/elf/rtld.c
index 174954b..004060b 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1658,6 +1658,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
   /* We start adding objects.  */
   r->r_state = RT_ADD;
   _dl_debug_state ();
+  _dl_debug_state_extended (LM_ID_BASE, r);
 
   /* Auditing checkpoint: we are ready to signal that the initial map
      is being constructed.  */
@@ -2361,6 +2362,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
   r = _dl_debug_initialize (0, LM_ID_BASE);
   r->r_state = RT_CONSISTENT;
   _dl_debug_state ();
+  _dl_debug_state_extended (LM_ID_BASE, r);
 
 #ifndef MAP_COPY
   /* We must munmap() the cache file.  */
diff --git a/elf/dl-open.c b/elf/dl-open.c
index cf8e8cc..b487203 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -293,7 +293,9 @@ dl_open_worker (void *a)
     }
 #endif
 
-  /* Notify the debugger all new objects are now ready to go.  */
+  /* Notify the debugger all new objects are now ready to go.
+     Note that the corresponding _dl_debug_state_extended call
+     to this happens further on, after relocation is complete.  */
   struct r_debug *r = _dl_debug_initialize (0, args->nsid);
   r->r_state = RT_CONSISTENT;
   _dl_debug_state ();
@@ -460,6 +462,9 @@ cannot load any more object with static TLS"));
     _dl_fatal_printf (N_("\
 TLS generation counter wrapped!  Please report this."));
 
+  /* Notify the debugger all new objects have been relocated.  */
+  _dl_debug_state_extended (args->nsid, r);
+  
   /* Run the initializer functions of new objects.  */
   _dl_init (new, args->argc, args->argv, args->env);
 
diff --git a/elf/dl-close.c b/elf/dl-close.c
index efb2b58..49a3d07 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -471,6 +471,7 @@ _dl_close_worker (struct link_map *map)
   struct r_debug *r = _dl_debug_initialize (0, nsid);
   r->r_state = RT_DELETE;
   _dl_debug_state ();
+  _dl_debug_state_extended (nsid, r);
 
   if (unload_global)
     {
@@ -724,6 +725,7 @@ _dl_close_worker (struct link_map *map)
   /* Notify the debugger those objects are finalized and gone.  */
   r->r_state = RT_CONSISTENT;
   _dl_debug_state ();
+  _dl_debug_state_extended (nsid, r);
 
   /* Recheck if we need to retry, release the lock.  */
  out:
diff --git a/elf/dl-load.c b/elf/dl-load.c
index f866066..7588ddf 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -793,7 +793,7 @@ _dl_init_paths (const char *llp)
 static void
 __attribute__ ((noreturn, noinline))
 lose (int code, int fd, const char *name, char *realname, struct link_map *l,
-      const char *msg, struct r_debug *r)
+      const char *msg, struct r_debug *r, Lmid_t nsid)
 {
   /* The file might already be closed.  */
   if (fd != -1)
@@ -805,6 +805,7 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
     {
       r->r_state = RT_CONSISTENT;
       _dl_debug_state ();
+      _dl_debug_state_extended (nsid, r);
     }
 
   _dl_signal_error (code, name, NULL, msg);
@@ -843,7 +844,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
       errval = errno;
     call_lose:
       lose (errval, fd, name, realname, l, errstring,
-	    make_consistent ? r : NULL);
+	    make_consistent ? r : NULL, nsid);
     }
 
   /* Look again to see if the real name matched another already loaded.  */
@@ -950,6 +951,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
 	 linking has not been used before.  */
       r->r_state = RT_ADD;
       _dl_debug_state ();
+      _dl_debug_state_extended (nsid, r);
       make_consistent = true;
     }
   else
@@ -1645,7 +1647,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
 	      name = strdupa (realname);
 	      free (realname);
 	    }
-	  lose (errval, fd, name, NULL, NULL, errstring, NULL);
+	  lose (errval, fd, name, NULL, NULL, errstring, NULL, 0);
 	}
 
       /* See whether the ELF header is what we expect.  */

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

* Re: Improved linker-debugger interface
  2011-05-25 15:37 Improved linker-debugger interface Gary Benson
@ 2011-05-25 18:21 ` Tom Tromey
  2011-05-26 17:02   ` Gary Benson
  2011-05-26 17:37 ` Paul Pluzhnikov
  2011-06-07 16:58 ` Jan Kratochvil
  2 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2011-05-25 18:21 UTC (permalink / raw)
  To: archer

Gary> I've attached a patch for glibc that implements its side of the
Gary> interface, designed to apply after the SystemTap support that was
Gary> added for F15, so if what I've written doesn't make sense then maybe
Gary> at least the code will!

Gary> Does this all seem ok?

It makes sense as far as I understand it.  I don't know glibc very well.
I'd suggest asking Andreas Schwab for a review.

Gary> +void
Gary> +_dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
Gary> +{
Gary> +  if (r->r_state == RT_CONSISTENT)
Gary> +    {
Gary> +      LIBC_PROBE (r_debug_mod_complete, 2, ns, r);
Gary> +    }
Gary> +  else
Gary> +    {
Gary> +      LIBC_PROBE (r_debug_mod_starting, 2, ns, r);
Gary> +    }

Will 'ns' give gdb enough information to do its work in the dlmopen
case?  Or will we want gdb to have access to some other thing?  I am
wondering Lmid_t is just some internal-to-glibc cookie, where gdb will
actually want a pointer to some ld.so data structure.  I don't actually
know anything about this though.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-25 18:21 ` Tom Tromey
@ 2011-05-26 17:02   ` Gary Benson
  2011-05-26 17:25     ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Gary Benson @ 2011-05-26 17:02 UTC (permalink / raw)
  To: archer

Tom Tromey wrote:
> Gary> I've attached a patch for glibc that implements its side of
> Gary> the interface, designed to apply after the SystemTap support
> Gary> that was added for F15, so if what I've written doesn't make
> Gary> sense then maybe at least the code will!
> 
> Gary> Does this all seem ok?
> 
> It makes sense as far as I understand it.  I don't know glibc very
> well.  I'd suggest asking Andreas Schwab for a review.

Cool, I'll mail him.

> Gary> +void
> Gary> +_dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
> Gary> +{
> Gary> +  if (r->r_state == RT_CONSISTENT)
> Gary> +    {
> Gary> +      LIBC_PROBE (r_debug_mod_complete, 2, ns, r);
> Gary> +    }
> Gary> +  else
> Gary> +    {
> Gary> +      LIBC_PROBE (r_debug_mod_starting, 2, ns, r);
> Gary> +    }
> 
> Will 'ns' give gdb enough information to do its work in the dlmopen
> case?  Or will we want gdb to have access to some other thing?  I am
> wondering Lmid_t is just some internal-to-glibc cookie, where gdb
> will actually want a pointer to some ld.so data structure.  I don't
> actually know anything about this though.

The way glibc handles multiple namespaces is to have a separate
r_debug structure for each one.  The problem is that the current
interface only has provision for exposing one of these to the
debugger, so gdb can only see the initial namespace.  The "r"
argument to the probe is the r_debug structure for the namespace
being modified.

The "ns" argument isn't internal (at least, I don't think it is).
It's the argument you pass to dlmopen to tell it which namespace
to load the library into:

  extern void *dlmopen (Lmid_t nsid, const char *file, int mode);

It might be internal, though.  There are two special values,
LM_ID_BASE (load into the initial namespace) and LM_ID_NEWLM
(create a new namespace and load into that).  It may be that
these are the only values external code should call.  I couldn't
see any obvious way of discovering the namespace id of a loaded
object so you could then load other things into it, but I haven't
looked exhaustively.

Anyway, the "ns" argument isn't enough for gdb to work on, but
the "r" argument is.  I put them both into the probe because
gdb will probably have to maintain some internal list and the
"ns" argument would be useful to track them against (and maybe
get displayed in a column in "info shared").

Cheers,
Gary

-- 
http://gbenson.net/

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

* Re: Improved linker-debugger interface
  2011-05-26 17:02   ` Gary Benson
@ 2011-05-26 17:25     ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2011-05-26 17:25 UTC (permalink / raw)
  To: archer

Gary> Anyway, the "ns" argument isn't enough for gdb to work on, but
Gary> the "r" argument is.  I put them both into the probe because
Gary> gdb will probably have to maintain some internal list and the
Gary> "ns" argument would be useful to track them against (and maybe
Gary> get displayed in a column in "info shared").

Perfect, thanks.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-25 15:37 Improved linker-debugger interface Gary Benson
  2011-05-25 18:21 ` Tom Tromey
@ 2011-05-26 17:37 ` Paul Pluzhnikov
  2011-05-26 17:47   ` Tom Tromey
  2011-05-31 16:25   ` Gary Benson
  2011-06-07 16:58 ` Jan Kratochvil
  2 siblings, 2 replies; 17+ messages in thread
From: Paul Pluzhnikov @ 2011-05-26 17:37 UTC (permalink / raw)
  To: archer

Gary,

On Wed, May 25, 2011 at 8:36 AM, Gary Benson <gbenson@redhat.com> wrote:

> The solution I'm proposing is this:

It's great that you are working on this, thanks!

You might wish to consider the following points, which (AFAICT) you are
not currently addressing.

1. Stripped binaries.

   There is a DT_DEBUG entry pointing to _dl_debug_state (set by ld-linux)

   You might want to have a new dynamic tag, pointing to
   _dl_debug_state_extended(), so the debugger would be able to track
   shared libs using the new mechanism even when everything is stripped.

2. In-process debuggers.

   There are many use cases for "self-aware" binaries. For example,
   google-perftools collects profiling info with stack traces, and getting
   a stack trace requires that you know which DSOs are loaded into the
   process.

   The current interface is very hostile to such debuggers, as
   _dl_debug_state

   A) is called directly by libc (so there is no way to interpose it), and
   B) compiles down to a single 'ret', so there is no way to place a patch
   on top of it.

   This leads to all kinds of suboptimal solutions (such as scanning
   /proc/self/maps; which doesn't work if /proc is not mounted).

   A patch to make _dl_debug_state indirect through r_debug.r_brk has
   been rejected:
   https://bugzilla.redhat.com/show_bug.cgi?id=70407

   Perhaps co-operation with "in-process" debuggers would be more acceptable
   for a new interface?


Thanks,
-- 
Paul Pluzhnikov

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

* Re: Improved linker-debugger interface
  2011-05-26 17:37 ` Paul Pluzhnikov
@ 2011-05-26 17:47   ` Tom Tromey
  2011-05-26 21:07     ` Tom Tromey
  2011-05-31 16:25   ` Gary Benson
  1 sibling, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2011-05-26 17:47 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: archer

Paul> 1. Stripped binaries.

Paul>    There is a DT_DEBUG entry pointing to _dl_debug_state (set by ld-linux)

Paul>    You might want to have a new dynamic tag, pointing to
Paul>    _dl_debug_state_extended(), so the debugger would be able to track
Paul>    shared libs using the new mechanism even when everything is stripped.

SystemTap probes aren't stripped, so I think this should work fine.
This is one of the big plusses (from my POV) of using probes, and it is
why we introduced probes into longjmp and the exception unwinder too --
it lets gdb features work even when the user hasn't installed debuginfo
for glibc and/or libgcc.

This stuff is in F15.  It wasn't accepted into glibc mainline.

Paul> 2. In-process debuggers.

Paul>    B) compiles down to a single 'ret', so there is no way to place a patch
Paul>    on top of it.

Right now a SystemTap probe is a single nop, so probably no joy there.
We have talked about making it a wider nop, though, to facilitate this
kind of patching, primarily I think by LTTng.  What do you think of
this?

Paul>    Perhaps co-operation with "in-process" debuggers would be more
Paul>    acceptable for a new interface?

It is fine by me.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-26 17:47   ` Tom Tromey
@ 2011-05-26 21:07     ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2011-05-26 21:07 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: archer

Tom> Right now a SystemTap probe is a single nop, so probably no joy there.
Tom> We have talked about making it a wider nop, though, to facilitate this
Tom> kind of patching, primarily I think by LTTng.  What do you think of
Tom> this?

Oh, and if it won't work, or if you need something extra, now is the
best time to write up your requirements for Gary's consideration.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-26 17:37 ` Paul Pluzhnikov
  2011-05-26 17:47   ` Tom Tromey
@ 2011-05-31 16:25   ` Gary Benson
  2011-05-31 19:46     ` Tom Tromey
  1 sibling, 1 reply; 17+ messages in thread
From: Gary Benson @ 2011-05-31 16:25 UTC (permalink / raw)
  To: archer; +Cc: Paul Pluzhnikov

Paul Pluzhnikov wrote:
> On Wed, May 25, 2011 at 8:36 AM, Gary Benson <gbenson@redhat.com> wrote:
> > The solution I'm proposing is this:
> 
> It's great that you are working on this, thanks!

No problem!

> 1. Stripped binaries.
> 
>    There is a DT_DEBUG entry pointing to _dl_debug_state (set by
>    ld-linux)
> 
>    You might want to have a new dynamic tag, pointing to
>    _dl_debug_state_extended(), so the debugger would be able
>    to track shared libs using the new mechanism even when
>    everything is stripped.

Like Tom said, I'm using SystemTap probes within the function rather
than looking up the symbol.  Having more than one probe in there
allows gdb to be more selective about when it stops (and therefore
stop less often).

Is it possible for in-process debuggers to locate SystemTap probes?
Forgive me if this is an obvious question but I'm new to this :)

> 2. In-process debuggers.
> 
>    There are many use cases for "self-aware" binaries. For example,
>    google-perftools collects profiling info with stack traces, and
>    getting a stack trace requires that you know which DSOs are
>    loaded into the process.
> 
>    The current interface is very hostile to such debuggers, as
>    _dl_debug_state
> 
>    A) is called directly by libc (so there is no way to interpose
>    it), and
>    B) compiles down to a single 'ret', so there is no way to place
>    a patch on top of it.
> 
>    This leads to all kinds of suboptimal solutions (such as scanning
>    /proc/self/maps; which doesn't work if /proc is not mounted).
> 
>    A patch to make _dl_debug_state indirect through r_debug.r_brk
>    has been rejected:
>    https://bugzilla.redhat.com/show_bug.cgi?id=70407
> 
>    Perhaps co-operation with "in-process" debuggers would be more
>    acceptable for a new interface?

Is making _dl_debug_state_extended an indirect call the only non-hacky
way to allow in-process debuggers to get these notifications, or are
there other possibilities?

By the way, although _dl_debug_state compiles to a single ret, I think
function entries are aligned so you might get a few more bytes to work
with on some platforms.  Obviously this doesn't negate the need for a
proper interface, but I noticed it earlier and wondered if it might
help you.

Also by the way, glibc has what it calls "auditing checkpoints" in
various places, I don't know if these are something you can use?

Thanks,
Gary

-- 
http://gbenson.net/

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

* Re: Improved linker-debugger interface
  2011-05-31 16:25   ` Gary Benson
@ 2011-05-31 19:46     ` Tom Tromey
  2011-05-31 20:41       ` Paul Pluzhnikov
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2011-05-31 19:46 UTC (permalink / raw)
  To: archer; +Cc: Paul Pluzhnikov

Gary> Is it possible for in-process debuggers to locate SystemTap probes?

The probes are described in an non-allocated section, so IIUC they
aren't mapped by ld.so.  I guess a program could examine the executables
and libraries it uses and map them itself.

I am not sure how LLTng plans to address this.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-31 19:46     ` Tom Tromey
@ 2011-05-31 20:41       ` Paul Pluzhnikov
  2011-05-31 20:46         ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Pluzhnikov @ 2011-05-31 20:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: archer

On Tue, May 31, 2011 at 12:46 PM, Tom Tromey <tromey@redhat.com> wrote:
> Gary> Is it possible for in-process debuggers to locate SystemTap probes?
>
> The probes are described in an non-allocated section, so IIUC they
> aren't mapped by ld.so.  I guess a program could examine the executables
> and libraries it uses and map them itself.

Yes, the in-process debugger usually will try to open the ELF file (e.g. to
get to .debug_*, etc.), so it looks like locating the SystemTap probes
would work fine.

On Tue, May 31, 2011 at 9:24 AM, Gary Benson <gbenson@redhat.com> wrote:

> Is making _dl_debug_state_extended an indirect call the only non-hacky
> way to allow in-process debuggers to get these notifications, or are
> there other possibilities?

Once the probe is located, the in-process debugger still has a problem:
not enough space to place a patch on (in-process debugger can not put a
breakpoint on the one instruction that is available).

Making the probes wider would be ideal.

> By the way, although _dl_debug_state compiles to a single ret, I think
> function entries are aligned so you might get a few more bytes to work
> with on some platforms.

I believe I've seen some *86 builds of glibc which did not align functions.

-- 
Paul Pluzhnikov

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

* Re: Improved linker-debugger interface
  2011-05-31 20:41       ` Paul Pluzhnikov
@ 2011-05-31 20:46         ` Tom Tromey
  2011-05-31 21:05           ` Paul Pluzhnikov
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2011-05-31 20:46 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: archer

Paul> Once the probe is located, the in-process debugger still has a problem:
Paul> not enough space to place a patch on (in-process debugger can not put a
Paul> breakpoint on the one instruction that is available).

Paul> Making the probes wider would be ideal.

I asked and the SystemTap guys said that they'd welcome a patch, but
that the widening would have to be optional.  (There are already a
couple of knobs in sdt.h, controlled by some #define.)

Tom

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

* Re: Improved linker-debugger interface
  2011-05-31 20:46         ` Tom Tromey
@ 2011-05-31 21:05           ` Paul Pluzhnikov
  2011-06-02 23:56             ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Pluzhnikov @ 2011-05-31 21:05 UTC (permalink / raw)
  To: Tom Tromey; +Cc: archer

On Tue, May 31, 2011 at 1:46 PM, Tom Tromey <tromey@redhat.com> wrote:

> Paul> Making the probes wider would be ideal.
>
> I asked and the SystemTap guys said that they'd welcome a patch, but
> that the widening would have to be optional.

Optional widening would likely be useless in this context: an in-process
debugger wants to just work (TM). Requiring a special build of ld-linux
(with widened probes) is the opposite of that ;-(

One could hope that upstream glibc would adopt wide probes by default,
but I would guess that chances of that actually happening are nil.

-- 
Paul Pluzhnikov

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

* Re: Improved linker-debugger interface
  2011-05-31 21:05           ` Paul Pluzhnikov
@ 2011-06-02 23:56             ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2011-06-02 23:56 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: archer

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Tom> I asked and the SystemTap guys said that they'd welcome a patch, but
Tom> that the widening would have to be optional.

Paul> Optional widening would likely be useless in this context: an in-process
Paul> debugger wants to just work (TM). Requiring a special build of ld-linux
Paul> (with widened probes) is the opposite of that ;-(

Paul> One could hope that upstream glibc would adopt wide probes by default,
Paul> but I would guess that chances of that actually happening are nil.

I think we can make these probes wide and other ones in glibc narrow.

However, you may still have difficulties, since I believe upstream glibc
will probably not accept the SystemTap probes at all.  They have been
rejected so far, aside from Roland's branch.  We do ship them in Fedora,
though.

Tom

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

* Re: Improved linker-debugger interface
  2011-05-25 15:37 Improved linker-debugger interface Gary Benson
  2011-05-25 18:21 ` Tom Tromey
  2011-05-26 17:37 ` Paul Pluzhnikov
@ 2011-06-07 16:58 ` Jan Kratochvil
  2011-06-08 10:56   ` Gary Benson
  2 siblings, 1 reply; 17+ messages in thread
From: Jan Kratochvil @ 2011-06-07 16:58 UTC (permalink / raw)
  To: archer; +Cc: Gary Benson

On Wed, 25 May 2011 17:36:50 +0200, Gary Benson wrote:
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
[...]
> +/* The dynamic linker calls this function after having changed any
> +   shared object mappings.  */
> +extern void _dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
> +     internal_function;
> +
[...]
> --- a/elf/dl-debug.c
> +++ b/elf/dl-debug.c
> @@ -76,3 +76,19 @@ _dl_debug_state (void)
>  {
>  }
>  rtld_hidden_def (_dl_debug_state)
> +
> +
> +/* This function exists solely to contain SystemTap probes which the
> +   debugger can set breakpoints on.  */

Here should be also `internal_function' otherwise on i686 it will:

dl-debug.c:84:1: error: conflicting types for '_dl_debug_state_extended'
../sysdeps/generic/ldsodefs.h:963:13: note: previous declaration of '_dl_debug_state_extended' was here


> +void
> +_dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
> +{
> +  if (r->r_state == RT_CONSISTENT)
> +    {
> +      LIBC_PROBE (r_debug_mod_complete, 2, ns, r);
> +    }
> +  else
> +    {
> +      LIBC_PROBE (r_debug_mod_starting, 2, ns, r);
> +    }
> +}


Thanks,
Jan

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

* Re: Improved linker-debugger interface
  2011-06-07 16:58 ` Jan Kratochvil
@ 2011-06-08 10:56   ` Gary Benson
  0 siblings, 0 replies; 17+ messages in thread
From: Gary Benson @ 2011-06-08 10:56 UTC (permalink / raw)
  To: archer

Jan Kratochvil wrote:
> On Wed, 25 May 2011 17:36:50 +0200, Gary Benson wrote:
> > --- a/sysdeps/generic/ldsodefs.h
> > +++ b/sysdeps/generic/ldsodefs.h
> [...]
> > +/* The dynamic linker calls this function after having changed any
> > +   shared object mappings.  */
> > +extern void _dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
> > +     internal_function;
> > +
> [...]
> > --- a/elf/dl-debug.c
> > +++ b/elf/dl-debug.c
> > @@ -76,3 +76,19 @@ _dl_debug_state (void)
> >  {
> >  }
> >  rtld_hidden_def (_dl_debug_state)
> > +
> > +
> > +/* This function exists solely to contain SystemTap probes which the
> > +   debugger can set breakpoints on.  */
> 
> Here should be also `internal_function' otherwise on i686 it will:
> 
> dl-debug.c:84:1: error: conflicting types for '_dl_debug_state_extended'
> ../sysdeps/generic/ldsodefs.h:963:13: note: previous declaration of '_dl_debug_state_extended' was here
> 
> > +void
> > +_dl_debug_state_extended (Lmid_t ns, struct r_debug *r)
...

Thanks Jan.

Cheers,
Gary

-- 
http://gbenson.net/

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

* Re: Improved linker-debugger interface
  2011-06-01  1:24 Frank Ch. Eigler
@ 2011-06-02 23:56 ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2011-06-02 23:56 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: archer, ppluzhnikov

Frank> It depends.  If it's out of the hot path, a five or eight-byte noop
Frank> may be OK.  Measurements as to the actual impact would help.

Considering that this is in dlopen, I am not concerned.
It is already making dummy function calls along this path, also for
debuggers.

Tom

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

* Re: Improved linker-debugger interface
@ 2011-06-01  1:24 Frank Ch. Eigler
  2011-06-02 23:56 ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Ch. Eigler @ 2011-06-01  1:24 UTC (permalink / raw)
  To: archer, ppluzhnikov

Hi -

ppluzhnikov wrote:

> [...] One could hope that upstream glibc would adopt wide probes by
> default, but I would guess that chances of that actually happening
> are nil.

It depends.  If it's out of the hot path, a five or eight-byte noop
may be OK.  Measurements as to the actual impact would help.

- FChE

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

end of thread, other threads:[~2011-06-08 10:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 15:37 Improved linker-debugger interface Gary Benson
2011-05-25 18:21 ` Tom Tromey
2011-05-26 17:02   ` Gary Benson
2011-05-26 17:25     ` Tom Tromey
2011-05-26 17:37 ` Paul Pluzhnikov
2011-05-26 17:47   ` Tom Tromey
2011-05-26 21:07     ` Tom Tromey
2011-05-31 16:25   ` Gary Benson
2011-05-31 19:46     ` Tom Tromey
2011-05-31 20:41       ` Paul Pluzhnikov
2011-05-31 20:46         ` Tom Tromey
2011-05-31 21:05           ` Paul Pluzhnikov
2011-06-02 23:56             ` Tom Tromey
2011-06-07 16:58 ` Jan Kratochvil
2011-06-08 10:56   ` Gary Benson
2011-06-01  1:24 Frank Ch. Eigler
2011-06-02 23:56 ` Tom Tromey

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).