[RFC] stepping over permanent breakpoint

[RFC] stepping over permanent breakpoint

[Aleksandar Ristovski]

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

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.

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


Thanks,

Aleksandar Ristovski
QNX Software Systems



ChangeLog for your reference:

* infrun.c (adjust_pc_after_break): On systems with adjusted 
PC after break, make sure hard-coded breakpoint is stepped-over.

[-- Attachment #2: infrun.c-adjust_pc_after_break-permanent-breakpoints-20090316.diff --]
[-- Type: text/x-patch, Size: 1309 bytes --]

Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.361
diff -u -p -r1.361 infrun.c
--- gdb/infrun.c	1 Mar 2009 23:18:36 -0000	1.361
+++ gdb/infrun.c	16 Mar 2009 17:33:22 -0000
@@ -2089,14 +2089,29 @@ adjust_pc_after_break (struct execution_
      we have nothing to do.  */
   regcache = get_thread_regcache (ecs->ptid);
   gdbarch = get_regcache_arch (regcache);
-  if (gdbarch_decr_pc_after_break (gdbarch) == 0)
-    return;
 
   /* Find the location where (if we've hit a breakpoint) the
      breakpoint would be.  */
   breakpoint_pc = regcache_read_pc (regcache)
 		  - gdbarch_decr_pc_after_break (gdbarch);
 
+  if (gdbarch_decr_pc_after_break (gdbarch) == 0)
+    {
+      /* Check if we have stopped at a permanent breakpoint.  */
+      int len;
+      const gdb_byte *brk;
+      gdb_byte *target_mem = alloca(32);
+
+      brk = gdbarch_breakpoint_from_pc (gdbarch, &breakpoint_pc, &len);
+      if (!target_read_memory (breakpoint_pc, target_mem, len)
+	  && memcmp (target_mem, brk, len) == 0)
+	{
+	  breakpoint_pc += len;
+	  regcache_write_pc (regcache, breakpoint_pc);
+	}
+      return;
+    }
+
   /* Check whether there actually is a software breakpoint inserted at
      that location.
 

Re: [RFC] stepping over permanent breakpoint

[Pedro Alves]

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

Re: [RFC] stepping over permanent breakpoint

[Aleksandar Ristovski]

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

Re: [RFC] stepping over permanent breakpoint

[Pedro Alves]

On Monday 16 March 2009 18:55:27, Aleksandar Ristovski wrote:
> Pedro Alves wrote:
> > On Monday 16 March 2009 17:40:49, Aleksandar Ristovski wrote:
> >> 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.

Not all architectures that run linux need PC adjustment.  You're
thinking x86-linux.  Anyway, I meant that you're breaking setting
a user breakpoint on top of a permanent breakpoint.  Try
setting a breakpoint with "break *int3_addr", on top
of that int3, and running to it.  When it is hit, you're moving
the PC passed it, so later calls to bpstat_stop_status like:

      /* See if there is a breakpoint at the current PC.  */
      ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);

... will not see the permanent breakpoint, right?

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

Yeah, I considered that, but I think that it is legitimate to
want to pass SIGTRAPs to the inferior, and have a SIGTRAP handler
see whatever it would see if GDB wasn't there.  This may be
useful for debugging programs that embed a gdb stub in process,
for example.  So, on decr_pc_after_break != 0 targets, I'd leave
things as they are.

Mark's point about considering a trap instruction as a normal
instruction is valid, so I'm not sure if we'd want to do this
skipping by default or not.  I'll let you guys fight
over it.  :-)

-- 
Pedro Alves

Re: [RFC] stepping over permanent breakpoint

[Aleksandar Ristovski]

Pedro Alves wrote:
> 
> Not all architectures that run linux need PC adjustment.  You're
> thinking x86-linux.  Anyway, I meant that you're breaking setting
> a user breakpoint on top of a permanent breakpoint.  Try
> setting a breakpoint with "break *int3_addr", on top
> of that int3, and running to it.  When it is hit, you're moving
> the PC passed it, so later calls to bpstat_stop_status like:
> 
>       /* See if there is a breakpoint at the current PC.  */
>       ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
> 
> ... will not see the permanent breakpoint, right?


Hmm... very interesting. I am not seeing what you suggest 
(it works for me) but by looking at the code, I don't 
understand why - I agree it should be broken as you say, but 
for some reason it isn't. I will have to look into this a 
bit more (I am working in gdb 6.7).


> 
> Mark's point about considering a trap instruction as a normal
> instruction is valid, so I'm not sure if we'd want to do this
> skipping by default or not.  I'll let you guys fight
> over it.  :-)
> 

Other than wanting to make gdb on qnx work, I have no strong 
feelings about one way or the other... It was a thought.

Re: [RFC] stepping over permanent breakpoint

[Mark Kettenis]

> From: Aleksandar Ristovski <aristovski@qnx.com>
> Date:  Mon, 16 Mar 2009 13:40:49 -0400
> 
> 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.

Well, on Linux, that instruction will not be interpreted as a
permanent breakpoint, just like on QNX.

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

The big question here is whether a breakpoint trap instruction should
always be interpreted as a permanent breakpoint in GDB or that it only
gets interpreted as such if you actually tell GDB about it.  Up until
now, we've always done the latter.  If you don't tell GDB, random
breakpoint trap instructions are handled as normal instructions and
you get to see whatever the architecture/OS does for these
instructions.

> 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

If you want to fix things, it should be fixed for *all* systems.  

Re: [RFC] stepping over permanent breakpoint

[Aleksandar Ristovski]

Mark Kettenis wrote:
>> From: Aleksandar Ristovski <aristovski@qnx.com>
>> Date:  Mon, 16 Mar 2009 13:40:49 -0400
>>
>> 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.
> 
> Well, on Linux, that instruction will not be interpreted as a
> permanent breakpoint, just like on QNX.

Except on QNX gdb will not be able to continue or step over 
that instruction.

> 
>> 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.
> 
> The big question here is whether a breakpoint trap instruction should
> always be interpreted as a permanent breakpoint in GDB or that it only
> gets interpreted as such if you actually tell GDB about it.  Up until
> now, we've always done the latter.  If you don't tell GDB, random
> breakpoint trap instructions are handled as normal instructions and
> you get to see whatever the architecture/OS does for these
> instructions.

Yes, this is my dilemma. I think we could print more 
informative message, but I am not sure.

> 
>> 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
> 
> If you want to fix things, it should be fixed for *all* systems.  
> 

What I proposed only brings in line those systems and makes 
them able to continue after hitting a permanent breakpoint 
(so on systems that normally do not need adjustment, I added 
advancing over the hardcoded breakpoint).

RFC: Program Breakpoints (was: [RFC] stepping over permanent breakpoint)

[Ross Morley]

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

It was interesting to see the matter of skipping permanent breakpoints
come up on this list. This is very similar to an issue we dealt with
a couple of years ago with Xtensa and remote targets. Our solution
has been stable for at least two years, but has not yet been prepared
for submission because I've had higher priorities (and at that time we
were way out of sync with mainline GDB). The issue is actually more
complicated than the forgoing discussion suggests.

I'd like to discuss it here and present our solution. I'll attach a
patch for our current solution relative to GDB 6.8, but I do propose
to revise it to improve some areas based on our experience (in the
next few weeks) - I'll discuss that toward the end of this post.


The Problem
-----------

When the target program hits a breakpoint of any kind, GDB receives
a SIGTRAP event. As Aleksander pointed out, if the PC is not in the
breakpoint table GDB stops because it doesn't know what to do with
this event. In fact it doesn't always stop. When GDB is stepping
(for example when a software watchpoint is set) it sees the SIGTRAP
as a normal step event, so it continues stepping. If the break
instruction didn't increment the PC, it keeps executing the same
break instruction forever. This is very frustrating for users (who
reported this as a bug) because when you have a s/w watchpoint and
are stepping, execution is expected to be slow, so you wait a long
time before you realize it's hung.


