Re: [RFC] stepping over permanent breakpoint

[Aleksandar Ristovski <aristovski@qnx.com>]

[-- Attachment #1: Type: text/plain, Size: 4429 bytes --]

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.




[-- Attachment #2: infrun.c.diff --]
[-- Type: text/x-patch, Size: 606 bytes --]

@@ -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