From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22185 invoked by alias); 28 Jan 2010 22:29:54 -0000 Received: (qmail 21930 invoked by uid 22791); 28 Jan 2010 22:29:52 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 28 Jan 2010 22:29:46 +0000 Received: (qmail 10592 invoked from network); 28 Jan 2010 22:29:43 -0000 Received: from unknown (HELO caradoc.them.org) (dan@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Jan 2010 22:29:43 -0000 Date: Thu, 28 Jan 2010 22:29:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Cc: Eli Zaretskii Subject: [RFA/doc] Redefine the length argument in remote Z packets Message-ID: <20100128222940.GE2813@caradoc.them.org> Mail-Followup-To: gdb-patches@sourceware.org, Eli Zaretskii MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-01/txt/msg00623.txt.bz2 We currently have five kinds of "Z" packets, for software/hardware breakpoints and write/read/access watchpoints. They all take an address and a length. For watchpoints, the length is obvious: how large an area to watch. For breakpoints, it's a convenient shortcut that I'm running into trouble with. The reason for the length argument on breakpoints, particularly software breakpoints, is that some platforms have more than one breakpoint instruction. For example, on ARM targets, you need to use an ARM mode breakpoint if you're replacing an ARM instruction and a Thumb mode breakpoint if you're replacing a Thumb instruction. Conveniently, on both ARM and MIPS the compact ISA has smaller breakpoints than the normal ISA. So we use Z0,ADDR,4 for one and Z0,ADDR,2 for the other. On recent ARM processors there's an extended version of Thumb mode, called Thumb-2. This has both 16-bit and 32-bit instructions. In most situations we can replace the first half of a 32-bit instruction with the 16-bit breakpoint, and everything will work out fine - that is what we also do on i386. But there's one corner case, which I'll go into more in the next patch, where we need to replace 32-bit Thumb instructions with a 32-bit Thumb breakpoint. Unfortunately, it doesn't *quite* line up with the ARM mode breakpoints; just a few bits off. So we need a third kind. This new breakpoint has an actual length of four bytes. So gdbarch_breakpoint_from_pc correctly reports that it is a four byte breakpoint. For native debugging, this is as far as we have to go; everything will work. But for remote targets this will cause us to insert the wrong breakpoint instruction. We can't use Z0,ADDR,4 for this breakpoint. I had two options for the new breakpoint. We could either send Z0,ADDR,2 and have the target figure out whether to use the 16-bit or 32-bit breakpoint, or we could send Z0,ADDR,3 [number chosen arbitrarily] and have the target decide based on that to use a 32-bit breakpoint. Both are feasible; it's easy to tell if a Thumb instruction is the first half of a 32-bit sequence. I opted for the new number as a safeguard. The failure if a 16-bit breakpoint is used will be quite hard to identify, and could silently corrupt program execution in some cases. So let's make sure stubs that don't implement the right breakpoint fail. This is mostly academic. It only applies to Linux, and similar OS environments; bare metal can use 16-bit breakpoints all the time. And gdbserver doesn't support Z0 yet. I'm only fixing this thoroughly because I really don't want it to come back to haunt me. Eli, could you review the documentation patch? I rearranged a little; this isn't the only target-specific knob in the remote protocol, so I made a place to further document such knobs. Tested on arm-none-linux-gnueabi. -- Daniel Jacobowitz CodeSourcery 2010-01-28 Daniel Jacobowitz gdb/ * arch-utils.c (default_remote_breakpoint_from_pc): New function. * arch-utils.h (default_remote_breakpoint_from_pc): Declare. * gdbarch.c, gdbarch.h: Regenerated. * gdbarch.sh (remote_breakpoint_from_pc): New architecture method. * remote.c (remote_insert_breakpoint, remote_insert_hw_breakpoint): Use gdbarch_remote_breakpoint_from_pc. gdb/doc/ * gdb.texinfo (Architecture-Specific Protocol Details): New section. Document ARM breakpoint types. (Register Packet Format): Move into the new section. (Packets): Describe the KIND argument for Z0, z0, Z1, and z1 packets. --- gdb/arch-utils.c | 7 ++++++ gdb/arch-utils.h | 3 ++ gdb/doc/gdb.texinfo | 60 +++++++++++++++++++++++++++++++++++++++------------- gdb/gdbarch.c | 25 +++++++++++++++++++++ gdb/gdbarch.h | 8 ++++++ gdb/gdbarch.sh | 4 +++ gdb/remote.c | 4 +-- 7 files changed, 95 insertions(+), 16 deletions(-) Index: gdb-mainline/gdb/arch-utils.c =================================================================== --- gdb-mainline.orig/gdb/arch-utils.c 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/arch-utils.c 2010-01-11 15:04:43.000000000 -0800 @@ -776,6 +776,13 @@ default_fast_tracepoint_valid_at (struct return 1; } +void +default_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, + int *kindptr) +{ + gdbarch_breakpoint_from_pc (gdbarch, pcptr, kindptr); +} + /* */ extern initialize_file_ftype _initialize_gdbarch_utils; /* -Wmissing-prototypes */ Index: gdb-mainline/gdb/arch-utils.h =================================================================== --- gdb-mainline.orig/gdb/arch-utils.h 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/arch-utils.h 2010-01-11 15:04:43.000000000 -0800 @@ -159,4 +159,7 @@ extern int default_fast_tracepoint_valid CORE_ADDR addr, int *isize, char **msg); +extern void default_remote_breakpoint_from_pc (struct gdbarch *, + CORE_ADDR *pcptr, int *kindptr); + #endif Index: gdb-mainline/gdb/doc/gdb.texinfo =================================================================== --- gdb-mainline.orig/gdb/doc/gdb.texinfo 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/doc/gdb.texinfo 2010-01-11 15:04:43.000000000 -0800 @@ -28114,7 +28114,7 @@ Show the current setting of the target w * Packets:: * Stop Reply Packets:: * General Query Packets:: -* Register Packet Format:: +* Architecture-Specific Protocol Details:: * Tracepoint Packets:: * Host I/O Packets:: * Interrupts:: @@ -28867,7 +28867,8 @@ for an error @cindex @samp{Z} packets Insert (@samp{Z}) or remove (@samp{z}) a @var{type} breakpoint or watchpoint starting at address @var{address} and covering the next -@var{length} bytes. +@var{length} bytes. @var{length} may also be a @var{kind} value +for breakpoint packets. Each breakpoint and watchpoint packet @var{type} is documented separately. @@ -28879,18 +28880,21 @@ remote target shall support either both avoid potential problems with duplicate packets, the operations should be implemented in an idempotent way.} -@item z0,@var{addr},@var{length} -@itemx Z0,@var{addr},@var{length} +@item z0,@var{addr},@var{kind} +@itemx Z0,@var{addr},@var{kind} @cindex @samp{z0} packet @cindex @samp{Z0} packet Insert (@samp{Z0}) or remove (@samp{z0}) a memory breakpoint at address -@var{addr} of size @var{length}. +@var{addr} of type @var{kind}. A memory breakpoint is implemented by replacing the instruction at @var{addr} with a software breakpoint or trap instruction. The -@var{length} is used by targets that indicates the size of the -breakpoint (in bytes) that should be inserted (e.g., the @sc{arm} and -@sc{mips} can insert either a 2 or 4 byte breakpoint). +@var{kind} is used by targets that have multiple breakpoint +instructions. It typically indicates the size of the breakpoint in +bytes that should be inserted (e.g., the @sc{arm} and @sc{mips} can +insert either a 2 or 4 byte breakpoint). Some architectures have +additional meanings for @var{kind}; @xref{Architecture-Specific +Protocol Details}. @emph{Implementation note: It is possible for a target to copy or move code that contains memory breakpoints (e.g., when implementing @@ -28907,15 +28911,16 @@ not supported for an error @end table -@item z1,@var{addr},@var{length} -@itemx Z1,@var{addr},@var{length} +@item z1,@var{addr},@var{kind} +@itemx Z1,@var{addr},@var{kind} @cindex @samp{z1} packet @cindex @samp{Z1} packet Insert (@samp{Z1}) or remove (@samp{z1}) a hardware breakpoint at -address @var{addr} of size @var{length}. +address @var{addr} of size @var{kind}. A hardware breakpoint is implemented using a mechanism that is not -dependant on being able to modify the target's memory. +dependant on being able to modify the target's memory. @var{kind} +has the same meaning as in @samp{Z0} packets. @emph{Implementation note: A hardware breakpoint is not affected by code movement.} @@ -30004,8 +30009,35 @@ A badly formed request or an error was e @end table -@node Register Packet Format -@section Register Packet Format +@node Architecture-Specific Protocol Details +@section Architecture-Specific Protocol Details + +This section describes how the remote protocol is applied to specific +target architectures. Also see @ref{Standard Target Features}, for +details of XML target descriptions for each architecture. + +@subsection ARM + +@subsubsection Breakpoint Kinds + +These breakpoint kinds are defined for the @samp{Z0} and @samp{Z1} packets. + +@table @r + +@item 2 +16-bit Thumb mode breakpoint. + +@item 3 +32-bit Thumb mode (Thumb-2) breakpoint. + +@item 4 +32-bit ARM mode breakpoint. + +@end table + +@subsection MIPS + +@subsubsection Register Packet Format The following @code{g}/@code{G} packets have previously been defined. In the below, some thirty-two bit registers are transferred as Index: gdb-mainline/gdb/gdbarch.c =================================================================== --- gdb-mainline.orig/gdb/gdbarch.c 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/gdbarch.c 2010-01-11 15:04:43.000000000 -0800 @@ -188,6 +188,7 @@ struct gdbarch gdbarch_skip_main_prologue_ftype *skip_main_prologue; gdbarch_inner_than_ftype *inner_than; gdbarch_breakpoint_from_pc_ftype *breakpoint_from_pc; + gdbarch_remote_breakpoint_from_pc_ftype *remote_breakpoint_from_pc; gdbarch_adjust_breakpoint_address_ftype *adjust_breakpoint_address; gdbarch_memory_insert_breakpoint_ftype *memory_insert_breakpoint; gdbarch_memory_remove_breakpoint_ftype *memory_remove_breakpoint; @@ -330,6 +331,7 @@ struct gdbarch startup_gdbarch = 0, /* skip_main_prologue */ 0, /* inner_than */ 0, /* breakpoint_from_pc */ + default_remote_breakpoint_from_pc, /* remote_breakpoint_from_pc */ 0, /* adjust_breakpoint_address */ default_memory_insert_breakpoint, /* memory_insert_breakpoint */ default_memory_remove_breakpoint, /* memory_remove_breakpoint */ @@ -456,6 +458,7 @@ gdbarch_alloc (const struct gdbarch_info gdbarch->value_from_register = default_value_from_register; gdbarch->pointer_to_address = unsigned_pointer_to_address; gdbarch->address_to_pointer = unsigned_address_to_pointer; + gdbarch->remote_breakpoint_from_pc = default_remote_breakpoint_from_pc; gdbarch->memory_insert_breakpoint = default_memory_insert_breakpoint; gdbarch->memory_remove_breakpoint = default_memory_remove_breakpoint; gdbarch->remote_register_number = default_remote_register_number; @@ -593,6 +596,8 @@ verify_gdbarch (struct gdbarch *gdbarch) fprintf_unfiltered (log, "\n\tinner_than"); if (gdbarch->breakpoint_from_pc == 0) fprintf_unfiltered (log, "\n\tbreakpoint_from_pc"); + if (gdbarch->remote_breakpoint_from_pc == default_remote_breakpoint_from_pc) + fprintf_unfiltered (log, "\n\tremote_breakpoint_from_pc"); /* Skip verify of adjust_breakpoint_address, has predicate */ /* Skip verify of memory_insert_breakpoint, invalid_p == 0 */ /* Skip verify of memory_remove_breakpoint, invalid_p == 0 */ @@ -1067,6 +1072,9 @@ gdbarch_dump (struct gdbarch *gdbarch, s "gdbarch_dump: regset_from_core_section = <%s>\n", host_address_to_string (gdbarch->regset_from_core_section)); fprintf_unfiltered (file, + "gdbarch_dump: remote_breakpoint_from_pc = <%s>\n", + host_address_to_string (gdbarch->remote_breakpoint_from_pc)); + fprintf_unfiltered (file, "gdbarch_dump: remote_register_number = <%s>\n", host_address_to_string (gdbarch->remote_register_number)); fprintf_unfiltered (file, @@ -2284,6 +2292,23 @@ set_gdbarch_breakpoint_from_pc (struct g gdbarch->breakpoint_from_pc = breakpoint_from_pc; } +void +gdbarch_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *kindptr) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->remote_breakpoint_from_pc != NULL); + if (gdbarch_debug >= 2) + fprintf_unfiltered (gdb_stdlog, "gdbarch_remote_breakpoint_from_pc called\n"); + gdbarch->remote_breakpoint_from_pc (gdbarch, pcptr, kindptr); +} + +void +set_gdbarch_remote_breakpoint_from_pc (struct gdbarch *gdbarch, + gdbarch_remote_breakpoint_from_pc_ftype remote_breakpoint_from_pc) +{ + gdbarch->remote_breakpoint_from_pc = remote_breakpoint_from_pc; +} + int gdbarch_adjust_breakpoint_address_p (struct gdbarch *gdbarch) { Index: gdb-mainline/gdb/gdbarch.h =================================================================== --- gdb-mainline.orig/gdb/gdbarch.h 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/gdbarch.h 2010-01-11 15:04:43.000000000 -0800 @@ -407,6 +407,14 @@ typedef const gdb_byte * (gdbarch_breakp extern const gdb_byte * gdbarch_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr); extern void set_gdbarch_breakpoint_from_pc (struct gdbarch *gdbarch, gdbarch_breakpoint_from_pc_ftype *breakpoint_from_pc); +/* Return the adjusted address and kind to use for Z0/Z1 packets. + KIND is usually the memory length of the breakpoint, but may have a + different target-specific meaning. */ + +typedef void (gdbarch_remote_breakpoint_from_pc_ftype) (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *kindptr); +extern void gdbarch_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *kindptr); +extern void set_gdbarch_remote_breakpoint_from_pc (struct gdbarch *gdbarch, gdbarch_remote_breakpoint_from_pc_ftype *remote_breakpoint_from_pc); + extern int gdbarch_adjust_breakpoint_address_p (struct gdbarch *gdbarch); typedef CORE_ADDR (gdbarch_adjust_breakpoint_address_ftype) (struct gdbarch *gdbarch, CORE_ADDR bpaddr); Index: gdb-mainline/gdb/gdbarch.sh =================================================================== --- gdb-mainline.orig/gdb/gdbarch.sh 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/gdbarch.sh 2010-01-11 15:04:43.000000000 -0800 @@ -486,6 +486,10 @@ m:CORE_ADDR:skip_prologue:CORE_ADDR ip:i M:CORE_ADDR:skip_main_prologue:CORE_ADDR ip:ip f:int:inner_than:CORE_ADDR lhs, CORE_ADDR rhs:lhs, rhs:0:0 m:const gdb_byte *:breakpoint_from_pc:CORE_ADDR *pcptr, int *lenptr:pcptr, lenptr::0: +# Return the adjusted address and kind to use for Z0/Z1 packets. +# KIND is usually the memory length of the breakpoint, but may have a +# different target-specific meaning. +m:void:remote_breakpoint_from_pc:CORE_ADDR *pcptr, int *kindptr:pcptr, kindptr::default_remote_breakpoint_from_pc: M:CORE_ADDR:adjust_breakpoint_address:CORE_ADDR bpaddr:bpaddr m:int:memory_insert_breakpoint:struct bp_target_info *bp_tgt:bp_tgt:0:default_memory_insert_breakpoint::0 m:int:memory_remove_breakpoint:struct bp_target_info *bp_tgt:bp_tgt:0:default_memory_remove_breakpoint::0 Index: gdb-mainline/gdb/remote.c =================================================================== --- gdb-mainline.orig/gdb/remote.c 2010-01-11 15:04:14.000000000 -0800 +++ gdb-mainline/gdb/remote.c 2010-01-11 15:04:43.000000000 -0800 @@ -7043,7 +7043,7 @@ remote_insert_breakpoint (struct gdbarch char *p; int bpsize; - gdbarch_breakpoint_from_pc (gdbarch, &addr, &bpsize); + gdbarch_remote_breakpoint_from_pc (gdbarch, &addr, &bpsize); rs = get_remote_state (); p = rs->buf; @@ -7245,7 +7245,7 @@ remote_insert_hw_breakpoint (struct gdba /* The length field should be set to the size of a breakpoint instruction, even though we aren't inserting one ourselves. */ - gdbarch_breakpoint_from_pc + gdbarch_remote_breakpoint_from_pc (gdbarch, &bp_tgt->placed_address, &bp_tgt->placed_size); if (remote_protocol_packets[PACKET_Z1].support == PACKET_DISABLE)