* Possible remote.c patch for Z-packet breakpoints + Harvard + SID
[not found] <o5pub0sqqy.fsf@toenail.toronto.redhat.com>
@ 2001-07-16 14:35 ` Nick Duffek
2001-07-16 16:42 ` Mark Salter
[not found] ` <200107161931.f6GJVV719853.cygnus.project.sid@rtl.cygnus.com>
1 sibling, 1 reply; 3+ messages in thread
From: Nick Duffek @ 2001-07-16 14:35 UTC (permalink / raw)
To: fche, cagney, jtc; +Cc: sid, gdb-patches
Executive summary for the lists I've added to this thread: Z-packet
breakpoints don't work with SID on Harvard architectures.
On 16-Jul-2001, I wrote:
>When a Harvard-architecture GDB sends an address to SID, the address has a
>high bit set indicating whether the address space, e.g. instruction
>vs. data.
>
>However, gdb::add_hw_breakpoint() in sid/component/gdb/gdb.cxx doesn't
>trim the high bit before setting a watchpoint at that address.
>Consequently, hardware breakpoints don't work with Harvard architectures.
>
>It'd be possible to trim the bit in GDB, but I think it's better to fix
>the problem in SID and avoid special-casing hardware breakpoints in the
>GDB <-> SID protocol.
>
>Perhaps a new sid::bus method could be introduced to translate addresses
>without writing or reading memory, and {add,remove}_hw_breakpoint() could
>call that.
On 16-Jul-2001, Frank Ch . Eigler replied:
>: When a Harvard-architecture GDB sends an address to SID, the address has a
>: high bit set indicating whether the address space, e.g. instruction
>: vs. data.
>
>Yes, it should do so, when dealing with memory-related addresses.
>
>: However, gdb::add_hw_breakpoint() in sid/component/gdb/gdb.cxx doesn't
>: trim the high bit before setting a watchpoint at that address. [...]
>
>That's a separate matter - a hardware breakpoint relates to the PC
>register value, which does not contain the address-space tag byte.
>
>: It'd be possible to trim the bit in GDB [...]
>
>Thanks for the other ideas, but methinks GDB is the right place to do
>this. GDB already knows how to map PC register values to/from memory
>addresses; it needs to do the opposite when requesting hardware
>breakpoints.
I see your point, though I think it's debatable. For one thing, Z-packet
breakpoints are tagged as "Z_PACKET_SOFTWARE_BP" in gdb/remote.c, so it
looks like they're intended to be a generic protocol speedup rather than a
hardware breakpoint mechanism.
As far as I know, gdb/remote.c has never translated Z-packet breakpoints
as described above, so I doubt we can do ADDRESS_TO_POINTER()
unconditionally on those breakpoints without breaking other targets.
I proposed a patch in January to call BREAKPOINT_FROM_PC() before rather
than after remote.c stores hardware breakpoint addresses in outgoing
packets, but it was decided that such a change should be at a higher
level, e.g. in target_insert_breakpoint().
This problem can't be fixed in target_insert_breakpoint(), because the
distinction between "hardware" (Z-packet) and software breakpoints is made
in remote.c.
So, here's a hook to add to gdb/remote.c. Comments?
Nick
ChangeLog:
* gdbarch.sh (ADJUST_REMOTE_Z_BREAKPOINT): New function.
* remote.c (remote_insert_breakpoint, remote_remove_breakpoint):
Call ADJUST_REMOTE_Z_BREAKPOINT().
Index: gdb/gdbarch.sh
===================================================================
diff -up gdb/gdbarch.sh gdb/gdbarch.sh
--- gdb/gdbarch.sh Mon Jul 16 15:25:50 2001
+++ gdb/gdbarch.sh Mon Jul 16 15:25:42 2001
@@ -488,6 +488,8 @@ f:2:SKIP_PROLOGUE:CORE_ADDR:skip_prologu
f:2:PROLOGUE_FRAMELESS_P:int:prologue_frameless_p:CORE_ADDR ip:ip::0:generic_prologue_frameless_p::0
f:2:INNER_THAN:int:inner_than:CORE_ADDR lhs, CORE_ADDR rhs:lhs, rhs::0:0
f:2:BREAKPOINT_FROM_PC:unsigned char *:breakpoint_from_pc:CORE_ADDR *pcptr, int *lenptr:pcptr, lenptr:::legacy_breakpoint_from_pc::0
+# Update remote Z-packet breakpoint ADDR before sending it to the target.
+F:2:ADJUST_REMOTE_Z_BREAKPOINT:void:adjust_remote_z_breakpoint:CORE_ADDR addr:addr::0
f:2:MEMORY_INSERT_BREAKPOINT:int:memory_insert_breakpoint:CORE_ADDR addr, char *contents_cache:addr, contents_cache::0:default_memory_insert_breakpoint::0
f:2:MEMORY_REMOVE_BREAKPOINT:int:memory_remove_breakpoint:CORE_ADDR addr, char *contents_cache:addr, contents_cache::0:default_memory_remove_breakpoint::0
v:2:DECR_PC_AFTER_BREAK:CORE_ADDR:decr_pc_after_break::::0:-1
Index: gdb/remote.c
===================================================================
diff -up gdb/remote.c gdb/remote.c
--- gdb/remote.c Mon Jul 16 15:25:59 2001
+++ gdb/remote.c Mon Jul 16 15:25:42 2001
@@ -4513,13 +4513,17 @@ remote_insert_breakpoint (CORE_ADDR addr
{
char *buf = alloca (PBUFSIZ);
char *p = buf;
+ CORE_ADDR addr2;
- addr = remote_address_masked (addr);
+ addr2 = remote_address_masked (addr);
+ if (ADJUST_REMOTE_Z_BREAKPOINT_P ())
+ ADJUST_REMOTE_Z_BREAKPOINT (&addr2);
+
*(p++) = 'Z';
*(p++) = '0';
*(p++) = ',';
- p += hexnumstr (p, (ULONGEST) addr);
- BREAKPOINT_FROM_PC (&addr, &bp_size);
+ p += hexnumstr (p, (ULONGEST) addr2);
+ BREAKPOINT_FROM_PC (&addr2, &bp_size);
sprintf (p, ",%d", bp_size);
putpkt (buf);
@@ -4564,14 +4568,18 @@ remote_remove_breakpoint (CORE_ADDR addr
{
char *buf = alloca (PBUFSIZ);
char *p = buf;
+ CORE_ADDR addr2;
*(p++) = 'z';
*(p++) = '0';
*(p++) = ',';
- addr = remote_address_masked (addr);
- p += hexnumstr (p, (ULONGEST) addr);
- BREAKPOINT_FROM_PC (&addr, &bp_size);
+ addr2 = remote_address_masked (addr);
+ if (ADJUST_REMOTE_Z_BREAKPOINT_P ())
+ ADJUST_REMOTE_Z_BREAKPOINT (&addr2);
+
+ p += hexnumstr (p, (ULONGEST) addr2);
+ BREAKPOINT_FROM_PC (&addr2, &bp_size);
sprintf (p, ",%d", bp_size);
putpkt (buf);
^ permalink raw reply [flat|nested] 3+ messages in thread