Terminology
-----------

I use the term "program breakpoint" to denote a break instruction that
is part of the program, that GDB did not plant nor know about. There is
a feature called "permanent breakpoint" that is a little different in
that GDB knows about it (it's in the breakpoint table). I didn't want to
use the same term for something different (this is open to discussion).
As far as I can tell, only the HPPA arch uses permanent breakpoints.
It would be good to reconcile the two, but I haven't attempted that.


The Solution
------------

The first part is to recognize when you hit a program break.
The second is to step over it when you want to resume.

Note that the instruction that represents a program break is entirely
arch-specific, and need not be the same one GDB plants for breakpoints.
There might be several instructions or variations.

One solution we rejected was to have GDB probe the code at the PC
(if not at a known planted breakpoint). There are two problems:
1. GDB has to know which instructions to look for.
2. GDB has to probe memory every instruction when stepping, which
   can seriously hurt performance when there are s/w watchpoints
   (especially with remote targets).

We decided it was best to have the target agent communicate that
it is signalling a breakpoint. Note that the target agent might
have no idea whether the break was planted by GDB, so it's up to
GDB to distinguish by looking up the PC in its breakpoint table.

Currently only a SIGTRAP is communicated, and GDB then looks up the
PC in its breakpoint table to see if it hit a planted breakpoint.
Otherwise, if stepping then GDB assumes it's a single-step trap,
else GDB gives up and stops without explanation.

For Xtensa we use only the remote protocol. The target signals a "T"
stop-reply packet with a trap signal number. It does this whether
it hit a break or was stepping. I needed to distinguish the cases.
There are already extensions to the "T" packet (eg. "watch", "awatch",
"rwatch", "thread"). I added another:
    TAApbreak:kind
where the suffix "kind" is a number that gives arch-specific info
about the type of breakpoint, including its size (for skipping).

A target interface function and macro is defined so the higher
levels of GDB can find out whether it was stopped by a program
breakpoint (as opposed to a normal GDB planted breakpoint).
STOPPED_BY_PROGRAM_BREAKPOINT(k) returns true or false, and
writes the "kind" argument into (int *) k.

Some questions arise which I'm sure are hitting your neurons about
now. I'll address some of them before I go on.

- What about target agents that don't (yet) implement the extension?
  Behavior is exactly as before: GDB will just treat the SIGTRAP
  as either a step or an unknown event.

- Why "pbreak"?
  Any extension that starts with letters a-f requires a special
  case in the parser in remote.c which thinks it's a hex number.
  There is already a special case to handle "awatch". I wanted
  to avoid that (it turns out the parser also treats "p" special
  for currthread, so I will propose using "TAAtrap" - later).

- What about non-remote targets?
  The target agent would have to communicate the information
  another way. If not, GDB behavior is exactly as before. I'm not
  sure whether this problem even exists for non-remote targets
  since I haven't looked into that. We define a target interface
  function and macro STOPPED_BY_PROGRAM_BREAKPOINT(k) which
  currently returns 0 (false) for non-remote targets.

- Why not use a special signal instead of a remote protocol extension?
  There is no clear signal that is safe to use. GDB uses just
  SIGTRAP and SIGINT, and already overloads SIGTRAP for most things.
  The use of a remote protocol extension to disambiguate is well
  precedented.

- Isn't the size of a breakpoint constant for a given arch?
  No. There can be several kinds used for different reasons.
  There can be several encodings on a single break instruction
  (Xtensa has the normal 3 byte and a "density" 2 byte encoding).

- Can't GDB figure out the size of the break by itself?
  Of course GDB could disassemble the instruction. But that needs
  another remote protocol exchange to get the code. Why not have
  the debug agent just pass it along with the signal?

Having discovered that it stopped for a program breakpoint, GDB
is able to report the fact to both the CLI and MI. I also defined
a gdbarch function to report the meaning of "kind" in an arch-
specific way. This improves the debugging experience when the
target uses different kinds of breakpoints to denote different
reasons for stopping. (However, in the interest of simplicity,
my revised proposal won't report all that - it is anyway obvious
from a disassembly if not a symbol associated with the PC; GDB
itself only needs to know the size to skip the breakpoint).

When the user elects to continue after a program breakpoint,
GDB increments the PC by the size of the break that was reported
in the "kind" argument. A gdbarch function handles this. Of
course it might do nothing if the PC was already advanced.

I haven't discussed all the implementation details here.
Please see the attached 6.8 based patch for the actual code.
Note this patch does not pretend to be ready for submission!


Problems with Current Implementation and Proposed Simplification
----------------------------------------------------------------

1. "TAApbreak" requires a special case in the remote stop-reply
   packet parser to differentiate it from {S,T}AA:p<currthread>.
   This is really a parser issue, but I'd rather avoid the need
   to rewrite it or add another special case. I propose "TAAtrap".

2. The "kind" argument of "TAApbreak:kind" can encode some more
   information the user might be interested in, but the cost is
   adding two gdbarch functions. Yet the user can figure out the
   meaning from a disassembly. The only information GDB itself
   really needs is the size, so it can skip the breakpoint.

3. Non-remote targets aren't handled. However that should be
   just a matter of adding an implementation to the target
   interface, and nothing breaks meanwhile.

4. There are cases in which a target OS or debug agent uses a
   break to trigger an exception of some kind, and then unwinds
   the stack to before the exception (eg. in the Xtensa ISS).
   Then when GDB tries to skip the break, it skips real code.


Proposed Improved, Simplified Solution
--------------------------------------

The remote protocol extension would be:
    TAAtrap[:size]
where ":size" is optional and may only be provided to GDB by the
target agent if the PC is in fact pointing to the instruction
that caused the break, and if omitted is taken to be 0.
GDB will skip the break by PC += size (no effect if size is 0).
Note it is not necessary to call gdbarch_breakpoint_from_pc().

The gdbarch functions to extract the size and decipher "kind"
are not needed. The target interface function
    STOPPED_BY_PROGRAM_BREAKPOINT(k)
becomes
    STOPPED_BY_PROGRAM_BREAKPOINT(size)
where 'size' is an address in which the size is returned.

We might also want to consider calling it "program trap" and
keep the term "breakpoint" for things that GDB knows about.


Comments are welcome. We at Tensilica would like to see this
refined and incorporated into mainline GDB.

This can certainly coexist with permanent breakpoints,
however it is (I think) a bit more general. If the people
who use permanent breakpoints would care to comment,
perhaps we can somehow reconcile these into one feature.

Thanks,
Ross Morley
Tensilca, Inc.
ross@tensilica.com


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.
>
> 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).
>
>
> Thanks,
>
> Aleksandar Ristovski
> QNX Software Systems
>
>
>
> ChangeLog for your reference:
>
> * infrun.c (adjust_pc_after_break): On systems with adjusted PC after 
> break, make sure hard-coded breakpoint is stepped-over.
>
>------------------------------------------------------------------------
>
>Index: gdb/infrun.c
>===================================================================
>RCS file: /cvs/src/src/gdb/infrun.c,v
>retrieving revision 1.361
>diff -u -p -r1.361 infrun.c
>--- gdb/infrun.c	1 Mar 2009 23:18:36 -0000	1.361
>+++ gdb/infrun.c	16 Mar 2009 17:33:22 -0000
>@@ -2089,14 +2089,29 @@ adjust_pc_after_break (struct execution_
>      we have nothing to do.  */
>   regcache = get_thread_regcache (ecs->ptid);
>   gdbarch = get_regcache_arch (regcache);
>-  if (gdbarch_decr_pc_after_break (gdbarch) == 0)
>-    return;
> 
>   /* Find the location where (if we've hit a breakpoint) the
>      breakpoint would be.  */
>   breakpoint_pc = regcache_read_pc (regcache)
> 		  - gdbarch_decr_pc_after_break (gdbarch);
> 
>+  if (gdbarch_decr_pc_after_break (gdbarch) == 0)
>+    {
>+      /* Check if we have stopped at a permanent breakpoint.  */
>+      int len;
>+      const gdb_byte *brk;
>+      gdb_byte *target_mem = alloca(32);
>+
>+      brk = gdbarch_breakpoint_from_pc (gdbarch, &breakpoint_pc, &len);
>+      if (!target_read_memory (breakpoint_pc, target_mem, len)
>+	  && memcmp (target_mem, brk, len) == 0)
>+	{
>+	  breakpoint_pc += len;
>+	  regcache_write_pc (regcache, breakpoint_pc);
>+	}
>+      return;
>+    }
>+
>   /* Check whether there actually is a software breakpoint inserted at
>      that location.
> 
>  
>

[-- Attachment #2: xtensa_program_bp.patch --]
[-- Type: text/x-patch, Size: 23862 bytes --]

diff -urN gdb-6.8-orig/gdb/gdbarch.c gdb-6.8-new/gdb/gdbarch.c
--- gdb-6.8-orig/gdb/gdbarch.c	2009-03-19 17:29:47.000000000 -0700
+++ gdb-6.8-new/gdb/gdbarch.c	2009-03-20 11:29:29.000000000 -0700
@@ -190,6 +190,8 @@
   gdbarch_memory_insert_breakpoint_ftype *memory_insert_breakpoint;
   gdbarch_memory_remove_breakpoint_ftype *memory_remove_breakpoint;
   CORE_ADDR decr_pc_after_break;
+  gdbarch_skip_program_breakpoint_ftype *skip_program_breakpoint;
+  gdbarch_describe_program_breakpoint_ftype *describe_program_breakpoint;
   CORE_ADDR deprecated_function_start_offset;
   gdbarch_remote_register_number_ftype *remote_register_number;
   gdbarch_fetch_tls_load_module_address_ftype *fetch_tls_load_module_address;
@@ -312,6 +314,8 @@
   default_memory_insert_breakpoint,  /* memory_insert_breakpoint */
   default_memory_remove_breakpoint,  /* memory_remove_breakpoint */
   0,  /* decr_pc_after_break */
+  0,  /* skip_program_breakpoint */
+  0,  /* describe_program_breakpoint */
   0,  /* deprecated_function_start_offset */
   default_remote_register_number,  /* remote_register_number */
   0,  /* fetch_tls_load_module_address */
@@ -550,6 +554,10 @@
   /* Skip verify of memory_insert_breakpoint, invalid_p == 0 */
   /* Skip verify of memory_remove_breakpoint, invalid_p == 0 */
   /* Skip verify of decr_pc_after_break, invalid_p == 0 */
+  if (gdbarch->skip_program_breakpoint == 0)
+    gdbarch->skip_program_breakpoint = default_skip_program_breakpoint;
+  if (gdbarch->describe_program_breakpoint == 0)
+    gdbarch->describe_program_breakpoint = default_describe_program_breakpoint;
   /* Skip verify of deprecated_function_start_offset, invalid_p == 0 */
   /* Skip verify of remote_register_number, invalid_p == 0 */
   /* Skip verify of fetch_tls_load_module_address, has predicate */
@@ -709,6 +717,9 @@
                       "gdbarch_dump: deprecated_function_start_offset = 0x%s\n",
                       paddr_nz (gdbarch->deprecated_function_start_offset));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: describe_program_breakpoint = <0x%lx>\n",
