From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Duffek To: fche@redhat.com, cagney@redhat.com, jtc@redback.com Cc: sid@sources.redhat.com, gdb-patches@cygnus.com Subject: Possible remote.c patch for Z-packet breakpoints + Harvard + SID Date: Mon, 16 Jul 2001 14:35:00 -0000 Message-id: <200107161931.f6GJVV719853@rtl.cygnus.com> References: X-SW-Source: 2001-q3/msg00014.html 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);