[7.4 regression] Stand-alone Cell debugging broken (Re: RFC: don't set the pspace on ordinary breakpoints)

["Ulrich Weigand" <uweigand@de.ibm.com>]

I wrote:
> Tom Tromey wrote:
> > >>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:
> > 
> > Ulrich> Sorry for the delay; I wanted to test your patch on Cell to make
> > Ulrich> sure it doesn't regress in the scenario bp_startup_disabled was
> > Ulrich> introduced to fix.
> > 
> > It is no problem.
> > 
> > Ulrich> Unfortunately, I've been blocked by a number of SPU regressions
> > Ulrich> that make results on Cell currently bad anyway, see e.g.
> > Ulrich> http://sourceware.org/ml/gdb-patches/2011-11/msg00437.html
> > Ulrich> http://sourceware.org/ml/gdb-patches/2011-11/msg00439.html
> > 
> > Ulrich> I'm still planning on doing the run once SPU is stable again.
> > 
> > Ok.  I will wait for that then.
> 
> OK, I've now completed a run with your patch in the situation where
> bp_startup_disabled was necessary (stand-alone SPU executables on
> a Cell/B.E. system).   Everything still works fine.

While this patch by itself didn't break stand-alone SPU debugging,
the whole "ambiguous linespec" series in total apparently did.

What happens is that if I set a breakpoint on "main", and then start
execution, the first time breakpoints are re-set, addr_string_to_sals
(as called from breakpoint_re_set_default) correctly recognizes that
even though "main" cannot be found at this moment, this is OK since
the address space is currently going through startup.

However, while this means that no error is thrown at this point,
addr_string_to_sals still returns an empty location list, and the
existing list of locations for the "main" breakpoint now gets thrown
away in update_breakpoint_locations.

This in turn means that the *next* time breakpoints are re-set, the
"main" breakpoint now no longer has any locations associated to it,
and therefore it is no longer recognized as being related to an
address space going through startup.

Thus, an error is thrown and the breakpoint is marked disabled, and
therefore once startup is completed and "main" now *could* be found
again, it is no longer even attempted to insert the breakpoint again.

This is a serious regression that bascially means debugging stand-
alone SPU programs with a Cell/B.E. debugger is impossible.  Thus
it would be really good to get this fixed before 7.4 is released ...

The following patch fixes the problem for me.  It hooks into an
existing mechanism to fix a similar problem for breakpoints in
shared libraries that are being unloaded:  update_breakpoint_locations
does:

  /* If there's no new locations, and all existing locations are
     pending, don't do anything.  This optimizes the common case where
     all locations are in the same shared library, that was unloaded.
     We'd like to retain the location, so that when the library is
     loaded again, we don't loose the enabled/disabled status of the
     individual locations.  */
  if (all_locations_are_pending (existing_locations) && sals.nelts == 0)
    return;

which has the effect to keep locations from such libraries present
even though the library is unloaded.  The patch below now extends
this to the executing_startup case by having all_locations_are_pending
consider locations in address spaces currently going through startup
as "pending" as well.

With this change, the breakpoint location is kept active until startup
is complete, and then replaced with the correct location.


Does this look reasonable?  Joel, would this be OK for 7.4 as well?

Thanks,
Ulrich


ChangeLog:

	* breakpoint.c (all_locations_are_pending): Consider locations
	in program spaces executing during startup pending as well.

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.633.2.2
diff -u -p -r1.633.2.2 breakpoint.c
--- gdb/breakpoint.c	23 Dec 2011 17:55:20 -0000	1.633.2.2
+++ gdb/breakpoint.c	2 Jan 2012 15:16:20 -0000
@@ -11500,7 +11500,8 @@ static int
 all_locations_are_pending (struct bp_location *loc)
 {
   for (; loc; loc = loc->next)
-    if (!loc->shlib_disabled)
+    if (!loc->shlib_disabled
+	&& !loc->pspace->executing_startup)
       return 0;
   return 1;
 }


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com