+                      (long) gdbarch->describe_program_breakpoint);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: double_bit = %s\n",
                       paddr_d (gdbarch->double_bit));
   fprintf_unfiltered (file,
@@ -940,6 +951,9 @@
                       "gdbarch_dump: skip_permanent_breakpoint = <0x%lx>\n",
                       (long) gdbarch->skip_permanent_breakpoint);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: skip_program_breakpoint = <0x%lx>\n",
+                      (long) gdbarch->skip_program_breakpoint);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: skip_prologue = <0x%lx>\n",
                       (long) gdbarch->skip_prologue);
   fprintf_unfiltered (file,
@@ -2183,6 +2197,40 @@
   gdbarch->decr_pc_after_break = decr_pc_after_break;
 }
 
+void
+gdbarch_skip_program_breakpoint (struct gdbarch *gdbarch, int kind)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->skip_program_breakpoint != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_skip_program_breakpoint called\n");
+  gdbarch->skip_program_breakpoint (kind);
+}
+
+void
+set_gdbarch_skip_program_breakpoint (struct gdbarch *gdbarch,
+                                     gdbarch_skip_program_breakpoint_ftype skip_program_breakpoint)
+{
+  gdbarch->skip_program_breakpoint = skip_program_breakpoint;
+}
+
+const char *
+gdbarch_describe_program_breakpoint (struct gdbarch *gdbarch, int kind)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->describe_program_breakpoint != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_describe_program_breakpoint called\n");
+  return gdbarch->describe_program_breakpoint (kind);
+}
+
+void
+set_gdbarch_describe_program_breakpoint (struct gdbarch *gdbarch,
+                                         gdbarch_describe_program_breakpoint_ftype describe_program_breakpoint)
+{
+  gdbarch->describe_program_breakpoint = describe_program_breakpoint;
+}
+
 CORE_ADDR
 gdbarch_deprecated_function_start_offset (struct gdbarch *gdbarch)
 {
diff -urN gdb-6.8-orig/gdb/gdbarch.h gdb-6.8-new/gdb/gdbarch.h
--- gdb-6.8-orig/gdb/gdbarch.h	2009-03-19 17:29:47.000000000 -0700
+++ gdb-6.8-new/gdb/gdbarch.h	2009-03-20 11:29:24.000000000 -0700
@@ -399,6 +399,21 @@
 extern CORE_ADDR gdbarch_decr_pc_after_break (struct gdbarch *gdbarch);
 extern void set_gdbarch_decr_pc_after_break (struct gdbarch *gdbarch, CORE_ADDR decr_pc_after_break);
 
+/* TENSILICA_LOCAL
+   Program breakpoints. These are break instructions in the target code,
+   not planted by GDB. Define this to skip program breakpoints: */
+
+typedef void (gdbarch_skip_program_breakpoint_ftype) (int kind);
+extern void gdbarch_skip_program_breakpoint (struct gdbarch *gdbarch, int kind);
+extern void set_gdbarch_skip_program_breakpoint (struct gdbarch *gdbarch, gdbarch_skip_program_breakpoint_ftype *skip_program_breakpoint);
+
+/* TENSILICA_LOCAL
+   Define this to describe program breakpoints: */
+
+typedef const char * (gdbarch_describe_program_breakpoint_ftype) (int kind);
+extern const char * gdbarch_describe_program_breakpoint (struct gdbarch *gdbarch, int kind);
+extern void set_gdbarch_describe_program_breakpoint (struct gdbarch *gdbarch, gdbarch_describe_program_breakpoint_ftype *describe_program_breakpoint);
+
 /* A function can be addressed by either it's "pointer" (possibly a
    descriptor address) or "entry point" (first executable instruction).
    The method "convert_from_func_ptr_addr" converting the former to the
diff -urN gdb-6.8-orig/gdb/gdbarch.sh gdb-6.8-new/gdb/gdbarch.sh
--- gdb-6.8-orig/gdb/gdbarch.sh	2009-03-19 17:29:47.000000000 -0700
+++ gdb-6.8-new/gdb/gdbarch.sh	2009-03-20 11:29:09.000000000 -0700
@@ -484,6 +484,14 @@
 m:int:memory_remove_breakpoint:struct bp_target_info *bp_tgt:bp_tgt:0:default_memory_remove_breakpoint::0
 v:CORE_ADDR:decr_pc_after_break:::0:::0
 
+# TENSILICA_LOCAL
+# Program breakpoints. These are break instructions in the target code,
+# not planted by GDB. Define this to skip program breakpoints:
+f:void:skip_program_breakpoint:int kind:kind:::default_skip_program_breakpoint::0
+# TENSILICA_LOCAL
+# Define this to describe program breakpoints:
+f:const char *:describe_program_breakpoint:int kind:kind:::default_describe_program_breakpoint::0
+
 # A function can be addressed by either it's "pointer" (possibly a
 # descriptor address) or "entry point" (first executable instruction).
 # The method "convert_from_func_ptr_addr" converting the former to the
diff -urN gdb-6.8-orig/gdb/infcmd.c gdb-6.8-new/gdb/infcmd.c
--- gdb-6.8-orig/gdb/infcmd.c	2009-03-19 17:29:48.000000000 -0700
+++ gdb-6.8-new/gdb/infcmd.c	2009-03-20 11:33:21.000000000 -0700
@@ -696,6 +696,9 @@
   struct frame_info *frame;
   struct cleanup *cleanups = 0;
   int async_exec = 0;
+/* TENSILICA_LOCAL */
+  int program_break_kind;
+/* END TENSILICA_LOCAL */
 
   ERROR_NO_INFERIOR;
 
@@ -725,6 +728,33 @@
       else
         make_exec_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
     }
