[PATCH] Fast tracepoints

[PATCH] Fast tracepoints

[Stan Shebs]

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

This patch adds "fast" tracepoints to GDB.  Formally, the meaning of a 
fast tracepoint is up to the architecture and target agent, and the user 
is simply requesting a faster implementation, with the possibility of 
the request being rejected, a la hardware breakpoints.  In our current 
implementation on x86, the target agent implements fast tracepoints as 
jumps directly into the agent, rather than as traps.

How fast is "fast" then?  We've gotten it down to under 100 clock cycles 
to save registers, test a conditional (with bytecodes compiled into x86 
machine code), and get back to the program - not much worse than a 
homemade logger compiled into your program.  By contrast, taking a trap 
can chew up 10,000 clocks or more, depending on kernel efficiency.  The 
tradeoff (and you knew this was coming :-) ) is that on x86 for example, 
the jump is a 5-byte instruction, and should only be inserted at the 
sites of instruction that are 5 bytes or longer.

Most of the interesting trickery is on the target side - but don't 
despair, Pedro has upcoming patches for a free version using gdbserver 
and a special library.  The GDB side is in this patch, mostly consisting 
of handling for a second type of tracepoint, and the 
architecture-specific oracle that checks request validity.

Stan

2010-01-04  Stan Shebs  <stan@codesourcery.com>

    Add fast tracepoints.
    * arch-utils.h (default_fast_tracepoint_valid_at): Declare.
    * arch-utils.c (default_fast_tracepoint_valid_at): New function.
    * breakpoint.h (enum bptype): Add bp_fast_tracepoint.
    * breakpoint.c (tracepoint_type): New function.
    (ALL_TRACEPOINTS): Use it.
    (should_be_inserted): Ditto.
    (bpstat_check_location): Ditto.
    (print_one_breakpoint_location): Ditto.
    (user_settable_breakpoint): Ditto.
    (set_breakpoint_location_function): Ditto.
    (disable_breakpoints_in_shlibs): Ditto.
    (delete_trace_command): Ditto.
    (print_it_typical): Add bp_fast_tracepoint case.
    (bpstat_what): Ditto.
    (print_one_breakpoint_location): Ditto.
    (allocate_bp_location): Ditto.
    (mention): Ditto.
    (breakpoint_re_set_one): Ditto.
    (disable_command): Ditto.
    (enable_command): Ditto.
    (check_fast_tracepoint_sals): New function.
    (break_command_really): Call it.
    (ftrace_command): New function.
    (_initialize_breakpoint): Add ftrace command.
    * gdbarch.sh (fast_tracepoint_valid_at): New.
    * gdbarch.h, gdbarch.c: Regenerate.
    * i386-tdep.c (i386_fast_tracepoint_valid_at): New function.
    (i386_gdbarch_init): Use it.
    * remote.c (struct remote_state): New field fast_tracepoints.
    (PACKET_FastTracepoints): New packet config type.
    (remote_fast_tracepoint_feature): New function.
    (remote_protocol_features): Add FastTracepoints.
    (remote_supports_fast_tracepoints): New function.
    (_initialize_remote): Add FastTracepoints.
    * tracepoint.c (download_tracepoint): Add fast tracepoint option.
    * NEWS: Mention fast tracepoints.

    * gdb.texinfo (Create and Delete Tracepoints): Describe fast
    tracepoints.
   
    * gdb.trace/tracecmd.exp: Test ftrace.




[-- Attachment #2: ftrace-patch-1 --]
[-- Type: text/plain, Size: 30928 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.344
diff -p -r1.344 NEWS
*** NEWS	31 Dec 2009 17:47:42 -0000	1.344
--- NEWS	5 Jan 2010 02:13:28 -0000
*************** Renesas RX			rx
*** 36,42 ****
    tracepoint actions and condition expressions.  Use the "tvariable"
    command to create, and "info tvariables" to view; see "Trace State
    Variables" in the manual for more detail.
!   
  * Changed commands
  
  disassemble
--- 36,54 ----
    tracepoint actions and condition expressions.  Use the "tvariable"
    command to create, and "info tvariables" to view; see "Trace State
    Variables" in the manual for more detail.
! 
! * Fast tracepoints
! 
!   GDB now includes an option for defining fast tracepoints, which
!   targets may implement more efficiently, such as by installing a jump
!   into the target agent rather than a trap instruction.  The resulting
!   speedup can be by two orders of magnitude or more, although the
!   tradeoff is that some program locations on some target architectures
!   might not allow fast tracepoint installation, for instance if the
!   instruction to be replaced is shorter than the jump. To request a
!   fast tracepoint, use the "ftrace" command, with syntax identical to
!   the regular trace command.
! 
  * Changed commands
  
  disassemble
*************** teval EXPR, ...
*** 101,106 ****
--- 113,121 ----
    Evaluate the given expressions without collecting anything into the
    trace buffer. (Valid in tracepoint actions only.)
  
+ ftrace FN / FILE:LINE / *ADDR
+   Define a fast tracepoint at the given function, line, or address.
+ 
  * New options
  
  set follow-exec-mode new|same
Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.185
diff -p -r1.185 arch-utils.c
*** arch-utils.c	1 Jan 2010 07:31:29 -0000	1.185
--- arch-utils.c	5 Jan 2010 02:13:28 -0000
*************** default_has_shared_address_space (struct
*** 765,770 ****
--- 765,779 ----
    return 0;
  }
  
+ int
+ default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
+ 				  CORE_ADDR addr, int *isize)
+ {
+   /* We don't know if maybe the target has some way to do fast
+      tracepoints that doesn't need gdbarch, so always say yes.  */
+   return 1;
+ }
+ 
  /* */
  
  extern initialize_file_ftype _initialize_gdbarch_utils; /* -Wmissing-prototypes */
Index: arch-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.h,v
retrieving revision 1.107
diff -p -r1.107 arch-utils.h
*** arch-utils.h	1 Jan 2010 07:31:29 -0000	1.107
--- arch-utils.h	5 Jan 2010 02:13:28 -0000
*************** extern struct gdbarch *get_current_arch 
*** 155,158 ****
--- 155,161 ----
  
  extern int default_has_shared_address_space (struct gdbarch *);
  
+ extern int default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
+ 					     CORE_ADDR addr, int *isize);
+ 
  #endif
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.446
diff -p -r1.446 breakpoint.c
*** breakpoint.c	1 Jan 2010 07:31:30 -0000	1.446
--- breakpoint.c	5 Jan 2010 02:13:28 -0000
*************** static int overlay_events_enabled;
*** 353,359 ****
  
  #define ALL_TRACEPOINTS(B)  \
    for (B = breakpoint_chain; B; B = B->next)  \
