Re: gdb side of improved linker-debugger interface

[Tom Tromey <tromey@redhat.com>]

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> A week or so ago I mailed this list a description of a new linker-
Gary> debugger interface based on SystemTap probes, along with a patch
Gary> containing the glibc side:
Gary>   http://www.cygwin.com/ml/archer/2011-q2/msg00000.html

If this is ready then the next step is to try to get it into glibc.

I don't think it can go into glibc mainline, but perhaps we can get it
onto Roland's branch.

One thing that would be useful is a write-up explaining the issues and
why this approach is correct.  What I recall from some bug (I don't
recall which one) on the topic is that Ulrich was of the opinion that
the debug hook was in the right spot and that GDB was wrong.  It would
be good to have a rebuttal for this position in particular.

I'm not sure what the protocol is here -- email to glibc-alpha, or just
to Roland.  glibc-alpha will likely get you flamed, don your protective
gear.

Gary> I've seen mention that the RT_CONSISTENT called too early bug was the
Gary> cause of another issue where libthread_db would not be loaded if
Gary> libpthread was loaded with dlopen rather than being linked in, but I
Gary> haven't been able to discover if this really was ever the case.  If
Gary> anybody knows (or has a testcase) could you fill me in on it please.

I failed in my attempt to make a simple reproducer.
I know David Malcolm had this reliably fail with his Python plugin for
GCC; so you might try that, though it seems rather large for a test case.

Gary> +  /* SystemTap probes.  */
Gary> +  const struct stap_probe *pre_mod_probe;
Gary> +  const struct stap_probe *post_mod_probe;

You are going to have to merge or rebase and update this a bit.
I changed find_probe_in_objfile on the branch, to account for probes
with multiple locations.

Once the patch is ready I think it should go on archer-sergiodj-stap and
then on origin/archer-sergiodj-stap-patch-split as the last patch in the
series.

Gary> +static int
Gary> +svr4_update_solib_event_breakpoint (struct breakpoint *b, void *arg)
Gary> +{
Gary> +  struct svr4_info *info = get_svr4_info ();
Gary> +  struct bp_location *loc;
Gary> +
Gary> +  for (loc = b->loc; loc; loc = loc->next)
Gary> +    {
Gary> +      if (loc->pspace == current_program_space
Gary> +	  && loc->address == info->pre_mod_probe->address)
Gary> +	{
Gary> +	  b->enable_state = stop_on_solib_events;

This should have a constant from `enum enable_state' on the RHS.

Gary> -	  create_solib_event_breakpoint (target_gdbarch, sym_addr);
Gary> +	  svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);

I don't know this code very well, but it seems to me that it is looking
at different ways to set the solib breakpoints.  Your current code runs
at the very end -- GDB still tries the existing ways to find the
breakpoint locations, and then, if one is found, instead inserts at the
probe point if that is found.

Assuming this reading is correct, it seems like it would be better to
just check for SystemTap probe points early in enable_break, and then
prefer those to the other methods.

Tom