+/* TENSILICA_LOCAL */
+  else if (STOPPED_BY_PROGRAM_BREAKPOINT (&program_break_kind) && 
+	   read_pc() == stop_pc && count > 0)
+    {
+      /* "stepi" off program breakpoint: the first step is to just increment
+	 the PC past the break, then there are count-1 steps to go.
+	 Note proceed() won't be called the first time, and on subsequent
+	 steps the PC will already be off the break, so the entire handling
+	 of "stepi" off a program breakpoint is done here. If stopping after 
+	 the break, display location information as for normal_stop.  */
+      count--;
+      gdbarch_skip_program_breakpoint (current_gdbarch, program_break_kind);
+      if (count == 0)
+	{
+	  deprecated_update_frame_pc_hack (get_current_frame (), read_pc ());
+	  if (ui_out_is_mi_like_p (uiout))
+	    {
+	      ui_out_field_string (uiout, "reason", "end-stepping-range");
+	      ui_out_field_int (uiout, "thread-id",
+				pid_to_thread_id (inferior_ptid));
+	      print_stack_frame (get_selected_frame (NULL), -1, LOC_AND_ADDRESS);
+	    }
+	  else 
+	    print_stack_frame (get_selected_frame (NULL), -1, SRC_LINE);
+	}
+    }
+/* END TENSILICA_LOCAL */
 
   /* In synchronous case, all is well, just use the regular for loop. */
   if (!target_can_async_p ())
diff -urN gdb-6.8-orig/gdb/infrun.c gdb-6.8-new/gdb/infrun.c
--- gdb-6.8-orig/gdb/infrun.c	2009-03-19 17:29:48.000000000 -0700
+++ gdb-6.8-new/gdb/infrun.c	2009-03-20 11:36:37.000000000 -0700
@@ -511,6 +511,9 @@
 resume (int step, enum target_signal sig)
 {
   int should_resume = 1;
+/* TENSILICA_LOCAL */
+  int program_break_kind;
+/* END TENSILICA_LOCAL */
   struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
   QUIT;
 
@@ -549,6 +552,18 @@
 a command like `return' or `jump' to continue execution."));
     }
 
+/* TENSILICA_LOCAL */
+  /* Skip a program breakpoint, unless PC changed without running inferior. 
+     This is the same as skipping a permanent breakpoint, except that
+     the preceding macro does not provide an argument to differentiate
+     varieties of break instructions to skip, requiring the implementation
+     to probe and decode the target code. Also the SKIP_PERMANENT_BREAKPOINT
+     macro is used only by HPPA and is not in gdbarch.h.  */
+  if (STOPPED_BY_PROGRAM_BREAKPOINT (&program_break_kind) &&
+      read_pc() == stop_pc)
+    gdbarch_skip_program_breakpoint (current_gdbarch, program_break_kind);
+/* END TENSILICA_LOCAL */
+
   if (step && gdbarch_software_single_step_p (current_gdbarch))
     {
       /* Do it the hard way, w/temp breakpoints */
@@ -928,7 +943,11 @@
   /* Inferior exited. */
   EXITED,
   /* Inferior received signal, and user asked to be notified. */
-  SIGNAL_RECEIVED
+  SIGNAL_RECEIVED,
+/* TENSILICA_LOCAL */
+  /* Inferior stopped because of program breakpoint.  */
+  PROGRAM_BREAK
+/* END TENSILICA_LOCAL */
 };
 
 /* This structure contains what used to be local variables in
@@ -2007,6 +2026,9 @@
 	ecs->random_signal
 	  = !(bpstat_explains_signal (stop_bpstat)
 	      || stepping_over_breakpoint
+/* TENSILICA_LOCAL */
+	      || STOPPED_BY_PROGRAM_BREAKPOINT (NULL)
+/* END TENSILICA_LOCAL */
 	      || (step_range_end && step_resume_breakpoint == NULL));
       else
 	{
@@ -2329,6 +2351,23 @@
       }
   }
 
+/* TENSILICA_LOCAL */
+  /* Handle case of hitting a program breakpoint (break instruction
+     in target program, not planted by or known to GDB).  */
+  
+  if (STOPPED_BY_PROGRAM_BREAKPOINT (NULL))
+    {
+      stop_print_frame = 1;
+      
+      /* We are about to nuke the step_resume_breakpoint and
+	 through_sigtramp_breakpoint via the cleanup chain, so
+	 no need to worry about it here.  */
+      
+      stop_stepping (ecs);
+      return;
+    }
+/* END TENSILICA_LOCAL */
+
   /* We come here if we hit a breakpoint but should not
      stop for it.  Possibly we also were stepping
      and should stop for that.  So fall through and
@@ -3023,6 +3062,20 @@
 	    (uiout, "reason",
 	     async_reason_lookup (EXEC_ASYNC_END_STEPPING_RANGE));
       break;
+/* TENSILICA_LOCAL */
+    case PROGRAM_BREAK:
+      /* This stop reason is used only for program breaks, never planted. */
+      {
+	if (ui_out_is_mi_like_p (uiout))
+	  ui_out_field_string (uiout, "reason", "program-breakpoint");
+	ui_out_text (uiout, "\nProgram breakpoint \"");
+	ui_out_field_fmt (uiout, "break-insn", "%s", 
+			  gdbarch_describe_program_breakpoint (current_gdbarch, stop_info));
+	ui_out_text (uiout, "\", ");
+	/* to be followed by source location, as for planted breakpoint */
+      }
+      break;
+/* END TENSILICA_LOCAL */
     case SIGNAL_EXITED:
       /* The inferior was terminated by a signal. */
       annotate_signalled ();
@@ -3208,10 +3261,21 @@
 	  int source_flag;
 	  int do_frame_printing = 1;
 