!     if ((B)->type == bp_tracepoint)
  
  /* Chains of all breakpoints defined.  */
  
--- 353,359 ----
  
  #define ALL_TRACEPOINTS(B)  \
    for (B = breakpoint_chain; B; B = B->next)  \
!     if (tracepoint_type (B))
  
  /* Chains of all breakpoints defined.  */
  
*************** clear_breakpoint_hit_counts (void)
*** 422,427 ****
--- 422,435 ----
      b->hit_count = 0;
  }
  
+ /* Encapsulate tests for different types of tracepoints.  */
+ 
+ static int
+ tracepoint_type (const struct breakpoint *b)
+ {
+   return (b->type == bp_tracepoint || b->type == bp_fast_tracepoint);
+ }
+   
  /* Default address, symtab and line to put a breakpoint at
     for "break" command with no arg.
     if default_breakpoint_valid is zero, the other three are
*************** should_be_inserted (struct bp_location *
*** 1282,1288 ****
  
    /* Tracepoints are inserted by the target at a time of its choosing,
       not by us.  */
!   if (bpt->owner->type == bp_tracepoint)
      return 0;
  
    return 1;
--- 1290,1296 ----
  
    /* Tracepoints are inserted by the target at a time of its choosing,
       not by us.  */
!   if (tracepoint_type (bpt->owner))
      return 0;
  
    return 1;
*************** print_it_typical (bpstat bs)
*** 2977,2982 ****
--- 2985,2991 ----
      case bp_watchpoint_scope:
      case bp_call_dummy:
      case bp_tracepoint:
+     case bp_fast_tracepoint:
      case bp_jit_event:
      default:
        result = PRINT_UNKNOWN;
