[patch 3/3] bpstat_what removal [rediff]

Pedro Alves pedro@codesourcery.com
Wed Jun 16 19:13:00 GMT 2010


On Tuesday 15 June 2010 22:54:02, Jan Kratochvil wrote:
> On Tue, 15 Jun 2010 17:08:19 +0200, Pedro Alves wrote:
> > Do you think it would be hard to split this into smaller pieces?
> 
> Yes, I will do - just later.  IMO we should probably more talk about its goal
> than about specifics of the implementation.

I'll just do it now.

> > I've tried to review this a couple
> > of times already, but it looks tricky and easy to miss something.
> 
> I wrote some notes for it before, attached them for illustration.  I believe
> one should not follow them when reviewing the patch - for obvious reasons.
> 
> 
> >  - It should be possible to fix PR 9436 without
> 
> We can pretend PR 9436 does not exist.  

I disagree.  Fixing this exact design problem will make it much
clearer what needs to be done next.

> I noted a wish of removing bpstat_what
> already on #gdb 2008-12-31T22:51:10Z I believe not aware of this PR that time.
> I believe the wish lasts longer than I have expressed it on #gdb.

The comment mentioning that the table is not really needed is there
since 5.0.  That's like 10 years ago.  :-)

> > In fact, it's not clear to me yet why is the new interface better.
> 
> The interface was better (=completely removed) in the original post:
> 	[patch 3/3] bpstat_what removal
> 	http://sourceware.org/ml/gdb-patches/2010-05/msg00050.html

Nope, that was certainly much worse, for the reason Stan already
explained in that thread.

> Currently the interface was created with the goal of not introducing anything
> new.  All its element have both their names and functionality exactly matching
> the already existing (and currently required to stay in place) GDB data
> structures.  They had to copy them to comply with the separation requirements:
> 	Re: [patch 3/3] bpstat_what removal
> 	http://sourceware.org/ml/gdb-patches/2010-05/msg00186.html

I don't see how that relates, sorry.

> Contrary to it what exactly does BPSTAT_WHAT_STEP_RESUME and how to combine it
> with other events is unclear to me.
>     /* Clear step resume breakpoint, and keep checking.  */

Yes, there's an issue with step-resume's priority over other events.
step-resume takes priority over all other actions, even stops for other
breakpoints.  It usually just works (breakpoint on top of step-resume
breakpoint, for instance), because when you hit the step-resume, the step
range _also_ ends, so we stop anyway.  I can see that the (infrun.c)
optimization around:

      /* We are at the start of a different line.  So stop.  Note that
         we don't stop if we step into the middle of a different line.
         That is said to make things like for (;;) statements work
         better.  */

can make us miss a breakpoint in some rare cases.  Acknowledging this
doesn't mean we can't tackle one issue at a time.

> > To recap, IMO, the current problem with bpstat_main_action is that a few of
> > its values aren't really independent and mutually exclusive
> > with the others -- BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT.
> 
> What if a new breakpoint type wants to stop?  

BPSTAT_WHAT_STOP_SILENT / BPSTAT_WHAT_STOP_NOISY.

> What if a new breakpoint type
> does not want to stop?  

BPSTAT_WHAT_SINGLE

> And how they combine if they happend together with
> other events? 

The highest priority of the above wins.  If you're going to stop,
you'll never want to set longjmp or step resume breakpoints, or
start a step over.  You're just going to stop, period.

(My feeling is that we should completely move the infrun-specific
breakpoints handling to infrun (longjmp, longjmp-resume, step-resume),
though it isn't clear _how_ is best.  I hope that by eliminating
the table first, we can think of better interfaces afterwards.
We may even end up with something like yours, I'm not rejecting
it upfront, to be clear.)

> > How about fixing the PR that way, and adding the new testcase without all
> > the revamping?  That'd be surely a step in the right direction.
> 
> The revamping was the goal.  I only found the PR to have some advocacy for its
> acceptance but to be honest I would prefer to forget about the PR now at all.

I beg to differ :-)

> 
> 
> >  - I feel that even getting rid of the table bpstat_what_main_action
> > table
> 
> Yes, it has to be done.