-	  bpstat_ret = bpstat_print (stop_bpstat);
-	  switch (bpstat_ret)
+/* TENSILICA_LOCAL */
+	  /* Print program break stop reason and description. */
+
+	  if (STOPPED_BY_PROGRAM_BREAKPOINT (&bpstat_ret))
+	    {
+	      print_stop_reason (PROGRAM_BREAK, bpstat_ret);
+	      source_flag = SRC_AND_LOC;    /* print location and source line */
+	    }
+	  else
 	    {
-	    case PRINT_UNKNOWN:
+/* END TENSILICA_LOCAL */
+	      bpstat_ret = bpstat_print (stop_bpstat);
+	      switch (bpstat_ret)
+		{
+		case PRINT_UNKNOWN:
 	      /* If we had hit a shared library event breakpoint,
 		 bpstat_print would print out this message.  If we hit
 		 an OS-level shared library event, do the same
@@ -3224,30 +3288,34 @@
 		  break;
 		}
 
-	      /* FIXME: cagney/2002-12-01: Given that a frame ID does
-	         (or should) carry around the function and does (or
-	         should) use that when doing a frame comparison.  */
-	      if (stop_step
-		  && frame_id_eq (step_frame_id,
-				  get_frame_id (get_current_frame ()))
-		  && step_start_function == find_pc_function (stop_pc))
-		source_flag = SRC_LINE;	/* finished step, just print source line */
-	      else
-		source_flag = SRC_AND_LOC;	/* print location and source line */
-	      break;
-	    case PRINT_SRC_AND_LOC:
-	      source_flag = SRC_AND_LOC;	/* print location and source line */
-	      break;
-	    case PRINT_SRC_ONLY:
-	      source_flag = SRC_LINE;
-	      break;
-	    case PRINT_NOTHING:
-	      source_flag = SRC_LINE;	/* something bogus */
-	      do_frame_printing = 0;
-	      break;
-	    default:
-	      internal_error (__FILE__, __LINE__, _("Unknown value."));
+		  /* FIXME: cagney/2002-12-01: Given that a frame ID does
+		     (or should) carry around the function and does (or
+		     should) use that when doing a frame comparison.  */
+		  if (stop_step
+		      && frame_id_eq (step_frame_id,
+				      get_frame_id (get_current_frame ()))
+		      && step_start_function == find_pc_function (stop_pc))
+		    source_flag = SRC_LINE;	/* finished step, just print source line */
+		  else
+		    source_flag = SRC_AND_LOC;	/* print location and source line */
+		  break;
+		case PRINT_SRC_AND_LOC:
+		  source_flag = SRC_AND_LOC;	/* print location and source line */
+		  break;
+		case PRINT_SRC_ONLY:
+		  source_flag = SRC_LINE;
+		  break;
+		case PRINT_NOTHING:
+		  source_flag = SRC_LINE;	/* something bogus */
+		  do_frame_printing = 0;
+		  break;
+		default:
+		  internal_error (__FILE__, __LINE__, _("Unknown value."));
+		}
+/* TENSILICA_LOCAL */
 	    }
