Re: [RFA 3/3] gdb_breakpoint cleanup: Add support for drpintf, trace, ftrace

[Keith Seitz <keiths at redhat dot com>]

On 04/29/2013 11:17 AM, Pedro Alves wrote:
> Sorry to be picky, but this "e.g." confuses me.  What other examples
> could there be?  Maybe you meant "i.e." ?  (Though that doesn't
> seem to add anything over what the option name itself tells us?)

You're right. I shouldn't make the same mistake that is too often made: 
our OWN developers aren't stupid; when a fast tracepoint is mentioned, 
they (should) know that the procedure is talking about the gdb command 
"ftrace".

I've just removed this unnecessary noise.

>   gdb_breakpoint break "foo"
>
> Note the "temporary" option would disappear this way, replaced by
> "tbreak".
>
> Just brain storming.  Even if that would be agreed to be desirable,
> I don't mind the options-based API going in -- it doesn't really make
> such a change harder later.

Ok, as a pretty old-time Tcl developer, I would offer the following 
alternative: Introduce options and optional arguments. In Tcl, this is 
almost trivial to do.

Currently, gdb_breakpoint just searches for well-known flags. With a 
small change, we could allow usage like:

# Set the default type (breakpoint), using a default test name
gdb_breakpoint "main"

# Set a temporary breakpoint, using a default test name
gdb_breakpoint "main" -type temporary

# Set a pending tracepoint with a given test name
gdb_breakpoint "main" -pending -type trace \
    -message "set pending breakpoint"

and so on. We've been using this style of API in Insight/libgui for a 
very long time.

What do you think?

> > --- a/gdb/testsuite/gdb.trace/change-loc.exp
> > +++ b/gdb/testsuite/gdb.trace/change-loc.exp
> > @@ -187,11 +187,7 @@ proc tracepoint_change_loc_2 { trace_type } {
> >   	gdb_start
> >   	gdb_reinitialize_dir $srcdir/$subdir
> >
> > -	gdb_test_multiple "${trace_type} set_tracepoint" "set pending tracepoint" {
> > -	    -re ".*Make \(|fast \)tracepoint pending.*y or \\\[n\\\]. $" {
> > -		gdb_test "y" "\(Fast t|T\)racepoint.*set_tracepoint.*pending." "set pending tracepoint"
> > -	    }
> > -	}
> > +	gdb_breakpoint "set_tracepoint" pending trace message
>
> It seems we lost $trace_type here.

Indeed. Fixed in this revision.

Keith

testsuite/ChangeLog
2013-04-26  Keith Seitz  <keiths@redhat.com>

	* lib/gdb.exp (gdb_breakpoint): Add support for dprintf,
	trace, and ftrace.
	* gdb.base/dprintf-pending.exp: Use gdb_breakpoint to set
	all dprintfs.
	* gdb.base/dprintf.exp: Likewise.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_2): Use
	gdb_breakpoint to set pending traces and ftraces.
	* gdb.trace/pending.exp (pending_tracepoint_resolved): Likewise.
	(pending_tracepoint_works): Likewise.
	(pending_tracepoint_resolved_during_trace): Likewise.
	(pending_tracepoint_installed_during_trace): Likewise.
	(pending_tracepoint_disconnect_during_trace): Likewise.
	(pending_tracepoint_disconnect_after_resolve): Likewise.
	(pending_tracepoint_with_action_resolved): Likewise.



diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 827295b..5d1b808 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -339,10 +339,13 @@ proc gdb_start_cmd {args} {
 #
 # Supported options are:
 # allow-pending -- permit a pending breakpoint to be set
-# temporary -- set a temporary breakpoint, e.g., "tbreak"
+# temporary -- set a temporary breakpoint
 # message -- print PASS messages*
 # no-message -- do not print FAIL messages*
 # pending -- set a pending breakpoint (FAIL if it is not pending)
+# trace -- set a tracepoint
+# ftrace -- set a fast tracepoint
+# dprintf -- set a dynamic printf
 #
 # The result is 1 for success, 0 for failure.
 #
@@ -365,12 +368,26 @@ proc gdb_breakpoint { function args } {
 	set pending_response "y"
     }
 
-    set break_command "break"
-    set break_message "Breakpoint"
-    set type "breakpoint"
-    if {[lsearch -exact $args temporary] != -1} {
-	set break_command "tbreak"
-	set break_message "Temporary breakpoint"
+    if {[lsearch -exact $args "dprintf"] != -1} {
+	set break_command "dprintf"
+	set break_message "Dprintf"
+	set type "dprintf"
+    } elseif {[lsearch -exact $args "temporary"] != -1} {
+	set break_command "break"
+	set break_message "Breakpoint"
+	set type "breakpoint"
+    } elseif {[lsearch -exact $args "trace"] != -1} {
+	set break_command "trace"
+	set break_message "Tracepoint"
+	set type "tracepoint"
+    } elseif {[lsearch -exact $args "ftrace"] != -1} {
+	set break_command "ftrace"
+	set break_message "Fast tracepoint"
+	set type "fast tracepoint"
+    } else {
+	set break_command "break"
+	set break_message "Breakpoint"
+	set type "breakpoint"
     }
 
     set print_pass 0
diff --git a/gdb/testsuite/gdb.base/dprintf-pending.exp b/gdb/testsuite/gdb.base/dprintf-pending.exp
index 87abfce..954024e 100644
--- a/gdb/testsuite/gdb.base/dprintf-pending.exp
+++ b/gdb/testsuite/gdb.base/dprintf-pending.exp
@@ -41,12 +41,7 @@ with_test_prefix "without format" {
     gdb_start
     gdb_reinitialize_dir $srcdir/$subdir
 
-    gdb_test \
-	"dprintf pendfunc" \
-	"Dprintf.*pendfunc.*pending." \
-	"set pending dprintf" \
-	".*Make dprintf pending.*y or \\\[n\\\]. $" \
-	"y"
+    gdb_breakpoint "pendfunc" pending dprintf message
 
     gdb_test "file ${binfile}" ".*Error in re-setting breakpoint.*" "resolved dprintf fails to be re-set"
 }
@@ -56,12 +51,7 @@ with_test_prefix "without symbols" {
     gdb_start
     gdb_reinitialize_dir $srcdir/$subdir
 
-    gdb_test \
-	"dprintf pendfunc1, \"x=%d\\n\", x" \
-	"Dprintf.*pendfunc1.*pending." \
-	"set pending dprintf" \
-	".*Make dprintf pending.*y or \\\[n\\\]. $" \
-	"y"
+    gdb_breakpoint "pendfunc1, \"x=%d\\n\", x" pending dprintf message
 
     gdb_test "info break" \
         "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -83,12 +73,7 @@ gdb_load_shlibs $lib_sl
 # Test setting, querying, and modifying pending breakpoints
 #
 
-gdb_test \
-    "dprintf pendfunc1, \"x=%d\\n\", x" \
-    "Dprintf.*pendfunc1.*pending." \
-    "set pending dprintf" \
-    ".*Make dprintf pending.*y or \\\[n\\\]. $" \
-    "y"
+gdb_breakpoint "pendfunc1, \"x=%d\\n\", x" pending dprintf message
 
 gdb_test "info break" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 71f3554..84d86d8 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -37,13 +37,11 @@ delete_breakpoints
 
 gdb_breakpoint "main"
 
-gdb_test "dprintf foo,\"At foo entry\\n\"" \
-  "Dprintf .*"
+gdb_breakpoint "foo,\"At foo entry\\n\"" dprintf message
 
 gdb_test "ignore \$bpnum 1" ".*Will ignore next crossing of breakpoint.*"
 
-gdb_test "dprintf $dp_location1,\"arg=%d, g=%d\\n\", arg, g" \
-  "Dprintf .*"
+gdb_breakpoint "$dp_location1,\"arg=%d, g=%d\\n\", arg, g" dprintf message
 
 gdb_test_sequence "info breakpoints" "dprintf info 1" {
     "\[\r\n\]Num     Type           Disp Enb Address +What"
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index 3a79b10..7bf8c2e 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -187,11 +187,7 @@ proc tracepoint_change_loc_2 { trace_type } {
 	gdb_start
 	gdb_reinitialize_dir $srcdir/$subdir
 
-	gdb_test_multiple "${trace_type} set_tracepoint" "set pending tracepoint" {
-	    -re ".*Make \(|fast \)tracepoint pending.*y or \\\[n\\\]. $" {
-		gdb_test "y" "\(Fast t|T\)racepoint.*set_tracepoint.*pending." "set pending tracepoint"
-	    }
-	}
+	gdb_breakpoint "set_tracepoint" pending $trace_type message
 
 	gdb_trace_setactions "set action for tracepoint" "" \
 	    "collect \$$pcreg" "^$"
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 0308e13..28ed5f0 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -71,12 +71,7 @@ proc pending_tracepoint_resolved { trace_type } {
 	gdb_start
 	gdb_reinitialize_dir $srcdir/$subdir
 
-	gdb_test_multiple "$trace_type set_point1" "set pending tracepoint" {
-	    -re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-		gdb_test "y" "\(Fast t|T\)racepoint.*set_point1.*pending." \
-		    "set pending tracepoint (without symbols)"
-	    }
-	}
+	gdb_breakpoint "set_point1" pending $trace_type message
 
 	gdb_test "info trace" \
 	    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -113,13 +108,7 @@ proc pending_tracepoint_works { trace_type } {
 
 	# Test setting and querying pending tracepoints
 
-	gdb_test_multiple "$trace_type set_point1" "set pending tracepoint" {
-	    -re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-		gdb_test "y" \
-		    "\(Fast t|T\)racepoint.*set_point1.*pending." \
-		    "set pending tracepoint"
-	    }
-	}
+	gdb_breakpoint "set_point1" pending $trace_type message
 
 	gdb_test "info trace" \
 	    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -187,12 +176,7 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
 	return -1
     }
 
-    gdb_test_multiple "$trace_type set_point2" "set pending tracepoint on set_point2" {
-	-re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-	    gdb_test "y" "\(Fast t|T\)racepoint.*set_point2.*pending." \
-		"set pending tracepoint (without symbols)"
-	}
-    }
+    gdb_breakpoint "set_point2" pending $trace_type message
 
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -268,12 +252,7 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 	"continue to marker 1"
 
     # Set a pending tracepoint during a tracing experiment.
-    gdb_test_multiple "$trace_type set_point2" "set pending tracepoint on set_point2" {
-	-re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-	    gdb_test "y" "\(Fast t|T\)racepoint.*set_point2.*pending." \
-		"set pending tracepoint"
-	}
-    }
+    gdb_breakpoint "set_point2" pending $trace_type message
 
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -329,12 +308,7 @@ proc pending_tracepoint_disconnect_during_trace { trace_type } \
 	return -1
     }
 
-    gdb_test_multiple "trace pendfunc3" "set pending tracepoint on set_point2" {
-	-re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-	    gdb_test "y" "\(Fast t|T\)racepoint.*pendfunc3.*pending." \
-		"set pending tracepoint on pendfun3"
-	}
-    }
+    gdb_breakpoint "pendfunc3" pending trace message
 
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
@@ -377,12 +351,7 @@ proc pending_tracepoint_disconnect_after_resolved { trace_type } \
 	return -1
     }
 
-    gdb_test_multiple "trace set_point2" "set pending tracepoint on set_point2" {
-	-re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-	    gdb_test "y" "\(Fast t|T\)racepoint.*set_point2.*pending." \
-		"set pending tracepoint on pendfun2"
-	}
-    }
+    gdb_breakpoint "set_point2" pending trace message
 
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
@@ -429,12 +398,7 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
 	return -1
     }
 
-    gdb_test_multiple "$trace_type set_point2" "set pending tracepoint on set_point2" {
-	-re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
-	    gdb_test "y" "\(Fast t|T\)racepoint.*set_point2.*pending." \
-		"set pending tracepoint (without symbols)"
-	}
-    }
+    gdb_breakpoint "set_point2" pending $trace_type message
 
     set pcreg "pc"
     if [is_amd64_regs_target] {