Re: [RFC] stepping over permanent breakpoint

[Aleksandar Ristovski <aristovski at qnx dot com>]

Pedro Alves wrote:
> On Monday 16 March 2009 17:40:49, Aleksandar Ristovski wrote:
> > Hello,
> >
> > When there is a hard-coded breakpoint in code, like in this 
> > example (for x86):
> >
> > #include <stdio.h>
> >
> > int main()
> > {
> >    __asm("       int $0x03\n");
> >    printf("Hello World\n");
> >    return 0;
> > }
> >
> > gdb on linux will appear to work correctly.
> >
> > However, on systems that do not need pc adjustment after 
> > break (like QNX) gdb will not be able to step over that 
> > breakpoint  (...)
>
> > (...) unless user explicitly sets a breakpoint on top  
> > of it.
>
> Which I think your patch breaks?  :-)

No, it doesn't, it will behave as before. Observe where is 
the code I added, it is inside
if (gdbarch_decr_pc_after_break (gdbarch) == 0)
so for linux, it won't even be executed.

> > I think that in case of linux it is actually working by 
> > accident - because kernel does not back-up instruction 
> > pointer after hard-coded breakpoint instruction was 
> > executed. Gdb will receive SIGTRAP but will not really know why.
> >
> > Attached patch fixes this for systems where 
> > gdbarch_decr_pc_after_break (gdbarch) == 0
> >
> > I am still not sure this is the final fix. Wouldn't it be 
> > better if we recognized a hard-coded breakpoint as a 
> > breakpoint? There would be an issue since it is not in the 
> > breakpoint list, but maybe we should either automatically 
> > add it when we encounter it, or perhaps print with some 
> > "special" number (to make it clear to the user it is not one 
> > of the user-generated breakpoints).
>
> How about if you do the detection on resume instead?
> (please forgive my manual-patch-writing-in-email skills)
>
> infrun.c:resume:
>
>   /* Normally, by the time we reach `resume', the breakpoints are either
>      removed or inserted, as appropriate.  The exception is if we're sitting
>      at a permanent breakpoint; we need to step over it, but permanent
>      breakpoints can't be removed.  So we have to test for it here.  */
> -  if (breakpoint_here_p (pc) == permanent_breakpoint_here)
> +  if (pc == stop_pc
> +      && gdbarch_decr_pc_after_break (gdbarch) == 0
> +      && (breakpoint_here_p (pc) == permanent_breakpoint_here
> +          || hardcoded_breakpoint_inserted_here_p (pc)))
>     {
>       if (gdbarch_skip_permanent_breakpoint_p (gdbarch))
> 	gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
>       else
> 	error (_("\
> The program is stopped at a permanent breakpoint, but GDB does not know\n\
> how to step past a permanent breakpoint on this architecture.  Try using\n\
> a command like `return' or `jump' to continue execution."));
>     }
>
> Then, have to make sure all decr_pc_after_break == 0 archs implement
> gdbarch_skip_permanent_breakpoint.  Maybe change the default to just
> skip the breakpoint op, like i386_skip_permanent_breakpoint.  I wonder
> why that isn't the case today?
>
> Hmmm, actually, why isn't this done on `proceed' instead of on `resume':
>
> infrun.c:proceed ():
> (...)
>   if (addr == (CORE_ADDR) -1)
>     {
> +    if (pc == stop_pc
> +         && gdbarch_decr_pc_after_break (gdbarch) == 0
> +         && execution_direction != EXEC_REVERSE
> +         && (breakpoint_here_p (pc) == permanent_breakpoint_here
> +             || hardcoded_breakpoint_inserted_here_p (pc)))
> +	gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
> -     if (pc == stop_pc && breakpoint_here_p (pc) 
> +     else if (pc == stop_pc && breakpoint_here_p (pc) 
>            && execution_direction != EXEC_REVERSE)
>
> ?
>
> What do you think?  What do others think?
>
> One thing this changes if that on decr_pc_after_break == 0 targets, if
> you single-step into a hardcoded breakpoint trap, and then issue
> a "continue", you'll not get a SIGTRAP reported, instead it is
> silently skipped.  Not sure if that's a problem, and if it is, if it is
> worth tackling.  I can't see how easily to fix it without having a
> "had been stepping before" thread flag, that isn't cleared by
> clear_proceed_status.


I have tried path similar to what you suggest. It seems more 
correct, but I would think that in addition to what you are 
doing, it would also need a change in adjust_pc_after_break 
to still decrement PC (to point to just-hit hardcoded 
breakpoint). Normally, adjust_pc_after_break will (on linux) 
miss this case and leave pc to point to instruction 
following breakpoint instruction.



@@ -2106,7 +2121,8 @@ adjust_pc_after_break (struct execution_
      SIGTRAPs, we keep a list of such breakpoint locations for a bit,
      and retire them after a number of stop events are reported.  */
   if (software_breakpoint_inserted_here_p (breakpoint_pc)
-      || (non_stop && moribund_breakpoint_here_p (breakpoint_pc)))
+      || (non_stop && moribund_breakpoint_here_p (breakpoint_pc))
+      || hardcoded_breakpoint_inserted_here_p (breakpoint_pc))
     {
       /* When using hardware single-step, a SIGTRAP is reported for both
 	 a completed single-step and a software breakpoint.  Need to