+/* END TENSILICA_LOCAL */
+
 
 	  if (ui_out_is_mi_like_p (uiout))
 	    ui_out_field_int (uiout, "thread-id",
diff -urN gdb-6.8-orig/gdb/remote.c gdb-6.8-new/gdb/remote.c
--- gdb-6.8-orig/gdb/remote.c	2009-03-19 17:29:48.000000000 -0700
+++ gdb-6.8-new/gdb/remote.c	2009-03-20 11:41:52.000000000 -0700
@@ -473,6 +473,15 @@
 /* This is non-zero if target stopped for a watchpoint.  */
 static int remote_stopped_by_watchpoint_p;
 
+/* TENSILICA_LOCAL */
+/* This is non-zero if target stopped for a program breakpoint. */
+static int remote_stopped_by_program_breakpoint_p = 0;
+
+/* This is set to a architecture-specific representation of the kind of
+   program breakpoint if remote_stopped_by_program_breakpoint_p != 0. */
+static int remote_program_breakpoint_kind = 0;
+/* END TENSILICA_LOCAL */
+
 static struct target_ops remote_ops;
 
 static struct target_ops extended_remote_ops;
@@ -3399,6 +3408,9 @@
 	(*deprecated_target_wait_loop_hook) ();
 
       remote_stopped_by_watchpoint_p = 0;
+/* TENSILICA_LOCAL */
+      remote_stopped_by_program_breakpoint_p = 0;
+/* END TENSILICA_LOCAL */
 
       switch (buf[0])
 	{
@@ -3474,6 +3486,19 @@
 			solibs_changed = 1;
 			p = p_temp;
 		      }
+/* TENSILICA_LOCAL */
+                    else if ((strncmp (p, "pbreak", p1 - p) == 0))
+		      {
+			/* Protocol extension for program break 'TAApbreak:kind'
+			   where kind is an arch-specific integer encoding info 
+			   about the break instruction (eg. size, type, args). */
+			ULONGEST kind;
+
+			remote_stopped_by_program_breakpoint_p = 1;
+			p = unpack_varlen_hex (++p1, &kind);
+			remote_program_breakpoint_kind = (int)kind;
+		      }
+/* END TENSILICA_LOCAL */
 		    else
  		      {
  			/* Silently skip unknown optional info.  */
@@ -3592,6 +3617,11 @@
 
   remote_stopped_by_watchpoint_p = 0;
 
+/* TENSILICA_LOCAL */
+  remote_stopped_by_program_breakpoint_p = 0;
+  sigint_interrupt_flag = 0;
+/* END TENSILICA_LOCAL PR */
+
   while (1)
     {
       char *buf, *p;
@@ -3702,6 +3732,18 @@
 			solibs_changed = 1;
 			p = p_temp;
 		      }
+/* TENSILICA_LOCAL */
+		    else if ((strncmp (p, "pbreak", p1 - p) == 0))
+		      {
+			/* Protocol extension for program break 'TAApbreak:kind'
+			   where kind is an arch-specific integer. */
+			ULONGEST kind;
+
+			remote_stopped_by_program_breakpoint_p = 1;
+			p = unpack_varlen_hex (++p1, &kind);
+			remote_program_breakpoint_kind = (int)kind;
+		      }
+/* END TENSILICA_LOCAL */
 		    else
  		      {
  			/* Silently skip unknown optional info.  */
@@ -5720,6 +5762,19 @@
   return rc;
 }
 
+/* TENSILICA_LOCAL */
+/* Return non-zero if target was stopped by a program breakpoint.
+   If so and kind != NULL, set *kind to an architecture-specific value. */
+
+static int
+remote_stopped_by_program_breakpoint (int *kind)
+{
+  if (remote_stopped_by_program_breakpoint_p && kind != NULL)
+    *kind = remote_program_breakpoint_kind;
+  return remote_stopped_by_program_breakpoint_p;
+}
+/* END TENSILICA_LOCAL */
+
 
 static int
 remote_insert_hw_breakpoint (struct bp_target_info *bp_tgt)
@@ -7229,6 +7284,10 @@
   remote_ops.to_remove_breakpoint = remote_remove_breakpoint;
   remote_ops.to_stopped_by_watchpoint = remote_stopped_by_watchpoint;
   remote_ops.to_stopped_data_address = remote_stopped_data_address;
+/* TENSILICA_LOCAL */
+  remote_ops.to_stopped_by_program_breakpoint =
+    				remote_stopped_by_program_breakpoint;
+/* END TENSILICA_LOCAL */
   remote_ops.to_can_use_hw_breakpoint = remote_check_watch_resources;
   remote_ops.to_insert_hw_breakpoint = remote_insert_hw_breakpoint;
   remote_ops.to_remove_hw_breakpoint = remote_remove_hw_breakpoint;
@@ -7365,6 +7424,10 @@
   remote_async_ops.to_remove_watchpoint = remote_remove_watchpoint;
   remote_async_ops.to_stopped_by_watchpoint = remote_stopped_by_watchpoint;
   remote_async_ops.to_stopped_data_address = remote_stopped_data_address;
+/* TENSILICA_LOCAL */
+  remote_async_ops.to_stopped_by_program_breakpoint =
+    				remote_stopped_by_program_breakpoint;
+/* END TENSILICA_LOCAL */
   remote_async_ops.to_terminal_inferior = remote_async_terminal_inferior;
   remote_async_ops.to_terminal_ours = remote_async_terminal_ours;
   remote_async_ops.to_kill = remote_async_kill;
diff -urN gdb-6.8-orig/gdb/target.c gdb-6.8-new/gdb/target.c
--- gdb-6.8-orig/gdb/target.c	2009-03-19 17:29:49.000000000 -0700
+++ gdb-6.8-new/gdb/target.c	2009-03-20 11:44:22.000000000 -0700
@@ -410,6 +410,9 @@
       INHERIT (to_insert_watchpoint, t);
       INHERIT (to_remove_watchpoint, t);
       INHERIT (to_stopped_data_address, t);
+/* TENSILICA_LOCAL */
+      INHERIT (to_stopped_by_program_breakpoint, t);
+/* END TENSILICA_LOCAL */
       INHERIT (to_stopped_by_watchpoint, t);
       INHERIT (to_have_steppable_watchpoint, t);
       INHERIT (to_have_continuable_watchpoint, t);
@@ -539,6 +542,11 @@
   de_fault (to_stopped_data_address,
 	    (int (*) (struct target_ops *, CORE_ADDR *))
 	    return_zero);
+/* TENSILICA_LOCAL  -- Program breakpoint.  */
+  de_fault (to_stopped_by_program_breakpoint,
+            (int (*) (int*))
+            return_zero);
+/* END TENSILICA_LOCAL */
   de_fault (to_region_ok_for_hw_watchpoint,
 	    default_region_ok_for_hw_watchpoint);
   de_fault (to_terminal_init,
@@ -2400,6 +2408,22 @@
   return retval;
 }
 
+/* TENSILICA_LOCAL */
+static int
+debug_to_stopped_by_program_breakpoint (int *kind)
+{
+  int retval;
+
+  retval = debug_target.to_stopped_by_program_breakpoint (kind);
+
+  fprintf_unfiltered (gdb_stdlog,
+		      "target_stopped_by_program_breakpoint (%ld) = %ld\n",
+		      (unsigned long) *kind,
+		      (unsigned long) retval);
+  return retval;
+}
+/* END TENSILICA_LOCAL */
+
 static void
 debug_to_terminal_init (void)
 {
@@ -2703,6 +2727,9 @@
   current_target.to_remove_watchpoint = debug_to_remove_watchpoint;
   current_target.to_stopped_by_watchpoint = debug_to_stopped_by_watchpoint;
   current_target.to_stopped_data_address = debug_to_stopped_data_address;
+/* TENSILICA_LOCAL */
+  current_target.to_stopped_by_program_breakpoint = debug_to_stopped_by_program_breakpoint;
+/* END TENSILICA_LOCAL */
   current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint;
   current_target.to_terminal_init = debug_to_terminal_init;
   current_target.to_terminal_inferior = debug_to_terminal_inferior;
diff -urN gdb-6.8-orig/gdb/target.h gdb-6.8-new/gdb/target.h
--- gdb-6.8-orig/gdb/target.h	2009-03-19 17:29:49.000000000 -0700
+++ gdb-6.8-new/gdb/target.h	2009-03-20 11:45:20.000000000 -0700
@@ -369,6 +369,9 @@
     int to_have_continuable_watchpoint;
     int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
     int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int);
+/* TENSILICA_LOCAL -- Program breakpoints */
+    int (*to_stopped_by_program_breakpoint) (int *);
+/* END TENSILICA_LOCAL */
     void (*to_terminal_init) (void);
     void (*to_terminal_inferior) (void);
     void (*to_terminal_ours_for_output) (void);
@@ -1107,6 +1110,18 @@
 
 extern const struct target_desc *target_read_description (struct target_ops *);
 
+/* TENSILICA_LOCAL */
+/* Returns non-zero if we were stopped by a program breakpoint (break instr
+   embedded in the target code, not planted by GDB). If k != 0, sets *k to
+   an arch-specific integer indicating the kind of break instruction.  */
+
+#ifndef STOPPED_BY_PROGRAM_BREAKPOINT
+#define STOPPED_BY_PROGRAM_BREAKPOINT(k) \
+  (*current_target.to_stopped_by_program_breakpoint) (k)
+#endif
+/* END TENSILICA_LOCAL */
+
+
 /* Command logging facility.  */
 
 #define target_log_command(p)						\
diff -urN gdb-6.8-orig/gdb/xtensa-tdep.c gdb-6.8-new/gdb/xtensa-tdep.c
--- gdb-6.8-orig/gdb/xtensa-tdep.c	2009-03-19 17:29:49.000000000 -0700
+++ gdb-6.8-new/gdb/xtensa-tdep.c	2009-03-20 11:52:50.000000000 -0700
@@ -2454,6 +2454,34 @@
   return body_pc != 0 ? body_pc : start_pc;
 }
 
+/* TENSILICA_LOCAL */
+static void
+xtensa_skip_program_breakpoint (int kind)
+{
+  DEBUGTRACE ("xtensa_skip_program_breakpoint (pc = 0x%08x, kind = 0x%03x)\n",
+              (unsigned)read_pc (), kind);
+
+  write_pc (read_pc () + 2 + ((kind >> 8) & 1));
+  return;
+}
+
+/* Provide a string description of a program breakpoint of a given kind.  */
+
+static const char *
+xtensa_describe_program_breakpoint (int kind)
+{
+/* Keep an eye: potential conflict with concurrent instances of this arch.  */
+  static char descr[16];
+  int len;
+
+  len = sprintf (descr, "break%s %u,%u",
+		 (kind & 0xf00) ? "" : ".n",
+		 (kind & 0x0f0) >> 4, 
+		 (kind & 0x00f));
+  gdb_assert (len < 16);
+  return descr;
+}
+
 /* Verify the current configuration.  */
 static void
 xtensa_verify_config (struct gdbarch *gdbarch)
@@ -2632,6 +2660,14 @@
   /* Advance PC across any prologue instructions to reach "real" code.  */
   set_gdbarch_skip_prologue (gdbarch, xtensa_skip_prologue);
 
+/* TENSILICA_LOCAL */
+    /* Handle program breakpoints not planted by GDB.
+     It's Xtensa-specific, although we want to submit it.  */
+  set_gdbarch_skip_program_breakpoint (gdbarch, xtensa_skip_program_breakpoint);
+  set_gdbarch_describe_program_breakpoint (gdbarch,
+					   xtensa_describe_program_breakpoint);
+/* END TENSILICA_LOCAL */
+
   /* Stack grows downward.  */
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
 

Re: RFC: Program Breakpoints (was: [RFC] stepping over permanent breakpoint)

[Daniel Jacobowitz]

On Mon, Mar 23, 2009 at 09:50:30AM -0700, Ross Morley wrote:
> It was interesting to see the matter of skipping permanent breakpoints
> come up on this list. This is very similar to an issue we dealt with
> a couple of years ago with Xtensa and remote targets. Our solution
> has been stable for at least two years, but has not yet been prepared
> for submission because I've had higher priorities (and at that time we
> were way out of sync with mainline GDB). The issue is actually more
> complicated than the forgoing discussion suggests.
>
> I'd like to discuss it here and present our solution. I'll attach a
> patch for our current solution relative to GDB 6.8, but I do propose
> to revise it to improve some areas based on our experience (in the
> next few weeks) - I'll discuss that toward the end of this post.

I read through this; overall, it looks sane.  On some targets
implementing this would require the remote stub to read from pc
anyway; that's faster than GDB doing it, but not necessarily much
faster.  But on some other targets the stub has to do this anyway,
or can pipeline it with other necessary operations, so it's not a big
loss.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: Program Breakpoints

[Ross Morley]

Daniel Jacobowitz wrote:

>On Mon, Mar 23, 2009 at 09:50:30AM -0700, Ross Morley wrote:
>  
>
>>It was interesting to see the matter of skipping permanent breakpoints
>>come up on this list. This is very similar to an issue we dealt with
>>a couple of years ago with Xtensa and remote targets. Our solution
>>has been stable for at least two years, but has not yet been prepared
>>for submission because I've had higher priorities (and at that time we
>>were way out of sync with mainline GDB). The issue is actually more
>>complicated than the forgoing discussion suggests.
>>
>>I'd like to discuss it here and present our solution. I'll attach a
>>patch for our current solution relative to GDB 6.8, but I do propose
>>to revise it to improve some areas based on our experience (in the
>>next few weeks) - I'll discuss that toward the end of this post.
>>    
>>
>
>I read through this; overall, it looks sane.  On some targets
>implementing this would require the remote stub to read from pc
>anyway; that's faster than GDB doing it, but not necessarily much
>faster.  But on some other targets the stub has to do this anyway,
>or can pipeline it with other necessary operations, so it's not a big
>loss.
>  
>