It doesn't _have_ to.  There's nothing wrong with it, if you look at
it as a state machine (and once you remove the states that should never
have been put there in the first place).  But yeah, there's a lot of
unnecessary indirection.  I don't find it hard to read, but you do
have to think a bit about it to understand it the first time;
afterwards, it's like riding a bike.  Basically, removing it means
moving from a data driven table to a code/switch driven table; a
different dressing, but it's mostly the same underneath.  :-)

> > could be done without changing the interface between breakpoint.c
> > and infrun.c (removing bpstat_main_action), and other way around too.
> 
> Changing the interface was also the goal - the goal of simplifying GDB.
> 
> So far I thought the patches should have the goal of making the patched code
> the best we can (in this case to make it more simple).  If we track the goal
> of a minimal patchset to reach some functionality we can completely drop this
> patch as its primary goal is no new functionality but just a code
> simplification (plus simplification of future code extensions).

Well, I agree with all that, but it just doesn't look simpler
to me at first look.  :-)  That's my main worry.  I'll explain further below.
If we split this in smaller pieces, it should make second looks
easier, hopefuly.

> > That is, it feels like we could tackle these changes independently.
> > Not sure though.
> 
> Probably it can although I find such patch more difficult than this one.

I'll prove you wrong.  :-)  I've taken your patch, and started hacking on
it, fixing a few things that didn't look exactly right, and the redundancy
in the perform's and the other flags, then, I renamed back the perform
actions to the current/old BPSTAT_WHAT_ names, and voila.  I just had
to reorder the bpstat_what_main_action enum list to match the priorities.

