This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch 3/3] bpstat_what removal [rediff]


On Saturday 12 June 2010 18:01:37, Jan Kratochvil wrote:
> Hi,
> 
> originally posted as:
> 	Re: [patch 3/3] bpstat_what removal
> 	http://sourceware.org/ml/gdb-patches/2010-05/msg00370.html
> 
> Rediffed only.

Do you think it would be hard to split this into smaller pieces?  It
would help a lot --- at least me.  :-)  I've tried to review this a couple
of times already, but it looks tricky and easy to miss something.

 - It should be possible to fix PR 9436 without removing bpstat_main_action.
In fact, it's not clear to me yet why is the new interface better.  But I
can be convinced.  I probably just need it explained to me.

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.

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.

 - I feel that even getting rid of the table bpstat_what_main_action
table could be done without changing the interface between breakpoint.c
and infrun.c (removing bpstat_main_action), and other way around too.
That is, it feels like we could tackle these changes independently.
Not sure though.  Let me know what you think, and if you disagree, I'll
try harder at reviewing this.

> 
> ------------------------------------------------------------------------------
> On Mon, 17 May 2010 23:45:07 +0200, Jan Kratochvil wrote:
> 
> On Fri, 07 May 2010 19:16:54 +0200, Pedro Alves wrote:
> > Why does infrun have to know about checking solib events at
> > all?  Checking for new loaded solibs looks like a detail of this
> > internal breakpoint.  There's not much of inferior run control
> > related to it.  It would seem to me that breakpoint.c could handle
> > it instead;
> 
> OK, split it now for doing everything doable in breakpoint.c's bpstat_what()
> there while the core inferior run control is still left in infrun.c's
> handle_inferior_event().  Some of the events are just flagged in bpstat_what()
> and delayed to later execution at handle_inferior_event() by:
> 
> +    unsigned stepping_over_breakpoint : 1;
> +    unsigned bp_longjmp : 1;
> +    unsigned bp_longjmp_resume : 1;
> +    unsigned bp_step_resume_on_stop : 1;
> 
> Also found a better testcase of PR 9436 really turning FSF GDB HEAD FAIL->PASS:
> 	(gdb) run 
> 	Starting program: .../gdb/testsuite/gdb.base/nostdlib 
> 	Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
> 	28      {
> 	(gdb) FAIL: gdb.base/nostdlib.exp: stop at run
> ->
> 	Starting program: .../gdb/testsuite/gdb.base/nostdlib 
> 	Breakpoint 2, Stopped due to shared library event
> 	_start () at ./gdb.base/nostdlib.c:20
> 	20      {
> 	(gdb) PASS: gdb.base/nostdlib.exp: stop at run
> 	continue
> 	Continuing.
> 	Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
> 	28      {
> 	(gdb) PASS: gdb.base/nostdlib.exp: continue to marker
> 
> I have some other local modification on top of it fixing inferior calls where
> currently on -nostdlib executables "shlib events" clash with "call dummy".
> 
> 
> > for example, II have a feeling it should be possible to add
> > a new breakpoint_ops for shlib_events, similar to the "catch catch"
> > implementation.  And with enough breakpoint_ops abstraction we could
> > even get rid of the shlib_event breakpoint type, and move the whole
> > shlib breakpoint definition to solib.c, like ada-lang.c handles
> > its catchpoints.  
> 
> Yes; doable later in a different patch.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/
> 2010-05-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* breakpoint.c (breakpoint_type_name, bpstat_what_merge)
> 	(bpstat_what_finalize): New functions.
> 	(bpstat_what): Replace.
> 	* breakpoint.h (enum bpstat_what_main_action): Remove.
> 	(struct bpstat_what): Replace.
> 	(bpstat_what): Update comment.
> 	* inferior.h (stop_on_solib_events): New declaration.
> 	(bpstat_what_debug): New prototype.
> 	* infrun.c (stop_on_solib_events): Remove static.
> 	(bpstat_what_debug): New function.
> 	(handle_inferior_event): Clear frame and gdbarch before deciding what
> 	action to take.  Remove variable jmp_buf_pc.  Replace block with
> 	bpstat_what.  Reinitialize even gdbarch when frame gets reinitialized.
> 
> gdb/testsuite/
> 2010-05-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/nostdlib.exp, gdb.base/nostdlib.c: New.
> 
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -4187,235 +4187,257 @@ bpstat_stop_status (struct address_space *aspace,
>  
>    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,
> +/* Return BPTYPE text representation for the purpose of debug messages.  */
>  
> -      /* 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,
> +static const char *
> +breakpoint_type_name (enum bptype bptype)
> +{
> +  switch (bptype)
> +    {
> +    case bp_none:
> +      return "bp_none";
> +    case bp_breakpoint:
> +      return "bp_breakpoint";
> +    case bp_hardware_breakpoint:
> +      return "bp_hardware_breakpoint";
> +    case bp_until:
> +      return "bp_until";
> +    case bp_finish:
> +      return "bp_finish";
> +    case bp_watchpoint:
> +      return "bp_watchpoint";
> +    case bp_hardware_watchpoint:
> +      return "bp_hardware_watchpoint";
> +    case bp_read_watchpoint:
> +      return "bp_read_watchpoint";
> +    case bp_access_watchpoint:
> +      return "bp_access_watchpoint";
> +    case bp_longjmp:
> +      return "bp_longjmp";
> +    case bp_longjmp_resume:
> +      return "bp_longjmp_resume";
> +    case bp_step_resume:
> +      return "bp_step_resume";
> +    case bp_watchpoint_scope:
> +      return "bp_watchpoint_scope";
> +    case bp_call_dummy:
> +      return "bp_call_dummy";
> +    case bp_std_terminate:
> +      return "bp_std_terminate";
> +    case bp_shlib_event:
> +      return "bp_shlib_event";
> +    case bp_thread_event:
> +      return "bp_thread_event";
> +    case bp_overlay_event:
> +      return "bp_overlay_event";
> +    case bp_longjmp_master:
> +      return "bp_longjmp_master";
> +    case bp_std_terminate_master:
> +      return "bp_std_terminate_master";
> +    case bp_catchpoint:
> +      return "bp_catchpoint";
> +    case bp_tracepoint:
> +      return "bp_tracepoint";
> +    case bp_fast_tracepoint:
> +      return "bp_fast_tracepoint";
> +    case bp_jit_event:
> +      return "bp_jit_event";
> +    }
> +  internal_error (__FILE__, __LINE__, _("Invalid breakpoint type %d"),
> +		  (int) bptype);
> +  return NULL;
> +}
>  
> -      /* We hit the longjmp_resume breakpoint.  */
> -      long_resume,
> +/* Return the more significant events from both A and B.  */
>  
> -      /* We hit the step_resume breakpoint.  */
> -      step_resume,
> +struct bpstat_what
> +bpstat_what_merge (struct bpstat_what a, struct bpstat_what b)
> +{
> +  struct bpstat_what retval;
>  
> -      /* We hit the shared library event breakpoint.  */
> -      shlib_event,
> +  retval.print_frame = max (a.print_frame, b.print_frame);
> +  retval.stop_step = max (a.stop_step, b.stop_step);
> +  retval.perform = max (a.perform, b.perform);
> +  retval.stepping_over_breakpoint = max (a.stepping_over_breakpoint,
> +					 b.stepping_over_breakpoint);
> +  retval.bp_longjmp = max (a.bp_longjmp, b.bp_longjmp);
> +  retval.bp_longjmp_resume = max (a.bp_longjmp_resume, b.bp_longjmp_resume);
> +  retval.bp_step_resume_on_stop = max (a.bp_step_resume_on_stop,
> +				       b.bp_step_resume_on_stop);
>  
> -      /* We hit the jit event breakpoint.  */
> -      jit_event,
> +  return retval;
> +}
>  
> -      /* This is just used to count how many enums there are.  */
> -      class_last
> -    };
> +/* Prepare WHAT final decision for infrun.  */
>  
> -  /* 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).  */
> -
> -  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}
> -  };
> +static void
> +bpstat_what_finalize (struct bpstat_what *what)
> +{
> +  if (what->print_frame == pf_default)
> +    what->print_frame = pf_yes;
> +  if (what->stop_step == ss_default)
> +    what->stop_step = ss_print_yes;
> +  gdb_assert (what->perform != pe_undef);
> +}
>  
> -#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;
> +/* Tell what to do about this bpstat.  */
> +struct bpstat_what
> +bpstat_what (bpstat bs)
> +{
>    struct bpstat_what retval;
> +  /* solib_add may call breakpoint_re_set which would clear many
> +     BREAKPOINT_AT entries still going to be processed.  breakpoint_re_set
> +     does not keep the same bp_location's even if they actually do not
> +     change.  */
> +  int perform_shlib = 0;
> +
> +  memset (&retval, 0, sizeof (retval));
> +  retval.print_frame = pf_default;
> +  retval.stop_step = ss_default;
> +  retval.perform = pe_default;
>  
> -  retval.call_dummy = STOP_NONE;
>    for (; bs != NULL; bs = bs->next)
>      {
> -      enum class bs_class = no_effect;
> +      /* Decisions for this specific BS, they get mapped to their *_max
> +	 variants at the end of this BS processing.  */
> +      struct bpstat_what this;
> +      enum bptype bptype;
> +
> +      memset (&this, 0, sizeof (this));
> +      this.print_frame = pf_default;
> +      this.stop_step = ss_default;
> +      this.perform = pe_undef;
> +
>        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 if (bs->breakpoint_at->owner == NULL)
> +	{
> +	  this.stepping_over_breakpoint = 1;
> +	  bptype = bp_none;
> +	}
>        else
> -      switch (bs->breakpoint_at->owner->type)
> +	bptype = bs->breakpoint_at->owner->type;
> +
> +      switch (bptype)
>  	{
>  	case bp_none:
> -	  continue;
> -
> +	  this.perform = pe_check_more;
> +	  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;
> +	      this.print_frame = bs->print ? pf_yes : pf_no;
> +	      this.perform = pe_stop;
>  	    }
>  	  else
> -	    bs_class = bp_nostop;
> +	    {
> +	      this.stepping_over_breakpoint = 1;
> +	      this.perform = pe_check_more;
> +	    }
>  	  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;
> -	      else
> -		bs_class = wp_silent;
> +	      this.print_frame = bs->print ? pf_yes : pf_no;
> +	      this.perform = pe_stop;
>  	    }
>  	  else
> -	    /* There was a watchpoint, but we're not stopping. 
> -	       This requires no further action.  */
> -	    bs_class = no_effect;
> +	    {
> +	      /* There was a watchpoint or catchpoint, but we're not
> +		 stopping.  This requires no further action.  */
> +	      this.perform = pe_check_more;
> +	    }
>  	  break;
>  	case bp_longjmp:
> -	  bs_class = long_jump;
> +	  this.bp_longjmp = 1;
> +	  this.stepping_over_breakpoint = 1;
> +	  this.perform = pe_going;
>  	  break;
>  	case bp_longjmp_resume:
> -	  bs_class = long_resume;
> +	  this.bp_longjmp_resume = 1;
> +	  this.stop_step = ss_print_no;
> +	  this.perform = pe_stop_end_range;
>  	  break;
>  	case bp_step_resume:
>  	  if (bs->stop)
>  	    {
> -	      bs_class = step_resume;
> +	      this.bp_step_resume_on_stop = 1;
> +	      gdb_assert (pe_check_more < pe_going);
> +	      this.perform = pe_check_more;
>  	    }
>  	  else
> -	    /* It is for the wrong frame.  */
> -	    bs_class = bp_nostop;
> +	    {
> +	      /* It is for the wrong frame.  */
> +	      this.stepping_over_breakpoint = 1;
> +	      this.perform = pe_check_more;
> +	    }
>  	  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;
> +	  this.stepping_over_breakpoint = 1;
> +	  this.perform = pe_check_more;
>  	  break;
> -	case bp_catchpoint:
> -	  if (bs->stop)
> +	case bp_shlib_event:
> +	  perform_shlib = 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 || stop_stack_dummy)
> +	    this.perform = pe_stop;
> +	  else
>  	    {
> -	      if (bs->print)
> -		bs_class = bp_noisy;
> -	      else
> -		bs_class = bp_silent;
> +	      /* We want to step over this breakpoint, then keep going.  */
> +	      this.stepping_over_breakpoint = 1;
> +	      this.perform = pe_check_more;
>  	    }
> -	  else
> -	    /* There was a catchpoint, but we're not stopping.  
> -	       This requires no further action.  */
> -	    bs_class = no_effect;
> +	  break;
> +	case bp_jit_event:
> +	  /* Switch terminal for any messages produced by breakpoint_re_set.  */
> +	  target_terminal_ours_for_output ();
> +
> +	  {
> +	    struct frame_info *frame = get_current_frame ();
> +	    struct gdbarch *gdbarch = get_frame_arch (frame);
> +
> +	    jit_event_handler (gdbarch);
> +	  }
> +
> +	  target_terminal_inferior ();
> +
> +	  /* We want to step over this breakpoint, then keep going.  */
> +	  this.stepping_over_breakpoint = 1;
> +	  this.perform = pe_check_more;
>  	  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;
> -	  retval.call_dummy = STOP_STACK_DUMMY;
> +	  stop_stack_dummy = STOP_STACK_DUMMY;
> +	  this.print_frame = pf_no;
> +	  this.perform = pe_stop;
>  	  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;
> -	  retval.call_dummy = STOP_STD_TERMINATE;
> +	  stop_stack_dummy = STOP_STD_TERMINATE;
> +	  this.print_frame = pf_no;
> +	  this.perform = pe_stop;
>  	  break;
>  	case bp_tracepoint:
>  	case bp_fast_tracepoint:
> @@ -4425,10 +4447,49 @@ bpstat_what (bpstat bs)
>  	  internal_error (__FILE__, __LINE__,
>  			  _("bpstat_what: tracepoint encountered"));
>  	  break;
> +	default:
> +	  internal_error (__FILE__, __LINE__,
> +			  _("bpstat_what: Unhandled bptype %s"),
> +			  breakpoint_type_name (bptype));
> +	  break;
>  	}
> -      current_action = table[(int) bs_class][(int) current_action];
> +
> +      /* THIS.PERFORM must be always decided.  */
> +      if (this.perform == pe_undef)
> +	internal_error (__FILE__, __LINE__,
> +			_("bpstat_what: Unset perform, bptype %s"),
> +			breakpoint_type_name (bptype));
> +
> +      bpstat_what_debug (this, breakpoint_type_name (bptype), 0);
> +
> +      retval = bpstat_what_merge (retval, this);
>      }
> -  retval.main_action = current_action;
> +
> +  if (perform_shlib)
> +    {
> +      /* 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 ();
> +    }
> +
> +  bpstat_what_debug (retval, _("summary"), 1);
> +
> +  bpstat_what_finalize (&retval);
> +
>    return retval;
>  }
>  
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -565,53 +565,49 @@ extern bpstat bpstat_stop_status (struct address_space *aspace,
>  /* This bpstat_what stuff tells wait_for_inferior what to do with a
>     breakpoint (a challenging task).  */
>  
> -enum bpstat_what_main_action
> +struct bpstat_what
>    {
> -    /* Perform various other tests; that is, this bpstat does not
> -       say to perform any action (e.g. failed watchpoint and nothing
> -       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
> -       cleanly handle BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE.  */
> -    BPSTAT_WHAT_SINGLE,
> -
> -    /* Set longjmp_resume breakpoint, remove all other breakpoints,
> -       and continue.  The "remove all other breakpoints" part is required
> -       if we are also stepping over another breakpoint as well as doing
> -       the longjmp handling.  */
> -    BPSTAT_WHAT_SET_LONGJMP_RESUME,
> -
> -    /* Clear longjmp_resume breakpoint, then handle as
> -       BPSTAT_WHAT_KEEP_CHECKING.  */
> -    BPSTAT_WHAT_CLEAR_LONGJMP_RESUME,
> -
> -    /* Clear step resume breakpoint, and keep checking.  */
> -    BPSTAT_WHAT_STEP_RESUME,
> -
> -    /* Check the dynamic linker's data structures for new libraries, then
> -       keep checking.  */
> -    BPSTAT_WHAT_CHECK_SHLIBS,
> -
> -    /* Check for new JITed code.  */
> -    BPSTAT_WHAT_CHECK_JIT,
> -
> -    /* This is just used to keep track of how many enums there are.  */
> -    BPSTAT_WHAT_LAST
> +    enum print_frame
> +      {
> +	/* pf_default is pf_yes.  */
> +	pf_default,
> +	/* stop_print_frame value 0.  */
> +	pf_no,
> +	/* stop_print_frame value 1.  */
> +	pf_yes,
> +      }
> +    print_frame;
> +    enum stop_step
> +      {
> +	/* ss_default is ss_print_yes.  */
> +	ss_default,
> +	/* ecs->event_thread->stop_step value 1.  */
> +	ss_print_no,
> +	/* ecs->event_thread->stop_step value 0.  */
> +	ss_print_yes,
> +      }
> +    stop_step;
> +    enum perform
> +      {
> +	pe_undef,
> +	/* Break from this block and check other possibilities why to stop.  */
> +	pe_check_more,
> +	pe_default = pe_check_more,
> +	/* Call stop_stepping (ecs).  */
> +	pe_stop,
> +	/* Like pe_stop but also print_stop_reason (END_STEPPING_RANGE, 0).  */
> +	pe_stop_end_range,
> +	/* Call keep_going (ecs) and return without breaking from this block
> +	   and checking other possibilities why to stop.  Some operations need
> +	   to finish before an already stepped on breakpoint is displayed to
> +	   the user.  */
> +	pe_going,
> +      }
> +    perform;
> +    unsigned stepping_over_breakpoint : 1;
> +    unsigned bp_longjmp : 1;
> +    unsigned bp_longjmp_resume : 1;
> +    unsigned bp_step_resume_on_stop : 1;
>    };
>  
>  /* An enum indicating the kind of "stack dummy" stop.  This is a bit
> @@ -628,17 +624,6 @@ enum stop_stack_kind
>      STOP_STD_TERMINATE
>    };
>  
> -struct bpstat_what
> -  {
> -    enum bpstat_what_main_action main_action;
> -
> -    /* Did we hit a call dummy breakpoint?  This only goes with a main_action
> -       of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
> -       continuing from a call dummy without popping the frame is not a
> -       useful one).  */
> -    enum stop_stack_kind call_dummy;
> -  };
> -
>  /* The possible return values for print_bpstat, print_it_normal,
>     print_it_done, print_it_noop. */
>  enum print_stop_action
> @@ -649,7 +634,7 @@ enum print_stop_action
>      PRINT_NOTHING
>    };
>  
> -/* Tell what to do about this bpstat.  */
> +/* Tell what to do about this list of bpstats.  */
>  struct bpstat_what bpstat_what (bpstat);
>  
>  /* Find the bpstat associated with a breakpoint.  NULL otherwise. */
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -232,10 +232,15 @@ extern char *construct_inferior_arguments (int, char **);
>  
>  /* From infrun.c */
>  
> +extern int stop_on_solib_events;
> +
>  extern void start_remote (int from_tty);
>  
>  extern void normal_stop (void);
>  
> +extern void bpstat_what_debug (struct bpstat_what what, const char *event,
> +			       int print_defaults);
> +
>  extern int signal_stop_state (int);
>  
>  extern int signal_print_state (int);
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -294,7 +294,7 @@ static struct symbol *step_start_function;
>  
>  /* 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)
> @@ -2926,6 +2926,45 @@ handle_syscall_event (struct execution_control_state *ecs)
>    return 1;
>  }
>  
> +/* Print debug information about WHAT on DEBUG_INFRUN.  EVENT contains string
> +   describing what WHAT is for.  PRINT_DEFAULTS will force printing event the
> +   WHAT fields which contain an unchanged default value.  */
> +
> +void
> +bpstat_what_debug (struct bpstat_what what, const char *event,
> +		   int print_defaults)
> +{
> +  if (! debug_infrun)
> +    return;
> +
> +  if (print_defaults || what.print_frame != pf_default)
> +    fprintf_unfiltered (gdb_stdlog, _("infrun: %s: print_frame %s\n"), event,
> +			what.print_frame == pf_default
> +			  ? "pf_default (pf_yes)"
> +			  : what.print_frame == pf_no ? "pf_no" : "pf_yes");
> +
> +  if (print_defaults || what.stop_step != ss_default)
> +    fprintf_unfiltered (gdb_stdlog, _("infrun: %s: stop_step %s\n"), event,
> +			what.stop_step == ss_default
> +			  ? "ss_default (ss_print_yes (stop_step 0))"
> +			  : what.stop_step == ss_print_no
> +			    ? "ss_print_no (stop_step 1)"
> +			    : "ss_print_yes (stop_step 0)");
> +
> +  gdb_assert (what.perform != pe_undef);
> +  fprintf_unfiltered (gdb_stdlog, _("infrun: %s: perform %s\n"), event,
> +		      what.perform == pe_check_more
> +			? "pe_check_more"
> +			: what.perform == pe_stop
> +			  ? "pe_stop" : what.perform == pe_stop_end_range
> +					? "pe_stop_end_range" : "pe_going");
> +
> +  if (print_defaults || what.stepping_over_breakpoint)
> +    fprintf_unfiltered (gdb_stdlog,
> +			_("infrun: %s: stepping_over_breakpoint %s\n"), event,
> +			what.stepping_over_breakpoint ? "yes" : "no");
> +}
> +
>  /* Given an execution control state that has been freshly filled in
>     by an event from the inferior, figure out what it means and take
>     appropriate action.  */
> @@ -4033,185 +4072,104 @@ process_event_stop_test:
>        return;
>      }
>  
> -  /* Handle cases caused by hitting a breakpoint.  */
> +  /* Breakpoints may get deleted and created in the block below.  It calls
> +     reinit_frame_cache thus invalidating current_frame.  In this block one
> +     needs to explicitly get_current_frame.  */
> +  frame = NULL;
> +  gdbarch = NULL;
> +
>    {
> -    CORE_ADDR jmp_buf_pc;
>      struct bpstat_what what;
>  
>      what = bpstat_what (ecs->event_thread->stop_bpstat);
>  
> -    if (what.call_dummy)
> +    if (what.bp_longjmp)
>        {
> -	stop_stack_dummy = what.call_dummy;
> -      }
> +	struct frame_info *frame = get_current_frame ();
> +	struct gdbarch *gdbarch = get_frame_arch (frame);
>  
> -    switch (what.main_action)
> -      {
> -      case BPSTAT_WHAT_SET_LONGJMP_RESUME:
>  	/* If we hit the breakpoint at longjmp while stepping, we
>  	   install a momentary breakpoint at the target of the
>  	   jmp_buf.  */
>  
> -	if (debug_infrun)
> -	  fprintf_unfiltered (gdb_stdlog,
> -			      "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
> -
> -	ecs->event_thread->stepping_over_breakpoint = 1;
> -
> -	if (!gdbarch_get_longjmp_target_p (gdbarch)
> -	    || !gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
> +	CORE_ADDR jmp_buf_pc;
> +	if (gdbarch_get_longjmp_target_p (gdbarch)
> +	    && gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
>  	  {
> -	    if (debug_infrun)
> -	      fprintf_unfiltered (gdb_stdlog, "\
> -infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
> -	    keep_going (ecs);
> -	    return;
> -	  }
> -
> -	/* We're going to replace the current step-resume breakpoint
> -	   with a longjmp-resume breakpoint.  */
> -	delete_step_resume_breakpoint (ecs->event_thread);
> -
> -	/* Insert a breakpoint at resume address.  */
> -	insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
> -
> -	keep_going (ecs);
> -	return;
> +	    /* We're going to replace the current step-resume breakpoint
> +	       with a longjmp-resume breakpoint.  */
> +	    delete_step_resume_breakpoint (ecs->event_thread);
>  
> -      case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
> -        if (debug_infrun)
> -	  fprintf_unfiltered (gdb_stdlog,
> -			      "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
> +	    /* Insert a breakpoint at resume address.  */
> +	    insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
> +	  }
> +      }
>  
> +    if (what.bp_longjmp_resume)
> +      {
>  	gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
>  	delete_step_resume_breakpoint (ecs->event_thread);
> +      }
>  
> -	ecs->event_thread->stop_step = 1;
> -	print_stop_reason (END_STEPPING_RANGE, 0);
> -	stop_stepping (ecs);
> -	return;
> -
> -      case BPSTAT_WHAT_SINGLE:
> -        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.  */
> -	break;
> -
> -      case BPSTAT_WHAT_STOP_NOISY:
> -        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
> -	   cleanup chain, so no need to worry about it here.  */
> -
> -	stop_stepping (ecs);
> -	return;
> -
> -      case BPSTAT_WHAT_STOP_SILENT:
> -        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
> -	   cleanup chain, so no need to worry about it here.  */
> -
> -	stop_stepping (ecs);
> -	return;
> -
> -      case BPSTAT_WHAT_STEP_RESUME:
> -        if (debug_infrun)
> -	  fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
> -
> +    if (what.bp_step_resume_on_stop)
> +      {
>  	delete_step_resume_breakpoint (ecs->event_thread);
>  	if (ecs->event_thread->step_after_step_resume_breakpoint)
>  	  {
>  	    /* Back when the step-resume breakpoint was inserted, we
>  	       were trying to single-step off a breakpoint.  Go back
> -	       to doing that.  */
> +	       to doing that.  pe_going must override pe_check_more so
> +	       that we do not stop again on that breakpoint.  */
>  	    ecs->event_thread->step_after_step_resume_breakpoint = 0;
> -	    ecs->event_thread->stepping_over_breakpoint = 1;
> -	    keep_going (ecs);
> -	    return;
> +	    what.stepping_over_breakpoint = 1;
> +	    gdb_assert (pe_check_more < pe_going);
> +	    if (what.perform < pe_going)
> +	      what.perform = pe_going;
>  	  }
> -	if (stop_pc == ecs->stop_func_start
> -	    && execution_direction == EXEC_REVERSE)
> +	else if (stop_pc == ecs->stop_func_start
> +		 && execution_direction == EXEC_REVERSE)
>  	  {
>  	    /* We are stepping over a function call in reverse, and
>  	       just hit the step-resume breakpoint at the start
>  	       address of the function.  Go back to single-stepping,
>  	       which should take us back to the function call.  */
> -	    ecs->event_thread->stepping_over_breakpoint = 1;
> -	    keep_going (ecs);
> -	    return;
> +	    what.stepping_over_breakpoint = 1;
> +	    gdb_assert (pe_check_more < pe_going);
> +	    if (what.perform < pe_going)
> +	      what.perform = pe_going;
>  	  }
> -	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 ();
> +	else
> +	  {
> +	    if (what.perform < pe_check_more)
> +	      what.perform = pe_check_more;
> +	  }
> +      }
>  
> -        /* We want to step over this breakpoint, then keep going.  */
> -        ecs->event_thread->stepping_over_breakpoint = 1;
> +    stop_print_frame = what.print_frame == pf_yes;
>  
> -        break;
> +    ecs->event_thread->stop_step = what.stop_step == ss_print_no;
>  
> -      case BPSTAT_WHAT_LAST:
> -	/* Not a real code, but listed here to shut up gcc -Wall.  */
> +    if (what.stepping_over_breakpoint)
> +      ecs->event_thread->stepping_over_breakpoint = 1;
>  
> -      case BPSTAT_WHAT_KEEP_CHECKING:
> +    switch (what.perform)
> +    {
> +      case pe_check_more:
> +	/* Still need to check other stuff, at least the case
> +	   where we are stepping and step out of the right range.  */
>  	break;
> -      }
> +      case pe_stop_end_range:
> +	print_stop_reason (END_STEPPING_RANGE, 0);
> +	/* FALLTHRU */
> +      case pe_stop:
> +	/* We are about to nuke the step_resume_breakpointt via the
> +	   cleanup chain, so no need to worry about it here.  */
> +	stop_stepping (ecs);
> +	return;
> +      case pe_going:
> +	keep_going (ecs);
> +	return;
> +    }
>    }
>  
>    /* We come here if we hit a breakpoint but should not
> @@ -4345,6 +4303,7 @@ infrun: not switching back to stepped thread, it has vanished\n");
>       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.
>  
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nostdlib.c
> @@ -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)
> +{
> +}
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/nostdlib.exp
> @@ -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"
> 


-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]