I think you're saying it's not a big deal performance-wise to do this
without a remote protocol extension. Is that correct?

I think it's quite possible to implement the target-specific macro
STOPPED_BY_PROGRAM_BREAKPOINT(size) without any help from the remote
protocol. That is certainly an option, and might be the method of
choice for non-remote targets and some remote targets.

Having the remote stub do the break detection has other benefits.
It removes the burden of parsing the set of possible instructions
that *might* cause a stop, from GDB, and lets the stub (which has
already decided to signal a stop) simply tell GDB why it stopped.
It also handles cases where the stub decides whether to stop or not
based on factors beyond the instruction itself. Seems cleaner and
more robust. In general I think SIGTRAP is too overloaded in GDB
which has to jump through hoops to figure out why it stopped.

What I'm hoping to get from this RFC is whether the community would
accept this method, or whether it would rather take another approach
(before I put in more effort to simplify our solution and submit it).
We'd like to have a solution that we don't have to keep merging. :-)
I'll wait a few more days in case others have something to say.

Thanks,
Ross

Re: RFC: Program Breakpoints

[Daniel Jacobowitz]

On Tue, Mar 24, 2009 at 01:32:57PM -0700, Ross Morley wrote:
>> I read through this; overall, it looks sane.  On some targets
>> implementing this would require the remote stub to read from pc
>> anyway; that's faster than GDB doing it, but not necessarily much
>> faster.  But on some other targets the stub has to do this anyway,
>> or can pipeline it with other necessary operations, so it's not a big
>> loss.
>>  
>>
>
> I think you're saying it's not a big deal performance-wise to do this
> without a remote protocol extension. Is that correct?

No, I was saying the opposite.  Sometimes it will still be expensive
to implement the protocol extension.  I'm interested in whether anyone
sees an approach that does not require instruction scanning.

GDB has the option to cheat - it can consult the program (ELF file),
separately from the target's view of memory.  This would not work for
stray breakpoints inserted in the program at runtime, though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: Program Breakpoints

[Pedro Alves]

On Tuesday 24 March 2009 20:39:53, Daniel Jacobowitz wrote:
> No, I was saying the opposite.  Sometimes it will still be expensive
> to implement the protocol extension.  I'm interested in whether anyone
> sees an approach that does not require instruction scanning.

[ For the record, since I was curious about the  win32 bits below ]

Several OSs already export that info on their debug APIs, but we
just discard it.

Some linux archs expose it in the SIGTRAP siginfo, in
the si_code field, in the form of TRAP_BRKPT, TRAP_TRACE.  E.g., I think
ppc does expose TRAP_BRKPT, but x86/x86_64 doesn't, at least not yet.

I believe mac/darwin also distinguishes breakpoint traps from
single-stepping traps at the debug api level.  At least include/gdb/signals.h
mentions TARGET_EXC_BREAKPOINT as being a Mach exception.  This could
mean that GNU/Hurd also distinguishes them.

Windows distinguishes breakpoints from singlesteps at the debug API level
too.  We have EXCEPTION_SINGLE_STEP and EXCEPTION_BREAKPOINT.  You'll
see that windows-nat.c converts both to SIGTRAP.  I've just confirmed this,
by enabling "set debugexceptions on" on a Cygwin GDB.

Probably other os/archs/targets have similar means to distinguish a
breakpoint trap from a singlestep.  Either through a different trap
vector for each case, or looking at the trace flag and at the intruction
stream themselves, etc.

-- 
Pedro Alves

Re: RFC: Program Breakpoints

