Re: [RFC] stepping over permanent breakpoint

[Pedro Alves <pedro at codesourcery dot com>]

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?  :-)

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

-- 
Pedro Alves