Re: [patch/ob] not_a_breakpoint -> not_a_sw_breakpoint

[Kevin Buettner <kevinb@redhat.com>]

On Aug 16,  1:34pm, Andrew Cagney wrote:

> > On Aug 16, 11:37am, Andrew Cagney wrote:
> > 
> > 
> >> This patch renames the ``not_a_breakpoint'' parameter of 
> >> bpstat_stop_status() to the more correct ``not_a_sw_breakpoint'' so that 
> >> it is clear that it is indicating nothing about hardware breakpoints.
> > 
> > 
> > This may be a good change, but I don't think it's obvious.  Could
> > you at least explain the reasoning that lead you to conclude that
> > the parameter in question indicates nothing about hardware breakpoints?
> 
> The variable is used vis:
> 
> bpstat_stop_status (CORE_ADDR *pc, int not_a_sw_breakpoint)
> {
> [...]
>    /* Get the address where the breakpoint would have been.  The
>       "not_a_sw_breakpoint" argument is meant to distinguish between a
>       breakpoint trap event and a trace/singlestep trap event.  For a
>       trace/singlestep trap event, we would not want to subtract
>       DECR_PC_AFTER_BREAK from the PC. */
> 
>    bp_addr = *pc - (not_a_sw_breakpoint && !SOFTWARE_SINGLE_STEP_P () ?
>                     0 : DECR_PC_AFTER_BREAK);
> 
> 
> Later in the code appears:
> 
>            if (DECR_PC_AFTER_HW_BREAK != 0)
>              {
>                *pc = *pc - DECR_PC_AFTER_HW_BREAK;
>                write_pc (*pc);
>              }
> 
> if not_a_sw_breakpoint applied to hardware breakpoints then the above 
> decrement would be guarded by not_a_sw_breakpoint.
> 
> BTW, an even more correct name is ``not_a_sw_breakpoint_trap''. 
> However, Joel might end up adding something better than that.

Your reasoning is sound so long as we assume that the code
that you're basing your reasoning on isn't bitrotted.  (I only see
one target with a non-zero DECR_PC_AFTER_HW_BREAK, and I'll bet that
hasn't been tested in a while.)

What I'd like to be convinced of is that the conditions which are
used to instantiate ``not_a_sw_breakpoint'' really imply that that
we could be stopped due to a hardware watchpoint, but not a software
breakpoint trap.

The conditions in question are:

    currently_stepping (ecs) && prev_pc != stop_pc - DECR_PC_AFTER_BREAK

and (worse still):

    (currently_stepping (ecs)
     && prev_pc !=
     stop_pc - DECR_PC_AFTER_BREAK
     && !(step_range_end
	  && INNER_THAN (read_sp (),
			 (step_sp -
			  16))))

Actually, I'm not convinced that the name "not a breakpoint" (whether
they be software or otherwise) is a good characterization of these
conditions.  That said, I don't have a better suggestion.

(My head hurts whenever I look at this code.)

Kevin