[Mark Kettenis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 24 Mar 2009 23:48:21 +0000
> 
> On Tuesday 24 March 2009 20:39:53, Daniel Jacobowitz wrote:
> > No, I was saying the opposite.  Sometimes it will still be expensive
> > to implement the protocol extension.  I'm interested in whether anyone
> > sees an approach that does not require instruction scanning.
> 
> [ For the record, since I was curious about the  win32 bits below ]
> 
> Several OSs already export that info on their debug APIs, but we
> just discard it.
> 
> Some linux archs expose it in the SIGTRAP siginfo, in
> the si_code field, in the form of TRAP_BRKPT, TRAP_TRACE.  E.g., I think
> ppc does expose TRAP_BRKPT, but x86/x86_64 doesn't, at least not yet.
> 
> I believe mac/darwin also distinguishes breakpoint traps from
> single-stepping traps at the debug api level.  At least include/gdb/signals.h
> mentions TARGET_EXC_BREAKPOINT as being a Mach exception.  This could
> mean that GNU/Hurd also distinguishes them.
> 
> Windows distinguishes breakpoints from singlesteps at the debug API level
> too.  We have EXCEPTION_SINGLE_STEP and EXCEPTION_BREAKPOINT.  You'll
> see that windows-nat.c converts both to SIGTRAP.  I've just confirmed this,
> by enabling "set debugexceptions on" on a Cygwin GDB.
> 
> Probably other os/archs/targets have similar means to distinguish a
> breakpoint trap from a singlestep.  Either through a different trap
> vector for each case, or looking at the trace flag and at the intruction
> stream themselves, etc.

These statements are all rather i386-centric.  Especially on targets
that emulate single-stepping it may be impossible to distinguish.  GDB
should not depend on the ability to do so.

Re: RFC: Program Breakpoints

[Pedro Alves]

On Wednesday 25 March 2009 07:57:26, Mark Kettenis wrote:
> These statements are all rather i386-centric.  

These were examples, not statements of all-truth.  I mentioned
ppc-linux, and darwin; you meant hardware-single-stepping-centric.

> Especially on targets 
> that emulate single-stepping it may be impossible to distinguish.  GDB
> should not depend on the ability to do so.

If it's impossible to distinguish for some reason, then the target would
still report SIGTRAP-means-all, like today.  When the distinction on the
debug api side is buggy and inconsistent, then GDB is better off
without it.

-- 
Pedro Alves

Re: RFC: Program Breakpoints

[Ross Morley]

Daniel Jacobowitz wrote:

>On Tue, Mar 24, 2009 at 01:32:57PM -0700, Ross Morley wrote:
>  
>
>>>I read through this; overall, it looks sane.  On some targets
>>>implementing this would require the remote stub to read from pc
>>>anyway; that's faster than GDB doing it, but not necessarily much
>>>faster.  But on some other targets the stub has to do this anyway,
>>>or can pipeline it with other necessary operations, so it's not a big
>>>loss.
>>> 
>>>
>>>      
>>>
>>I think you're saying it's not a big deal performance-wise to do this
>>without a remote protocol extension. Is that correct?
>>    
>>
>
>No, I was saying the opposite.  
>
Sorry for the misunderstanding.

>Sometimes it will still be expensive
>to implement the protocol extension.  
>
I would think that in most cases the stub already knows something
about what's going on, be it a single-step trap, a data watchpoint
match, or a breakpoint. It already has to distinguish some of those
cases. In many if not most cases the stub gets an exception directed
to a certain vector, an indication in a status register, or some other
clue as to the nature of the stop. It doesn't need to distinguish
types of break instruction except for size (if there's more than one).
It also needn't distinguish GDB planted breaks from program breaks.

If a target finds it's too expensive to implement the remote protocol
extension, then that target can have GDB probe the instruction.

>I'm interested in whether anyone
>sees an approach that does not require instruction scanning.
>  
>
I suspect it's going to be very target and arch specific.

>GDB has the option to cheat - it can consult the program (ELF file),
>separately from the target's view of memory.  This would not work for
>stray breakpoints inserted in the program at runtime, though.
>  
>
Not always. Program breaks are often very useful in situations where
you have code embedded in a ROM and no ELF file available. We put them
in exception handlers to allow a debugger to get control when certain
exceptions happen.

I do agree it would be good to hear if anyone has any other ideas.

Thanks,
Ross

Re: RFC: Program Breakpoints

[Ross Morley]

I'm preparing the simplified version I described earlier for submission.

In stepping over a program break I increment the PC without running the
inferior (in step_1 of infcmd.c - see trimmed post below for context).
Then I update the frame cache and report the stop reason, new PC, etc.
My old pre-6.x implementation used a function that was deprecated in
6.x and recently removed.

What is the proper way to update the frame without the call to
   deprecated_update_frame_pc_hack (get_current_frame (), read_pc ());

I tried calling get_frame_pc (get_current_frame ()) and ignoring the result.
That should call frame_pc_unwind (). But the subsequent
    print_stack_frame (get_selected_frame (NULL), -1, LOC_AND_ADDRESS);
reports the address of the break to MI, not the incremented PC.

I also tried reinit_frame_cache () and it works OK. Seems drastic though.

For convenience, here's the code snippet from step_1 of infcmd.c:

  else if (STOPPED_BY_PROGRAM_BREAKPOINT (&program_break_size) &&
           program_break_size != 0 && read_pc() == stop_pc && count > 0)
    {
      /* "stepi" off program breakpoint: the first step is to just increment
         the PC past the break, then there are count-1 steps to go.
         Note proceed() won't be called the first time, and on subsequent
         steps the PC will already be off the break, so the entire handling
         of "stepi" off a program breakpoint is done here. If stopping 
after
         the break, display location information as for normal_stop.  */
      count--;
      write_pc (read_pc () + program_break_size);
      if (count == 0)
        {
          deprecated_update_frame_pc_hack (get_current_frame (), read_pc 
());
          if (ui_out_is_mi_like_p (uiout))
            {
              ui_out_field_string
                (uiout, "reason",
                 async_reason_lookup (EXEC_ASYNC_END_STEPPING_RANGE));
              ui_out_field_int (uiout, "thread-id",
                                pid_to_thread_id (inferior_ptid));
              print_stack_frame (get_selected_frame (NULL), -1, 
LOC_AND_ADDRESS);
            }
          else
            print_stack_frame (get_selected_frame (NULL), -1, SRC_LINE);
        }
    }


Thanks,
Ross


Ross Morley wrote:


>
> The Problem
> -----------
>
> When the target program hits a breakpoint of any kind, GDB receives
> a SIGTRAP event. As Aleksander pointed out, if the PC is not in the
> breakpoint table GDB stops because it doesn't know what to do with
> this event. In fact it doesn't always stop. When GDB is stepping
> (for example when a software watchpoint is set) it sees the SIGTRAP
> as a normal step event, so it continues stepping. If the break
> instruction didn't increment the PC, it keeps executing the same
> break instruction forever. This is very frustrating for users (who
> reported this as a bug) because when you have a s/w watchpoint and
> are stepping, execution is expected to be slow, so you wait a long
> time before you realize it's hung.
>
<stuff deleted>

> The Solution
> ------------
>
> The first part is to recognize when you hit a program break.
> The second is to step over it when you want to resume.
>
<stuff deleted>

> Having discovered that it stopped for a program breakpoint, GDB
> is able to report the fact to both the CLI and MI. I also defined
> a gdbarch function to report the meaning of "kind" in an arch-
> specific way. This improves the debugging experience when the
> target uses different kinds of breakpoints to denote different
> reasons for stopping. (However, in the interest of simplicity,
> my revised proposal won't report all that - it is anyway obvious
> from a disassembly if not a symbol associated with the PC; GDB
> itself only needs to know the size to skip the breakpoint).
>
> When the user elects to continue after a program breakpoint,
> GDB increments the PC by the size of the break that was reported
> in the "kind" argument. A gdbarch function handles this. Of
> course it might do nothing if the PC was already advanced.
>
> I haven't discussed all the implementation details here.
> Please see the attached 6.8 based patch for the actual code.
> Note this patch does not pretend to be ready for submission!
>
<stuff deleted>

> Proposed Improved, Simplified Solution
> --------------------------------------
>
> The remote protocol extension would be:
>    TAAtrap[:size]
> where ":size" is optional and may only be provided to GDB by the
> target agent if the PC is in fact pointing to the instruction
> that caused the break, and if omitted is taken to be 0.
> GDB will skip the break by PC += size (no effect if size is 0).
> Note it is not necessary to call gdbarch_breakpoint_from_pc().
>
> The gdbarch functions to extract the size and decipher "kind"
> are not needed. The target interface function
>    STOPPED_BY_PROGRAM_BREAKPOINT(k)
> becomes
>    STOPPED_BY_PROGRAM_BREAKPOINT(size)
> where 'size' is an address in which the size is returned.
>
> We might also want to consider calling it "program trap" and
> keep the term "breakpoint" for things that GDB knows about.
>
>
> Comments are welcome. We at Tensilica would like to see this
> refined and incorporated into mainline GDB.
>
> This can certainly coexist with permanent breakpoints,
> however it is (I think) a bit more general. If the people
> who use permanent breakpoints would care to comment,
> perhaps we can somehow reconcile these into one feature.
>
> Thanks,
> Ross Morley
> Tensilca, Inc.
> ross@tensilica.com
>
>
>diff -urN gdb-6.8-orig/gdb/infcmd.c gdb-6.8-new/gdb/infcmd.c
>--- gdb-6.8-orig/gdb/infcmd.c	2009-03-19 17:29:48.000000000 -0700
>+++ gdb-6.8-new/gdb/infcmd.c	2009-03-20 11:33:21.000000000 -0700
>@@ -696,6 +696,9 @@
>   struct frame_info *frame;
>   struct cleanup *cleanups = 0;
>   int async_exec = 0;
>+/* TENSILICA_LOCAL */
>+  int program_break_kind;
>+/* END TENSILICA_LOCAL */
> 
>   ERROR_NO_INFERIOR;
> 
>@@ -725,6 +728,33 @@
>       else
>         make_exec_cleanup (disable_longjmp_breakpoint_cleanup, 0 /*ignore*/);
>     }
>+/* TENSILICA_LOCAL */
>+  else if (STOPPED_BY_PROGRAM_BREAKPOINT (&program_break_kind) && 
>+	   read_pc() == stop_pc && count > 0)
>+    {
>+      /* "stepi" off program breakpoint: the first step is to just increment
>+	 the PC past the break, then there are count-1 steps to go.
>+	 Note proceed() won't be called the first time, and on subsequent
>+	 steps the PC will already be off the break, so the entire handling
>+	 of "stepi" off a program breakpoint is done here. If stopping after 
>+	 the break, display location information as for normal_stop.  */
>+      count--;
>+      gdbarch_skip_program_breakpoint (current_gdbarch, program_break_kind);
>+      if (count == 0)
>+	{
>+	  deprecated_update_frame_pc_hack (get_current_frame (), read_pc ());
>+	  if (ui_out_is_mi_like_p (uiout))
>+	    {
>+	      ui_out_field_string (uiout, "reason", "end-stepping-range");
>+	      ui_out_field_int (uiout, "thread-id",
>+				pid_to_thread_id (inferior_ptid));
>+	      print_stack_frame (get_selected_frame (NULL), -1, LOC_AND_ADDRESS);
>+	    }
>+	  else 
>+	    print_stack_frame (get_selected_frame (NULL), -1, SRC_LINE);
>+	}
>+    }
>+/* END TENSILICA_LOCAL */
> 
>
>  
>

Re: RFC: Program Breakpoints

[Daniel Jacobowitz]

On Mon, Mar 30, 2009 at 05:44:20PM -0700, Ross Morley wrote:
> What is the proper way to update the frame without the call to
>   deprecated_update_frame_pc_hack (get_current_frame (), read_pc ());

If write_pc doesn't automatically, then flush the frame chain
yourself after write_pc.

-- 
Daniel Jacobowitz
CodeSourcery