I'd still prefer splitting the getting rid of BPSTAT_WHAT_CHECK_SHLIBS
and BPSTAT_WHAT_CHECK_JIT to its own patch, and, bpstat_what is probably
not exactly the ideal place to handle these breakpoints (it feels like
bpstop_what shouldn't have side effects), though that's no biggie and
can be fixed at any other time.

You'll notice that this bpstat_what version is even a bit more simplified
than yours, because it centralizes BPSTAT_WHAT_SINGLE handling (it's really
a property of whether there's a breakpoint at PC when we keep going,
so it could even be eliminated if we're okay with adding a
breakpoint_inserted_here call somewhere else; if it stays, it's really
a property of the breakpoint location, not the breakpoint, it seems.),
and, it starts off as BPSTAT_WHAT_KEEP_CHECKING, and so all cases that
would explicitly set no-action disappear.

Let me know what do you think.

-- 
Pedro Alves

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/breakpoint.c	2010-06-16 20:02:08.000000000 +0100
@@ -4115,7 +4115,7 @@ bpstat_stop_status (struct address_space
 	    bs->stop = 0;
 	  else
 	    bpstat_check_breakpoint_conditions (bs, ptid);
-	
+
 	  if (bs->stop)
 	    {
 	      ++(b->hit_count);
@@ -4187,235 +4187,133 @@ bpstat_stop_status (struct address_space
 
   return root_bs->next;
 }
-

-/* Tell what to do about this bpstat.  */
-struct bpstat_what
-bpstat_what (bpstat bs)
-{
-  /* Classify each bpstat as one of the following.  */
-  enum class
-    {
-      /* This bpstat element has no effect on the main_action.  */
-      no_effect = 0,
-
-      /* There was a watchpoint, stop but don't print.  */
-      wp_silent,
-
-      /* There was a watchpoint, stop and print.  */
-      wp_noisy,
-
-      /* There was a breakpoint but we're not stopping.  */
-      bp_nostop,
-
-      /* There was a breakpoint, stop but don't print.  */
-      bp_silent,
 
-      /* There was a breakpoint, stop and print.  */
-      bp_noisy,
-
-      /* We hit the longjmp breakpoint.  */
-      long_jump,
-
-      /* We hit the longjmp_resume breakpoint.  */
-      long_resume,
+static void
+handle_jit_event (void)
+{
+  struct frame_info *frame;
+  struct gdbarch *gdbarch;
 
-      /* We hit the step_resume breakpoint.  */
-      step_resume,
+  /* Switch terminal for any messages produced by
+     breakpoint_re_set.  */
+  target_terminal_ours_for_output ();
 
-      /* We hit the shared library event breakpoint.  */
-      shlib_event,
+  frame = get_current_frame ();
+  gdbarch = get_frame_arch (frame);
 
-      /* We hit the jit event breakpoint.  */
-      jit_event,
+  jit_event_handler (gdbarch);
 
-      /* This is just used to count how many enums there are.  */
-      class_last
-    };
+  target_terminal_inferior ();
+}
 
-  /* Here is the table which drives this routine.  So that we can
-     format it pretty, we define some abbreviations for the
-     enum bpstat_what codes.  */
-#define kc BPSTAT_WHAT_KEEP_CHECKING
-#define ss BPSTAT_WHAT_STOP_SILENT
-#define sn BPSTAT_WHAT_STOP_NOISY
-#define sgl BPSTAT_WHAT_SINGLE
-#define slr BPSTAT_WHAT_SET_LONGJMP_RESUME
-#define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
-#define sr BPSTAT_WHAT_STEP_RESUME
-#define shl BPSTAT_WHAT_CHECK_SHLIBS
-#define jit BPSTAT_WHAT_CHECK_JIT
-
-/* "Can't happen."  Might want to print an error message.
-   abort() is not out of the question, but chances are GDB is just
-   a bit confused, not unusable.  */
-#define err BPSTAT_WHAT_STOP_NOISY
-
-  /* Given an old action and a class, come up with a new action.  */
-  /* One interesting property of this table is that wp_silent is the same
-     as bp_silent and wp_noisy is the same as bp_noisy.  That is because
-     after stopping, the check for whether to step over a breakpoint
-     (BPSTAT_WHAT_SINGLE type stuff) is handled in proceed() without
-     reference to how we stopped.  We retain separate wp_silent and
-     bp_silent codes in case we want to change that someday. 
-
-     Another possibly interesting property of this table is that
-     there's a partial ordering, priority-like, of the actions.  Once
-     you've decided that some action is appropriate, you'll never go
-     back and decide something of a lower priority is better.  The
-     ordering is:
-
-     kc   < jit clr sgl shl slr sn sr ss
-     sgl  < jit shl slr sn sr ss
-     slr  < jit err shl sn sr ss
-     clr  < jit err shl sn sr ss
-     ss   < jit shl sn sr
-     sn   < jit shl sr
-     jit  < shl sr
-     shl  < sr
-     sr   <
-
-     What I think this means is that we don't need a damned table
-     here.  If you just put the rows and columns in the right order,
-     it'd look awfully regular.  We could simply walk the bpstat list
-     and choose the highest priority action we find, with a little
-     logic to handle the 'err' cases.  */
-
-  /* step_resume entries: a step resume breakpoint overrides another
-     breakpoint of signal handling (see comment in wait_for_inferior
-     at where we set the step_resume breakpoint).  */
+/* Prepare WHAT final decision for infrun.  */
 
-  static const enum bpstat_what_main_action
-    table[(int) class_last][(int) BPSTAT_WHAT_LAST] =
-  {
-  /*                              old action */
-  /*               kc   ss   sn   sgl  slr  clr  sr  shl  jit */
-/* no_effect */   {kc,  ss,  sn,  sgl, slr, clr, sr, shl, jit},
-/* wp_silent */   {ss,  ss,  sn,  ss,  ss,  ss,  sr, shl, jit},
-/* wp_noisy */    {sn,  sn,  sn,  sn,  sn,  sn,  sr, shl, jit},
-/* bp_nostop */   {sgl, ss,  sn,  sgl, slr, slr, sr, shl, jit},
-/* bp_silent */   {ss,  ss,  sn,  ss,  ss,  ss,  sr, shl, jit},
-/* bp_noisy */    {sn,  sn,  sn,  sn,  sn,  sn,  sr, shl, jit},
-/* long_jump */   {slr, ss,  sn,  slr, slr, err, sr, shl, jit},
-/* long_resume */ {clr, ss,  sn,  err, err, err, sr, shl, jit},
-/* step_resume */ {sr,  sr,  sr,  sr,  sr,  sr,  sr, sr,  sr },
-/* shlib */       {shl, shl, shl, shl, shl, shl, sr, shl, shl},
-/* jit_event */   {jit, jit, jit, jit, jit, jit, sr, jit, jit}
-  };
+/* Decide what infrun needs to do with this bpstat.  */
 
-#undef kc
-#undef ss
-#undef sn
-#undef sgl
-#undef slr
-#undef clr
-#undef err
-#undef sr
-#undef ts
-#undef shl
-#undef jit
-  enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
+struct bpstat_what
+bpstat_what (bpstat bs)
+{
   struct bpstat_what retval;
+  /* We need to defer calling `solib_add', as adding new symbols
+     resets breakpoints, which in turn deletes breakpoint locations,
+     and hence may clear unprocessed entries in the BS chain.  */
+  int shlib_event = 0;
+  int jit_event = 0;
 
+  retval.main_action = BPSTAT_WHAT_KEEP_CHECKING;
   retval.call_dummy = STOP_NONE;
+
   for (; bs != NULL; bs = bs->next)
     {
-      enum class bs_class = no_effect;
+      /* Extract this BS's action.  After processing each BS, we check
+	 if its action overrides all we've seem so far.  */
+      enum bpstat_what_main_action this_action = BPSTAT_WHAT_KEEP_CHECKING;
+      enum bptype bptype;
+
       if (bs->breakpoint_at == NULL)
-	/* I suspect this can happen if it was a momentary breakpoint
-	   which has since been deleted.  */
-	continue;
-      if (bs->breakpoint_at->owner == NULL)
-	bs_class = bp_nostop;
+	{
+	  /* I suspect this can happen if it was a momentary
+	     breakpoint which has since been deleted.  */
+	  bptype = bp_none;
+	}
       else
-      switch (bs->breakpoint_at->owner->type)
 	{
-	case bp_none:
-	  continue;
+	  enum bp_loc_type loc_type = bs->breakpoint_at->loc_type;
+
+	  /* If we end up proceeding, we'll need to step over this
+	     breakpoint.  */
+	  if (loc_type == bp_loc_software_breakpoint
+	      || loc_type == bp_loc_hardware_breakpoint)
+	    this_action = BPSTAT_WHAT_SINGLE;
 
+	  if (bs->breakpoint_at->owner == NULL)
+	    bptype = bp_none;
+	  else
+	    bptype = bs->breakpoint_at->owner->type;
+	}
+
+      switch (bptype)
+	{
+	case bp_none:
+	  break;
 	case bp_breakpoint:
 	case bp_hardware_breakpoint:
 	case bp_until:
 	case bp_finish:
-	  if (bs->stop)
-	    {
-	      if (bs->print)
-		bs_class = bp_noisy;
-	      else
-		bs_class = bp_silent;
-	    }
-	  else
-	    bs_class = bp_nostop;
-	  break;
 	case bp_watchpoint:
 	case bp_hardware_watchpoint:
 	case bp_read_watchpoint:
 	case bp_access_watchpoint:
+	case bp_catchpoint:
 	  if (bs->stop)
 	    {
 	      if (bs->print)
-		bs_class = wp_noisy;
+		this_action = BPSTAT_WHAT_STOP_NOISY;
 	      else
-		bs_class = wp_silent;
+		this_action = BPSTAT_WHAT_STOP_SILENT;
 	    }
-	  else
-	    /* There was a watchpoint, but we're not stopping. 
-	       This requires no further action.  */
-	    bs_class = no_effect;
 	  break;
 	case bp_longjmp:
-	  bs_class = long_jump;
+	  this_action = BPSTAT_WHAT_SET_LONGJMP_RESUME;
 	  break;
 	case bp_longjmp_resume:
-	  bs_class = long_resume;
+	  this_action = BPSTAT_WHAT_CLEAR_LONGJMP_RESUME;
 	  break;
 	case bp_step_resume:
 	  if (bs->stop)
-	    {
-	      bs_class = step_resume;
-	    }
-	  else
-	    /* It is for the wrong frame.  */
-	    bs_class = bp_nostop;
+	    this_action = BPSTAT_WHAT_STEP_RESUME;
 	  break;
 	case bp_watchpoint_scope:
-	  bs_class = bp_nostop;
-	  break;
-	case bp_shlib_event:
-	  bs_class = shlib_event;
-	  break;
-	case bp_jit_event:
-	  bs_class = jit_event;
-	  break;
 	case bp_thread_event:
 	case bp_overlay_event:
 	case bp_longjmp_master:
 	case bp_std_terminate_master:
-	  bs_class = bp_nostop;
 	  break;
-	case bp_catchpoint:
-	  if (bs->stop)
-	    {
-	      if (bs->print)
-		bs_class = bp_noisy;
-	      else
-		bs_class = bp_silent;
-	    }
-	  else
-	    /* There was a catchpoint, but we're not stopping.  
-	       This requires no further action.  */
-	    bs_class = no_effect;
+	case bp_shlib_event:
+	  shlib_event = 1;
+
+	  /* If requested, stop when the dynamic linker notifies GDB
+	     of events.  This allows the user to get control and place
+	     breakpoints in initializer routines for dynamically
+	     loaded objects (among other things).  */
+	  if (stop_on_solib_events)
+	    /* FIXME, noisy or not?  */
+	    this_action = BPSTAT_WHAT_STOP_NOISY;
+	  break;
+	case bp_jit_event:
+	  jit_event = 1;
 	  break;
 	case bp_call_dummy:
-	  /* Make sure the action is stop (silent or noisy),
-	     so infrun.c pops the dummy frame.  */
-	  bs_class = bp_silent;
+	  /* Make sure the action is stop (silent or noisy), so
+	     infrun.c pops the dummy frame.  */
 	  retval.call_dummy = STOP_STACK_DUMMY;
+	  this_action = BPSTAT_WHAT_STOP_SILENT;
 	  break;
 	case bp_std_terminate:
-	  /* Make sure the action is stop (silent or noisy),
-	     so infrun.c pops the dummy frame.  */
-	  bs_class = bp_silent;
+	  /* Make sure the action is stop (silent or noisy), so
+	     infrun.c pops the dummy frame.  */
 	  retval.call_dummy = STOP_STD_TERMINATE;
+	  this_action = BPSTAT_WHAT_STOP_SILENT;
 	  break;
 	case bp_tracepoint:
 	case bp_fast_tracepoint:
@@ -4424,11 +4322,37 @@ bpstat_what (bpstat bs)
 	     out already.  */
 	  internal_error (__FILE__, __LINE__,
 			  _("bpstat_what: tracepoint encountered"));
-	  break;
+	default:
+	  internal_error (__FILE__, __LINE__,
+			  _("bpstat_what: unhandled bptype %d"), (int) bptype);
 	}
-      current_action = table[(int) bs_class][(int) current_action];
+
+      retval.main_action = max (retval.main_action, this_action);
+    }
+
+  if (shlib_event)
+    {
+      /* Check for any newly added shared libraries if we're supposed
+	 to be adding them automatically.  */
+
+      /* Switch terminal for any messages produced by
+	 breakpoint_re_set.  */
+      target_terminal_ours_for_output ();
+
+#ifdef SOLIB_ADD
+      SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
+#else
+      solib_add (NULL, 0, &current_target, auto_solib_add);
+#endif
+
+      target_terminal_inferior ();
     }
-  retval.main_action = current_action;
+
+  if (jit_event)
+    {
+      handle_jit_event ();
+    }
+
   return retval;
 }
 
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/breakpoint.h	2010-06-16 20:02:08.000000000 +0100
@@ -563,7 +563,20 @@ extern bpstat bpstat_stop_status (struct
 				  CORE_ADDR pc, ptid_t ptid);
 

 /* This bpstat_what stuff tells wait_for_inferior what to do with a
-   breakpoint (a challenging task).  */
+   breakpoint (a challenging task).
+
+   The enum values order defines priority-like order of the actions.
+   Once you've decided that some action is appropriate, you'll never
+   go back and decide something of a lower priority is better.  Each
+   of these actions is mutually exclusive with the others.  That
+   means, that if you find yourself adding a new action class here and
+   wanting to tell GDB that you have two simultaneous actions to
+   handle, something is wrong, and you probably don't actually need a
+   new action type.
+
+   Note that a step resume breakpoint overrides another breakpoint of
+   signal handling (see comment in wait_for_inferior at where we set
+   the step_resume breakpoint).  */
 
 enum bpstat_what_main_action
   {
@@ -572,18 +585,6 @@ enum bpstat_what_main_action
        else).  */
     BPSTAT_WHAT_KEEP_CHECKING,
 
-    /* Rather than distinguish between noisy and silent stops here, it
-       might be cleaner to have bpstat_print make that decision (also
-       taking into account stop_print_frame and source_only).  But the
-       implications are a bit scary (interaction with auto-displays, etc.),
-       so I won't try it.  */
-
-    /* Stop silently.  */
-    BPSTAT_WHAT_STOP_SILENT,
-
-    /* Stop and print.  */
-    BPSTAT_WHAT_STOP_NOISY,
-
     /* Remove breakpoints, single step once, then put them back in and
        go back to what we were doing.  It's possible that this should be
        removed from the main_action and put into a separate field, to more
@@ -600,18 +601,20 @@ enum bpstat_what_main_action
        BPSTAT_WHAT_KEEP_CHECKING.  */
     BPSTAT_WHAT_CLEAR_LONGJMP_RESUME,
 
-    /* Clear step resume breakpoint, and keep checking.  */
-    BPSTAT_WHAT_STEP_RESUME,
+    /* Rather than distinguish between noisy and silent stops here, it
+       might be cleaner to have bpstat_print make that decision (also
+       taking into account stop_print_frame and source_only).  But the
+       implications are a bit scary (interaction with auto-displays, etc.),
+       so I won't try it.  */
 
-    /* Check the dynamic linker's data structures for new libraries, then
-       keep checking.  */
-    BPSTAT_WHAT_CHECK_SHLIBS,
+    /* Stop silently.  */
+    BPSTAT_WHAT_STOP_SILENT,
 
-    /* Check for new JITed code.  */
-    BPSTAT_WHAT_CHECK_JIT,
+    /* Stop and print.  */
+    BPSTAT_WHAT_STOP_NOISY,
 
-    /* This is just used to keep track of how many enums there are.  */
-    BPSTAT_WHAT_LAST
+    /* Clear step resume breakpoint, and keep checking.  */
+    BPSTAT_WHAT_STEP_RESUME,
   };
 
 /* An enum indicating the kind of "stack dummy" stop.  This is a bit
Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h	2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/inferior.h	2010-06-16 20:02:07.000000000 +0100
@@ -232,6 +232,8 @@ extern char *construct_inferior_argument
 
 /* From infrun.c */
 
+extern int stop_on_solib_events;
+
 extern void start_remote (int from_tty);
 
 extern void normal_stop (void);
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/infrun.c	2010-06-16 20:02:08.000000000 +0100
@@ -294,7 +294,7 @@ static struct symbol *step_start_functio
 
 /* Nonzero if we want to give control to the user when we're notified
    of shared library events by the dynamic linker.  */
-static int stop_on_solib_events;
+int stop_on_solib_events;
 static void
 show_stop_on_solib_events (struct ui_file *file, int from_tty,
 			   struct cmd_list_element *c, const char *value)
@@ -4045,6 +4045,12 @@ process_event_stop_test:
 	stop_stack_dummy = what.call_dummy;
       }
 
+    /* If we hit an internal event that triggers symbol changes, the
+       current frame will be invalidated within bpstat_what (e.g., if
+       we hit an internal solib event).  Re-fetch it.  */
+    frame = get_current_frame ();
+    gdbarch = get_frame_arch (frame);
+
     switch (what.main_action)
       {
       case BPSTAT_WHAT_SET_LONGJMP_RESUME:
@@ -4079,7 +4085,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	return;
 
       case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
-        if (debug_infrun)
+	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog,
 			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
 
@@ -4092,16 +4098,17 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	return;
 
       case BPSTAT_WHAT_SINGLE:
-        if (debug_infrun)
+	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
 	ecs->event_thread->stepping_over_breakpoint = 1;
-	/* Still need to check other stuff, at least the case
-	   where we are stepping and step out of the right range.  */
+	/* Still need to check other stuff, at least the case where we
+	   are stepping and step out of the right range.  */
 	break;
 
       case BPSTAT_WHAT_STOP_NOISY:
-        if (debug_infrun)
+	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
+
 	stop_print_frame = 1;
 
 	/* We are about to nuke the step_resume_breakpointt via the
@@ -4111,18 +4118,18 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	return;
 
       case BPSTAT_WHAT_STOP_SILENT:
-        if (debug_infrun)
+	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
 	stop_print_frame = 0;
 
-	/* We are about to nuke the step_resume_breakpoin via the
+	/* We are about to nuke the step_resume_breakpoint via the
 	   cleanup chain, so no need to worry about it here.  */
 
 	stop_stepping (ecs);
 	return;
 
       case BPSTAT_WHAT_STEP_RESUME:
-        if (debug_infrun)
+	if (debug_infrun)
 	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
 
 	delete_step_resume_breakpoint (ecs->event_thread);
@@ -4149,66 +4156,6 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
 	  }
 	break;
 
-      case BPSTAT_WHAT_CHECK_SHLIBS:
-	{
-          if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_SHLIBS\n");
-
-	  /* Check for any newly added shared libraries if we're
-	     supposed to be adding them automatically.  Switch
-	     terminal for any messages produced by
-	     breakpoint_re_set.  */
-	  target_terminal_ours_for_output ();
-	  /* NOTE: cagney/2003-11-25: Make certain that the target
-	     stack's section table is kept up-to-date.  Architectures,
-	     (e.g., PPC64), use the section table to perform
-	     operations such as address => section name and hence
-	     require the table to contain all sections (including
-	     those found in shared libraries).  */
-#ifdef SOLIB_ADD
-	  SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
-#else
-	  solib_add (NULL, 0, &current_target, auto_solib_add);
-#endif
-	  target_terminal_inferior ();
-
-	  /* If requested, stop when the dynamic linker notifies
-	     gdb of events.  This allows the user to get control
-	     and place breakpoints in initializer routines for
-	     dynamically loaded objects (among other things).  */
-	  if (stop_on_solib_events || stop_stack_dummy)
-	    {
-	      stop_stepping (ecs);
-	      return;
-	    }
-	  else
-	    {
-	      /* We want to step over this breakpoint, then keep going.  */
-	      ecs->event_thread->stepping_over_breakpoint = 1;
-	      break;
-	    }
-	}
-	break;
-
-      case BPSTAT_WHAT_CHECK_JIT:
-        if (debug_infrun)
-          fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_JIT\n");
-
-        /* Switch terminal for any messages produced by breakpoint_re_set.  */
-        target_terminal_ours_for_output ();
-
-        jit_event_handler (gdbarch);
-
-        target_terminal_inferior ();
-
-        /* We want to step over this breakpoint, then keep going.  */
-        ecs->event_thread->stepping_over_breakpoint = 1;
-
-        break;
-
-      case BPSTAT_WHAT_LAST:
-	/* Not a real code, but listed here to shut up gcc -Wall.  */
-
       case BPSTAT_WHAT_KEEP_CHECKING:
 	break;
       }
@@ -4345,6 +4292,7 @@ infrun: not switching back to stepped th
      the frame cache to be re-initialized, making our FRAME variable
      a dangling pointer.  */
   frame = get_current_frame ();
+  gdbarch = get_frame_arch (frame);
 
   /* If stepping through a line, keep going if still within it.
 
Index: src/gdb/testsuite/gdb.base/nostdlib.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/nostdlib.c	2010-06-16 20:02:07.000000000 +0100
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+_start (void)
+{
+  extern void marker (void);
+
+  marker ();
+}
+
+void
+marker (void)
+{
+}
Index: src/gdb/testsuite/gdb.base/nostdlib.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/nostdlib.exp	2010-06-16 20:02:07.000000000 +0100
@@ -0,0 +1,54 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "nostdlib"
+set srcfile ${testfile}.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+# default_target_compile would otherwise add "-lm" making the testcase
+# dependent on whether the system libraries are already prelinked.
+# prelink: Could not set /lib64/libm-2.11.1.so owner or mode: Operation not permitted
+set compile {
+    gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-nostdlib}
+}
+set board [target_info name]
+if [board_info $board exists mathlib] {
+    set mathlib [board_info $dest mathlib]
+    set_board_info mathlib ""
+    set err [eval $compile]
+    set_board_info mathlib $mathlib
+} else {
+    set_board_info mathlib ""
+    set err [eval $compile]
+    unset_board_info mathlib
+}
+if {$err != ""} {
+    untested ${testfile}.exp
+    return -1
+}
+
+clean_restart $executable
+
+gdb_breakpoint "*marker"
+gdb_breakpoint "*_start"
+
+gdb_run_cmd
+
+# Breakpoint 2, Stopped due to shared library event
+# _start () at ./gdb.base/nostdlib.c:20
+gdb_test "" {Breakpoint [0-9]+, .*_start .*} "stop at run"
+
+gdb_test "continue" {Breakpoint [0-9]+, marker .*} "continue to marker"



More information about the Gdb-patches mailing list