*************** bpstat_check_location (const struct bp_l
*** 3322,3328 ****
  
    /* By definition, the inferior does not report stops at
       tracepoints.  */
!   if (b->type == bp_tracepoint)
      return 0;
  
    if (b->type != bp_watchpoint
--- 3331,3337 ----
  
    /* By definition, the inferior does not report stops at
       tracepoints.  */
!   if (tracepoint_type (b))
      return 0;
  
    if (b->type != bp_watchpoint
*************** bpstat_what (bpstat bs)
*** 3914,3924 ****
  	  retval.call_dummy = 1;
  	  break;
  	case bp_tracepoint:
  	  /* Tracepoint hits should not be reported back to GDB, and
  	     if one got through somehow, it should have been filtered
  	     out already.  */
  	  internal_error (__FILE__, __LINE__,
! 			  _("bpstat_what: bp_tracepoint encountered"));
  	  break;
  	}
        current_action = table[(int) bs_class][(int) current_action];
--- 3923,3934 ----
  	  retval.call_dummy = 1;
  	  break;
  	case bp_tracepoint:
+ 	case bp_fast_tracepoint:
  	  /* Tracepoint hits should not be reported back to GDB, and
  	     if one got through somehow, it should have been filtered
  	     out already.  */
  	  internal_error (__FILE__, __LINE__,
! 			  _("bpstat_what: tracepoint encountered"));
  	  break;
  	}
        current_action = table[(int) bs_class][(int) current_action];
*************** print_one_breakpoint_location (struct br
*** 4044,4049 ****
--- 4054,4060 ----
      {bp_longjmp_master, "longjmp master"},
      {bp_catchpoint, "catchpoint"},
      {bp_tracepoint, "tracepoint"},
+     {bp_fast_tracepoint, "fast tracepoint"},
      {bp_jit_event, "jit events"},
    };
    
*************** print_one_breakpoint_location (struct br
*** 4173,4178 ****
--- 4184,4190 ----
        case bp_overlay_event:
        case bp_longjmp_master:
        case bp_tracepoint:
+       case bp_fast_tracepoint:
        case bp_jit_event:
  	if (opts.addressprint)
  	  {
*************** print_one_breakpoint_location (struct br
*** 4258,4264 ****
           because the condition is an internal implementation detail
           that we do not want to expose to the user.  */
        annotate_field (7);
!       if (b->type == bp_tracepoint)
  	ui_out_text (uiout, "\ttrace only if ");
        else
  	ui_out_text (uiout, "\tstop only if ");
--- 4270,4276 ----
           because the condition is an internal implementation detail
           that we do not want to expose to the user.  */
        annotate_field (7);
!       if (tracepoint_type (b))
  	ui_out_text (uiout, "\ttrace only if ");
        else
  	ui_out_text (uiout, "\tstop only if ");
*************** user_settable_breakpoint (const struct b
*** 4451,4457 ****
    return (b->type == bp_breakpoint
  	  || b->type == bp_catchpoint
  	  || b->type == bp_hardware_breakpoint
! 	  || b->type == bp_tracepoint
  	  || b->type == bp_watchpoint
  	  || b->type == bp_read_watchpoint
  	  || b->type == bp_access_watchpoint
--- 4463,4469 ----
    return (b->type == bp_breakpoint
  	  || b->type == bp_catchpoint
  	  || b->type == bp_hardware_breakpoint
! 	  || tracepoint_type (b)
  	  || b->type == bp_watchpoint
  	  || b->type == bp_read_watchpoint
  	  || b->type == bp_access_watchpoint
*************** allocate_bp_location (struct breakpoint 
*** 4804,4809 ****
--- 4816,4822 ----
      {
      case bp_breakpoint:
      case bp_tracepoint:
+     case bp_fast_tracepoint:
      case bp_until:
      case bp_finish:
      case bp_longjmp:
*************** set_breakpoint_location_function (struct
*** 4900,4906 ****
  {
    if (loc->owner->type == bp_breakpoint
        || loc->owner->type == bp_hardware_breakpoint
!       || loc->owner->type == bp_tracepoint)
      {
        find_pc_partial_function (loc->address, &(loc->function_name), 
  				NULL, NULL);
--- 4913,4919 ----
  {
    if (loc->owner->type == bp_breakpoint
        || loc->owner->type == bp_hardware_breakpoint
!       || tracepoint_type (loc->owner))
      {
        find_pc_partial_function (loc->address, &(loc->function_name), 
  				NULL, NULL);
*************** disable_breakpoints_in_shlibs (void)
*** 5159,5165 ****
         to insert those breakpoints and fail.  */
      if (((b->type == bp_breakpoint)
  	 || (b->type == bp_hardware_breakpoint)
! 	 || (b->type == bp_tracepoint))
  	&& loc->pspace == current_program_space
  	&& !loc->shlib_disabled
  #ifdef PC_SOLIB
--- 5172,5178 ----
         to insert those breakpoints and fail.  */
      if (((b->type == bp_breakpoint)
  	 || (b->type == bp_hardware_breakpoint)
! 	 || (tracepoint_type (b)))
  	&& loc->pspace == current_program_space
  	&& !loc->shlib_disabled
  #ifdef PC_SOLIB
*************** mention (struct breakpoint *b)
*** 6091,6096 ****
--- 6104,6119 ----
  	printf_filtered (_(" %d"), b->number);
  	say_where = 1;
  	break;
+       case bp_fast_tracepoint:
+ 	if (ui_out_is_mi_like_p (uiout))
+ 	  {
+ 	    say_where = 0;
+ 	    break;
+ 	  }
+ 	printf_filtered (_("Fast tracepoint"));
+ 	printf_filtered (_(" %d"), b->number);
+ 	say_where = 1;
+ 	break;
  
        case bp_until:
        case bp_finish:
*************** breakpoint_sals_to_pc (struct symtabs_an
*** 6593,6598 ****
--- 6616,6645 ----
      resolve_sal_pc (&sals->sals[i]);
  }
  
+ /* Fast tracepoints may have restrictions on valid locations.  For
+    instance, a fast tracepoint using a jump instead of a trap will
+    likely have to overwrite more bytes than a trap would, and so can
+    only be placed where the instruction is longer than the jump, or a
+    multi-instruction sequence does not have a jump into the middle of
+    it, etc.  */
+ 
+ static void
+ check_fast_tracepoint_sals (struct symtabs_and_lines *sals)
+ {
+   int i;
+   struct symtab_and_line *sal;
+ 
+   for (i = 0; i < sals->nelts; i++)
+     {
+       sal = &sals->sals[i];
+ 
+       if (!gdbarch_fast_tracepoint_valid_at (get_current_arch (),
+ 					     sal->pc, NULL))
+ 	error (_("May not have a fast tracepoint at 0x%s"),
+ 	       paddress (get_current_arch (), sal->pc));
+     }
+ }
+ 
  static void
  do_captured_parse_breakpoint (struct ui_out *ui, void *data)
  {
*************** break_command_really (struct gdbarch *gd
*** 6794,6802 ****
      breakpoint_sals_to_pc (&sals, addr_start);
  
    type_wanted = (traceflag
! 		 ? bp_tracepoint
  		 : (hardwareflag ? bp_hardware_breakpoint : bp_breakpoint));
  
    /* Verify that condition can be parsed, before setting any
       breakpoints.  Allocate a separate condition expression for each
       breakpoint. */
--- 6841,6853 ----
      breakpoint_sals_to_pc (&sals, addr_start);
  
    type_wanted = (traceflag
! 		 ? (hardwareflag ? bp_fast_tracepoint : bp_tracepoint)
  		 : (hardwareflag ? bp_hardware_breakpoint : bp_breakpoint));
  
+   /* Fast tracepoints may have additional restrictions on location.  */
+   if (type_wanted == bp_fast_tracepoint)
+     check_fast_tracepoint_sals (&sals);
+ 
    /* Verify that condition can be parsed, before setting any
       breakpoints.  Allocate a separate condition expression for each
       breakpoint. */
*************** breakpoint_re_set_one (void *bint)
*** 8970,8975 ****
--- 9021,9027 ----
      case bp_breakpoint:
      case bp_hardware_breakpoint:
      case bp_tracepoint:
+     case bp_fast_tracepoint:
        /* Do not attempt to re-set breakpoints disabled during startup.  */
        if (b->enable_state == bp_startup_disabled)
  	return 0;
*************** disable_command (char *args, int from_tt
*** 9369,9374 ****
--- 9421,9427 ----
  	continue;
        case bp_breakpoint:
        case bp_tracepoint:
+       case bp_fast_tracepoint:
        case bp_catchpoint:
        case bp_hardware_breakpoint:
        case bp_watchpoint:
*************** enable_command (char *args, int from_tty
*** 9462,9467 ****
--- 9515,9521 ----
  	continue;
        case bp_breakpoint:
        case bp_tracepoint:
+       case bp_fast_tracepoint:
        case bp_catchpoint:
        case bp_hardware_breakpoint:
        case bp_watchpoint:
*************** trace_command (char *arg, int from_tty)
*** 9769,9774 ****
--- 9823,9844 ----
    set_tracepoint_count (breakpoint_count);
  }
  
+ void
+ ftrace_command (char *arg, int from_tty)
+ {
+   break_command_really (get_current_arch (),
+ 			arg, 
+ 			NULL, 0, 1 /* parse arg */,
+ 			0 /* tempflag */, 1 /* hardwareflag */,
+ 			1 /* traceflag */,
+ 			0 /* Ignore count */,
+ 			pending_break_support, 
+ 			NULL,
+ 			from_tty,
+ 			1 /* enabled */);
+   set_tracepoint_count (breakpoint_count);
+ }
+ 
  /* Print information on tracepoint number TPNUM_EXP, or all if
     omitted.  */
  
*************** delete_trace_command (char *arg, int fro
*** 9846,9852 ****
  	{
  	  ALL_BREAKPOINTS_SAFE (b, temp)
  	  {
! 	    if (b->type == bp_tracepoint
  		&& b->number >= 0)
  	      delete_breakpoint (b);
  	  }
--- 9916,9922 ----
  	{
  	  ALL_BREAKPOINTS_SAFE (b, temp)
  	  {
! 	    if (tracepoint_type (b)
  		&& b->number >= 0)
  	      delete_breakpoint (b);
  	  }
*************** Do \"help tracepoints\" for info on othe
*** 10501,10506 ****
--- 10571,10583 ----
    add_com_alias ("tra", "trace", class_alias, 1);
    add_com_alias ("trac", "trace", class_alias, 1);
  
+   c = add_com ("ftrace", class_breakpoint, ftrace_command, _("\
+ Set a fast tracepoint at specified line or function.\n\
+ \n"
+ BREAK_ARGS_HELP ("ftrace") "\n\
+ Do \"help tracepoints\" for info on other tracepoint commands."));
+   set_cmd_completer (c, location_completer);
+ 
    add_info ("tracepoints", tracepoints_info, _("\
  Status of tracepoints, or tracepoint number NUMBER.\n\
  Convenience variable \"$tpnum\" contains the number of the\n\
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.105
diff -p -r1.105 breakpoint.h
*** breakpoint.h	1 Jan 2010 07:31:30 -0000	1.105
--- breakpoint.h	5 Jan 2010 02:13:28 -0000
*************** enum bptype
*** 121,126 ****
--- 121,127 ----
      bp_catchpoint,
  
      bp_tracepoint,
+     bp_fast_tracepoint,
  
      /* Event for JIT compiled code generation or deletion.  */
      bp_jit_event,
Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.457
diff -p -r1.457 gdbarch.c
*** gdbarch.c	11 Nov 2009 20:05:33 -0000	1.457
--- gdbarch.c	5 Jan 2010 02:13:28 -0000
*************** struct gdbarch
*** 250,255 ****
--- 250,256 ----
    int has_global_solist;
    int has_global_breakpoints;
    gdbarch_has_shared_address_space_ftype *has_shared_address_space;
+   gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at;
  };
  
  
*************** struct gdbarch startup_gdbarch =
*** 391,396 ****
--- 392,398 ----
    0,  /* has_global_solist */
    0,  /* has_global_breakpoints */
    default_has_shared_address_space,  /* has_shared_address_space */
+   default_fast_tracepoint_valid_at,  /* fast_tracepoint_valid_at */
    /* startup_gdbarch() */
  };
  
*************** gdbarch_alloc (const struct gdbarch_info
*** 475,480 ****
--- 477,483 ----
    gdbarch->target_signal_from_host = default_target_signal_from_host;
    gdbarch->target_signal_to_host = default_target_signal_to_host;
    gdbarch->has_shared_address_space = default_has_shared_address_space;
+   gdbarch->fast_tracepoint_valid_at = default_fast_tracepoint_valid_at;
    /* gdbarch_alloc() */
  
    return gdbarch;
*************** verify_gdbarch (struct gdbarch *gdbarch)
*** 653,658 ****
--- 656,662 ----
    /* Skip verify of has_global_solist, invalid_p == 0 */
    /* Skip verify of has_global_breakpoints, invalid_p == 0 */
    /* Skip verify of has_shared_address_space, invalid_p == 0 */
+   /* Skip verify of fast_tracepoint_valid_at, invalid_p == 0 */
    buf = ui_file_xstrdup (log, &length);
    make_cleanup (xfree, buf);
    if (length > 0)
*************** gdbarch_dump (struct gdbarch *gdbarch, s
*** 826,831 ****
--- 830,838 ----
                        "gdbarch_dump: elf_make_msymbol_special = <%s>\n",
                        host_address_to_string (gdbarch->elf_make_msymbol_special));
    fprintf_unfiltered (file,
+                       "gdbarch_dump: fast_tracepoint_valid_at = <%s>\n",
+                       host_address_to_string (gdbarch->fast_tracepoint_valid_at));
+   fprintf_unfiltered (file,
                        "gdbarch_dump: gdbarch_fetch_pointer_argument_p() = %d\n",
                        gdbarch_fetch_pointer_argument_p (gdbarch));
    fprintf_unfiltered (file,
*************** set_gdbarch_has_shared_address_space (st
*** 3528,3533 ****
--- 3535,3557 ----
    gdbarch->has_shared_address_space = has_shared_address_space;
  }
  
+ int
+ gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize)
+ {
+   gdb_assert (gdbarch != NULL);
+   gdb_assert (gdbarch->fast_tracepoint_valid_at != NULL);
+   if (gdbarch_debug >= 2)
+     fprintf_unfiltered (gdb_stdlog, "gdbarch_fast_tracepoint_valid_at called\n");
+   return gdbarch->fast_tracepoint_valid_at (gdbarch, addr, isize);
+ }
+ 
+ void
+ set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
+                                       gdbarch_fast_tracepoint_valid_at_ftype fast_tracepoint_valid_at)
+ {
+   gdbarch->fast_tracepoint_valid_at = fast_tracepoint_valid_at;
+ }
+ 
  
  /* Keep a registry of per-architecture data-pointers required by GDB
     modules. */
Index: gdbarch.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.407
diff -p -r1.407 gdbarch.h
*** gdbarch.h	11 Nov 2009 20:05:33 -0000	1.407
--- gdbarch.h	5 Jan 2010 02:13:28 -0000
*************** typedef int (gdbarch_has_shared_address_
*** 909,914 ****
--- 909,920 ----
  extern int gdbarch_has_shared_address_space (struct gdbarch *gdbarch);
  extern void set_gdbarch_has_shared_address_space (struct gdbarch *gdbarch, gdbarch_has_shared_address_space_ftype *has_shared_address_space);
  
+ /* True if a fast tracepoint can be set at an address. */
+ 
+ typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize);
+ extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize);
+ extern void set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at);
+ 
  /* Definition for an unknown syscall, used basically in error-cases.  */
  #define UNKNOWN_SYSCALL (-1)
  
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.294
diff -p -r1.294 i386-tdep.c
*** i386-tdep.c	1 Jan 2010 07:31:34 -0000	1.294
--- i386-tdep.c	5 Jan 2010 02:13:28 -0000
***************
*** 43,48 ****
--- 43,49 ----
  #include "target.h"
  #include "value.h"
  #include "dis-asm.h"
+ #include "disasm.h"
  
  #include "gdb_assert.h"
  #include "gdb_string.h"
*************** static const int i386_record_regmap[] =
*** 5567,5572 ****
--- 5568,5611 ----
    I386_DS_REGNUM, I386_ES_REGNUM, I386_FS_REGNUM, I386_GS_REGNUM
  };
  
+ /* Check that the given address appears suitable for a fast
+    tracepoint, which on x86 means that we need an instruction of at
+    least 5 bytes, so that we can overwrite it with a 4-byte-offset
+    jump and not have to worry about program jumps to an address in the
+    middle of the tracepoint jump.  */
+ 
+ static int
+ i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
+ 			       CORE_ADDR addr, int *isize)
+ {
+   int len, jumplen;
+   static struct ui_file *gdb_null = NULL;
+ 
+   /* This is based on the target agent using a 4-byte relative jump.
+      Alternate future possibilities include 8-byte offset for x86-84,
+      or 3-byte jumps if the program has trampoline space close by.  */
+   jumplen = 5;
+ 
+   /* Dummy file descriptor for the disassembler.  */
+   if (!gdb_null)
+     gdb_null = ui_file_new ();
+ 
+   /* Check for fit.  */
+   len = gdb_print_insn (gdbarch, addr, gdb_null, NULL);
+   if (len < jumplen)
+     {
+       /* Put out a bit of target-specific feedback, caller won't have this
+ 	 kind of detail.  */
+       printf_filtered (_("Instruction at 0x%s is only %d bytes long, need at least %d bytes for the fast tracepoint jump\n"),
+ 		       paddress (gdbarch, addr), len, jumplen);
+       return 0;
+     }
+ 
+   if (isize)
+     *isize = len;
+   return 1;
+ }
+ 
  \f
  static struct gdbarch *
  i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
*************** i386_gdbarch_init (struct gdbarch_info i
*** 5769,5774 ****
--- 5808,5816 ----
    set_gdbarch_skip_permanent_breakpoint (gdbarch,
  					 i386_skip_permanent_breakpoint);
  
+   set_gdbarch_fast_tracepoint_valid_at (gdbarch,
+ 					i386_fast_tracepoint_valid_at);
+ 
    return gdbarch;
  }
  
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.376
diff -p -r1.376 remote.c
*** remote.c	1 Jan 2010 07:31:40 -0000	1.376
--- remote.c	5 Jan 2010 02:13:28 -0000
*************** struct remote_state
*** 298,303 ****
--- 298,306 ----
    /* True if the stub reports support for conditional tracepoints.  */
    int cond_tracepoints;
  
+   /* True if the stub reports support for fast tracepoints.  */
+   int fast_tracepoints;
+ 
    /* Nonzero if the user has pressed Ctrl-C, but the target hasn't
       responded to that.  */
    int ctrlc_pending_p;
*************** enum {
*** 1066,1071 ****
--- 1069,1075 ----
    PACKET_qXfer_siginfo_write,
    PACKET_qAttached,
    PACKET_ConditionalTracepoints,
+   PACKET_FastTracepoints,
    PACKET_bc,
    PACKET_bs,
    PACKET_MAX
*************** remote_cond_tracepoint_feature (const st
*** 3136,3141 ****
--- 3140,3154 ----
    rs->cond_tracepoints = (support == PACKET_ENABLE);
  }
  
+ static void
+ remote_fast_tracepoint_feature (const struct protocol_feature *feature,
+ 				enum packet_support support,
+ 				const char *value)
+ {
+   struct remote_state *rs = get_remote_state ();
+   rs->fast_tracepoints = (support == PACKET_ENABLE);
+ }
+ 
  static struct protocol_feature remote_protocol_features[] = {
    { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
    { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
*************** static struct protocol_feature remote_pr
*** 3164,3169 ****
--- 3177,3184 ----
      PACKET_qXfer_siginfo_write },
    { "ConditionalTracepoints", PACKET_DISABLE, remote_cond_tracepoint_feature,
      PACKET_ConditionalTracepoints },
+   { "FastTracepoints", PACKET_DISABLE, remote_fast_tracepoint_feature,
+     PACKET_FastTracepoints },
    { "ReverseContinue", PACKET_DISABLE, remote_supported_packet,
      PACKET_bc },
    { "ReverseStep", PACKET_DISABLE, remote_supported_packet,
*************** remote_supports_cond_tracepoints (void)
*** 8895,8900 ****
--- 8910,8922 ----
    return rs->cond_tracepoints;
  }
  
+ int
+ remote_supports_fast_tracepoints (void)
+ {
+   struct remote_state *rs = get_remote_state ();
+   return rs->fast_tracepoints;
+ }
+ 
  static void
  init_remote_ops (void)
  {
*************** Show the maximum size of the address (in
*** 9371,9376 ****
--- 9393,9400 ----
  
    add_packet_config_cmd (&remote_protocol_packets[PACKET_ConditionalTracepoints],
  			 "ConditionalTracepoints", "conditional-tracepoints", 0);
+   add_packet_config_cmd (&remote_protocol_packets[PACKET_FastTracepoints],
+ 			 "FastTracepoints", "fast-tracepoints", 0);
  
    /* Keep the old ``set remote Z-packet ...'' working.  Each individual
       Z sub-packet has its own set and show commands, but users may
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.134
diff -p -r1.134 tracepoint.c
*** tracepoint.c	1 Jan 2010 07:31:42 -0000	1.134
--- tracepoint.c	5 Jan 2010 02:13:28 -0000
***************
*** 34,39 ****
--- 34,40 ----
  #include "tracepoint.h"
  #include "remote.h"
  extern int remote_supports_cond_tracepoints (void);
+ extern int remote_supports_fast_tracepoints (void);
  extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
  #include "linespec.h"
  #include "regcache.h"
*************** trace_start_command (char *args, int fro
*** 1690,1695 ****
--- 1691,1697 ----
  void
  download_tracepoint (struct breakpoint *t)
  {
+   CORE_ADDR tpaddr;
    char tmp[40];
    char buf[2048];
    char **tdp_actions;
*************** download_tracepoint (struct breakpoint *
*** 1699,1709 ****
    struct agent_expr *aexpr;
    struct cleanup *aexpr_chain = NULL;
  
!   sprintf_vma (tmp, (t->loc ? t->loc->address : 0));
    sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", t->number, 
  	   tmp, /* address */
  	   (t->enable_state == bp_enabled ? 'E' : 'D'),
  	   t->step_count, t->pass_count);
    /* If the tracepoint has a conditional, make it into an agent
       expression and append to the definition.  */
    if (t->loc->cond)
--- 1701,1738 ----
    struct agent_expr *aexpr;
    struct cleanup *aexpr_chain = NULL;
  
!   tpaddr = t->loc->address;
!   sprintf_vma (tmp, (t->loc ? tpaddr : 0));
    sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", t->number, 
  	   tmp, /* address */
  	   (t->enable_state == bp_enabled ? 'E' : 'D'),
  	   t->step_count, t->pass_count);
+   /* Fast tracepoints are mostly handled by the target, but we can
+      tell the target how big of an instruction block should be moved
+      around.  */
+   if (t->type == bp_fast_tracepoint)
+     {
+       /* Only test support at download time, we may not know target
+ 	 capabilities at definition time.  */
+       if (remote_supports_fast_tracepoints ())
+ 	{
+ 	  int isize;
+ 
+ 	  if (gdbarch_fast_tracepoint_valid_at (get_current_arch (),
+ 						tpaddr, &isize))
+ 	    sprintf (buf + strlen (buf), ":F%x,", isize);
+ 	  else
+ 	    /* If it passed validation at definition but fails now,
+ 	       something is very wrong.  */
+ 	    internal_error (__FILE__, __LINE__,
+ 			    "Fast tracepoint not valid during download");
+ 	}
+       else
+ 	/* Fast tracepoints are functionally identical to regular
+ 	   tracepoints, so don't take lack of support as a reason to
+ 	   give up on the trace run.  */
+ 	warning (_("Target does not support fast tracepoints, downloading %d as regular tracepoint"), t->number);
+     }
    /* If the tracepoint has a conditional, make it into an agent
       expression and append to the definition.  */
    if (t->loc->cond)
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.501
diff -p -r1.501 gdbarch.sh
*** gdbarch.sh	1 Jan 2010 07:31:32 -0000	1.501
--- gdbarch.sh	5 Jan 2010 02:13:28 -0000
*************** v:int:has_global_breakpoints:::0:0::0
*** 759,764 ****
--- 759,767 ----
  
  # True if inferiors share an address space (e.g., uClinux).
  m:int:has_shared_address_space:void:::default_has_shared_address_space::0
+ 
+ # True if a fast tracepoint can be set at an address.
+ m:int:fast_tracepoint_valid_at:CORE_ADDR addr, int *isize:addr, isize::default_fast_tracepoint_valid_at::0
  EOF
  }
  
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.654
diff -p -r1.654 gdb.texinfo
*** doc/gdb.texinfo	1 Jan 2010 07:54:37 -0000	1.654
--- doc/gdb.texinfo	5 Jan 2010 02:13:29 -0000
*************** expressions and ignore counts on tracepo
*** 9323,9328 ****
--- 9323,9332 ----
  tracepoints cannot run @value{GDBN} commands when they are
  hit.  Tracepoints may not be thread-specific either.
  
+ Some targets may support @dfn{fast tracepoints}, which are inserted in
+ a different way (such as with a jump instead of a trap), that is
+ faster but possibly restricted in where they may be installed.
+ 
  This section describes commands to set tracepoints and associated
  conditions and actions.
  
*************** if the value is nonzero---that is, if @v
*** 9378,9383 ****
--- 9382,9402 ----
  @xref{Tracepoint Conditions, ,Tracepoint Conditions}, for more
  information on tracepoint conditions.
  
+ @item ftrace @var{location} [ if @var{cond} ]
+ @cindex set fast tracepoint
+ @cindex fast tracepoints
+ @kindex ftrace
+ The @code{ftrace} command sets a fast tracepoint.  For targets that
+ support it, fast tracepoints will use a more efficient but possibly
+ less general technique to trigger data collection, such as a jump
+ instruction instead of a trap, or some sort of hardware support.  It
+ may not be possible to create a fast tracepoint at the desired
+ location, in which case the command will exit with an explanatory
+ message.
+ 
+ @value{GDBN} handles arguments to @code{ftrace} exactly as for
+ @code{trace}.
+ 
  @vindex $tpnum
  @cindex last tracepoint number
  @cindex recent tracepoint number
Index: testsuite/gdb.trace/tracecmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/tracecmd.exp,v
retrieving revision 1.15
diff -p -r1.15 tracecmd.exp
*** testsuite/gdb.trace/tracecmd.exp	1 Jan 2010 07:32:07 -0000	1.15
--- testsuite/gdb.trace/tracecmd.exp	5 Jan 2010 02:13:29 -0000
*************** gdb_test "info trace" "in gdb_recursion_
*** 164,167 ****
--- 164,179 ----
  # 1.14 help trace
  gdb_test "help trace" "Set a tracepoint at .*" "1.14: help trace"
  
+ # 1.15 ftrace
  
+ gdb_delete_tracepoints
+ 
+ send_gdb "ftrace gdb_recursion_test\n"
+ # Acceptance vs rejection of a location are target-specific, so allow both.
+ gdb_expect {
+     -re "Fast tracepoint $decimal at $hex: file.*$srcfile, line $testline1.*$gdb_prompt $"
+     { pass "Set a fast tracepoint" }
+     -re ".*May not have a fast tracepoint at $hex.*$gdb_prompt $"
+     { pass "Declined to set a fast tracepoint" }
+     timeout { fail "Timeout while setting fast tracepoint" }
+ }

Re: [PATCH] Fast tracepoints

[Eli Zaretskii]

> Date: Mon, 04 Jan 2010 18:38:32 -0800
> From: Stan Shebs <stan@codesourcery.com>
> 
> This patch adds "fast" tracepoints to GDB.

Thanks.

> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.344
> diff -p -r1.344 NEWS
> *** NEWS	31 Dec 2009 17:47:42 -0000	1.344
> --- NEWS	5 Jan 2010 02:13:28 -0000

This part is OK, but please give me 2 spaces between sentences.

> *** doc/gdb.texinfo	1 Jan 2010 07:54:37 -0000	1.654
> --- doc/gdb.texinfo	5 Jan 2010 02:13:29 -0000

A few comments for this part:

> + Some targets may support @dfn{fast tracepoints}, which are inserted in
> + a different way (such as with a jump instead of a trap), that is
> + faster but possibly restricted in where they may be installed.

Please put the "@cindex fast tracepoints" entry here, not 50 lines
below.

> + The @code{ftrace} command sets a fast tracepoint.  For targets that
> + support it, fast tracepoints will use a more efficient but possibly

"For targets that support them, fast tracepoints will use ..."

Okay with those changes.

Re: [PATCH] Fast tracepoints

[Tom Tromey]

>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:

Stan> This patch adds "fast" tracepoints to GDB.

Neat.

Stan> Most of the interesting trickery is on the target side - but don't
Stan> despair, Pedro has upcoming patches for a free version using gdbserver
Stan> and a special library.

I wonder whether this could somehow work with systemtap probe points.

Stan> + static void
Stan> + check_fast_tracepoint_sals (struct symtabs_and_lines *sals)
[...]
Stan> +       if (!gdbarch_fast_tracepoint_valid_at (get_current_arch (),
Stan> + 					     sal->pc, NULL))
Stan> + 	error (_("May not have a fast tracepoint at 0x%s"),
Stan> + 	       paddress (get_current_arch (), sal->pc));

Stan> + static int
Stan> + i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
Stan> + 			       CORE_ADDR addr, int *isize)
[...]
Stan> +       printf_filtered (_("Instruction at 0x%s is only %d bytes long, need at least %d bytes for the fast tracepoint jump\n"),
Stan> + 		       paddress (gdbarch, addr), len, jumplen);

It seems like it would be nicer for the user if the _at method returned
a reason, so that it could be printed as "May not have a fast
tracepoint: ... : reason".

Similarly, it seems a little strange to me that a predicate function is
expected to print a warning.

Tom

Re: [PATCH] Fast tracepoints

[Stan Shebs]

Tom Tromey wrote:
>>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
>>>>>>             
>
> Stan> This patch adds "fast" tracepoints to GDB.
>
> Neat.
>
> Stan> Most of the interesting trickery is on the target side - but don't
> Stan> despair, Pedro has upcoming patches for a free version using gdbserver
> Stan> and a special library.
>
> I wonder whether this could somehow work with systemtap probe points.
>   
One of our other projects is to do "static tracepoints" that originate 
outside GDB, and one of the side effects of the upcoming disconnected 
tracing patch is that it introduces the idea of keeping the tracepoint 
definition in the target.  So it seems like a useful convergence to have 
a tool-independent notion of a probe/tracepoint that can be used in 
different contexts.
> It seems like it would be nicer for the user if the _at method returned
> a reason, so that it could be printed as "May not have a fast
> tracepoint: ... : reason".
>   
Yeah, I considered it.  It seemed like a localization mess to be 
constructing strings from strings, plus I find it distasteful to have 
the callee decide to allocate space in a way that compels the caller to 
think about what to do with it.  But as you observe, predicates doing 
printouts isn't great either.

Stan

Re: [PATCH] Fast tracepoints

[Mark Wielaard]

Hi Stan,

On Tue, 2010-01-05 at 16:54 -0800, Stan Shebs wrote:
> Tom Tromey wrote:
> >>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
> > Stan> This patch adds "fast" tracepoints to GDB.
> >
> > Neat.
> >
> > Stan> Most of the interesting trickery is on the target side - but don't
> > Stan> despair, Pedro has upcoming patches for a free version using gdbserver
> > Stan> and a special library.
> >
> > I wonder whether this could somehow work with systemtap probe points.
> >   
> One of our other projects is to do "static tracepoints" that originate 
> outside GDB, and one of the side effects of the upcoming disconnected 
> tracing patch is that it introduces the idea of keeping the tracepoint 
> definition in the target.  So it seems like a useful convergence to have 
> a tool-independent notion of a probe/tracepoint that can be used in 
> different contexts.

If you have a proposal please do CC systemtap@sourceware.org (I added it
to the CC of this email already). Currently the way systemtap user space
static probe points are handled is by having dtrace compatible source
code markers in the application. We choose that representation in the
source files to maximize reuse of static user space tracepoints across
application/os domains. See also:
http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps
If we could reuse the definitions in sys/sdt.h as the canonical way of
adding static tracepoints to an application usable from multiple
debuggers/profilers/tracers that would be wonderful.

The markers are currently stored in a special .probes elf section inside
the application or shared library. I don't believe that format is
documented yet (partly because there are still some experiments going on
with adding different representations), but we probably should document
it formally if we would like gdb to reuse it.

Cheers,

Mark

Re: [PATCH] Fast tracepoints

[Tom Tromey]

>>>>> "Stan" == Stan Shebs <stanshebs@earthlink.net> writes:

Stan> Yeah, I considered it.  It seemed like a localization mess to be
Stan> constructing strings from strings, plus I find it distasteful to have
Stan> the callee decide to allocate space in a way that compels the caller
Stan> to think about what to do with it.  But as you observe, predicates
Stan> doing printouts isn't great either.

Yeah... well, just for the record, what you have is ok by me.

Tom

Re: [PATCH] Fast tracepoints

[Stan Shebs]

Tom Tromey wrote:
>>>>>> "Stan" == Stan Shebs <stanshebs@earthlink.net> writes:
>>>>>>             
>
> Stan> Yeah, I considered it.  It seemed like a localization mess to be
> Stan> constructing strings from strings, plus I find it distasteful to have
> Stan> the callee decide to allocate space in a way that compels the caller
> Stan> to think about what to do with it.  But as you observe, predicates
> Stan> doing printouts isn't great either.
>
> Yeah... well, just for the record, what you have is ok by me.
>   
Heh - I thought about it more, and ended up changing to return the 
informational string after all.  The prospect of multiple prints from 
multiple calls was the most distasteful of all! :-)

Stan