[PATCH] gdb, record: rephrase the 'not recording' error message

[PATCH] gdb, record: rephrase the 'not recording' error message

[Markus Metzger via Gdb-patches]

When trying to use one of the record commands without having enabled
recording first, GDB gives the error message:

    (gdb) record function-call-history
    No record target is currently active.
    Use one of the "target record-<TAB><TAB>" commands first.

In the record help, however, we say:

    (gdb) help record
    record, rec
    Start recording.

    List of record subcommands:

    record btrace, record b -- Start branch trace recording.
    record delete, record del, record d -- Delete the rest of execution log and start recording it anew.
    record full -- Start full execution recording.
    record function-call-history -- Prints the execution history at function granularity.
    record goto -- Restore the program to its state at instruction number N.
    record instruction-history -- Print disassembled instructions stored in the execution log.
    record save -- Save the execution log to a file.
    record stop, record s -- Stop the record/replay target.

Change the above error message to

    (gdb) record function-call-history
    No recording is currently active.
    Use the "record full" or "record btrace" command first.

to align with the help text.

gdb/ChangeLog:
2020-12-03  Markus Metzger  <markus.t.metzger@intel.com>

	* record.c (require_record_target): Rephrase error message.
	(info_record_command): Likewise.

gdb/testsuite/ChangeLog:
2020-12-03  Markus Metzger  <markus.t.metzger@intel.com>

	* gdb.btrace/enable.exp: Update error message.
	* gdb.btrace/multi-inferior.exp: Likewise.
	* gdb.btrace/reconnect.exp: Likewise.
	* gdb.python/py-record-btrace.exp: Likewise.
	* gdb.python/py-record-full.exp: Likewise.
---
 gdb/record.c                                  |  6 +++---
 gdb/testsuite/gdb.btrace/enable.exp           | 10 +++++-----
 gdb/testsuite/gdb.btrace/multi-inferior.exp   |  4 ++--
 gdb/testsuite/gdb.btrace/reconnect.exp        |  2 +-
 gdb/testsuite/gdb.python/py-record-btrace.exp |  2 +-
 gdb/testsuite/gdb.python/py-record-full.exp   |  2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gdb/record.c b/gdb/record.c
index f9141192779..204bda0a424 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -74,8 +74,8 @@ require_record_target (void)
 
   t = find_record_target ();
   if (t == NULL)
-    error (_("No record target is currently active.\n"
-	     "Use one of the \"target record-<TAB><TAB>\" commands first."));
+    error (_("No recording is currently active.\n"
+	     "Use the \"record full\" or \"record btrace\" command first."));
 
   return t;
 }
@@ -325,7 +325,7 @@ info_record_command (const char *args, int from_tty)
   t = find_record_target ();
   if (t == NULL)
     {
-      printf_filtered (_("No record target is currently active.\n"));
+      printf_filtered (_("No recording is currently active.\n"));
       return;
     }
 
diff --git a/gdb/testsuite/gdb.btrace/enable.exp b/gdb/testsuite/gdb.btrace/enable.exp
index a968d410384..8ce1dab076f 100644
--- a/gdb/testsuite/gdb.btrace/enable.exp
+++ b/gdb/testsuite/gdb.btrace/enable.exp
@@ -27,15 +27,15 @@ gdb_exit
 gdb_start
 
 # record cannot be stopped, if it was never active
-gdb_test "record stop" "No record target is currently active\\..*" "record stop without target"
+gdb_test "record stop" "No recording is currently active\\..*" "record stop without target"
 
 # btrace cannot be enabled without a running inferior
 gdb_test "record btrace" "The program is not being run\\." "record btrace without running program"
 
 # no function and no instruction history without btrace enabled
-gdb_test "record function-call-history" "No record target is currently active\\..*" "record function-call-history without target"
-gdb_test "record instruction-history" "No record target is currently active\\..*" "record instruction-history without target"
-gdb_test "info record" "No record target is currently active\\." "info record without target"
+gdb_test "record function-call-history" "No recording is currently active\\..*" "record function-call-history without target"
+gdb_test "record instruction-history" "No recording is currently active\\..*" "record instruction-history without target"
+gdb_test "info record" "No recording is currently active\\." "info record without target"
 
 standard_testfile
 if [prepare_for_testing "failed to prepare" $testfile {} {debug}] {
@@ -65,7 +65,7 @@ Recorded 0 instructions in 0 functions \\\(0 gaps\\\) for thread 1.*\\." "info r
 
 # stop btrace record
 gdb_test "record stop" "Process record is stopped and all execution logs are deleted\\."
-gdb_test "record stop" "No record target is currently active\\..*" "record stop the second time"
+gdb_test "record stop" "No recording is currently active\\..*" "record stop the second time"
 
 # enable btrace again
 gdb_test_no_output "record btrace" "record btrace re-enable"
diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
index fdf889f5ed8..23bc852c565 100644
--- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
+++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
@@ -55,7 +55,7 @@ with_test_prefix "inferior 2" {
 with_test_prefix "inferior 1" {
     gdb_test "inferior 1" "Switching to inferior 1.*"
 
-    gdb_test "info record" "No record target is currently active\\."
+    gdb_test "info record" "No recording is currently active\\."
     gdb_test_no_output "record btrace" "record btrace"
 }
 
@@ -69,6 +69,6 @@ with_test_prefix "inferior 3" {
 	return -1
     }
 
-    gdb_test "info record" "No record target is currently active\\."
+    gdb_test "info record" "No recording is currently active\\."
     gdb_test_no_output "record btrace" "record btrace"
 }
diff --git a/gdb/testsuite/gdb.btrace/reconnect.exp b/gdb/testsuite/gdb.btrace/reconnect.exp
index 26d04c38a9e..baec86f8aa3 100644
--- a/gdb/testsuite/gdb.btrace/reconnect.exp
+++ b/gdb/testsuite/gdb.btrace/reconnect.exp
@@ -81,5 +81,5 @@ with_test_prefix "second" {
 
 # Test that recording is now off.
 with_test_prefix "third" {
-  gdb_test "info record" "No record target is currently active."
+  gdb_test "info record" "No recording is currently active."
 }
diff --git a/gdb/testsuite/gdb.python/py-record-btrace.exp b/gdb/testsuite/gdb.python/py-record-btrace.exp
index bdfb2e3b125..3a84cbe7ac4 100644
--- a/gdb/testsuite/gdb.python/py-record-btrace.exp
+++ b/gdb/testsuite/gdb.python/py-record-btrace.exp
@@ -47,7 +47,7 @@ with_test_prefix "no or double record" {
     gdb_test "python gdb.start_recording(\"btrace\")" ".*gdb\.error: The process is already being recorded\..*"
 
     gdb_test_no_output "python gdb.stop_recording()" "first"
-    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No record target is currently active\..*" "second"
+    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No recording is currently active\..*" "second"
 }
 
 with_test_prefix "preopened record btrace" {
diff --git a/gdb/testsuite/gdb.python/py-record-full.exp b/gdb/testsuite/gdb.python/py-record-full.exp
index c823bc2c74c..a1f24a3634f 100644
--- a/gdb/testsuite/gdb.python/py-record-full.exp
+++ b/gdb/testsuite/gdb.python/py-record-full.exp
@@ -47,7 +47,7 @@ with_test_prefix "no or double record" {
     gdb_test "python gdb.start_recording(\"full\")" ".*gdb\.error: The process is already being recorded\..*"
 
     gdb_test_no_output "python gdb.stop_recording()" "first"
-    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No record target is currently active\..*" "second"
+    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No recording is currently active\..*" "second"
 }
 
 with_test_prefix "preopened record full" {
-- 
2.26.2

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 1/4] testsuite, gdb.btrace: make test names unique

[Markus Metzger via Gdb-patches]

gdb/testsuite/ChangeLog:
2020-12-04  Markus Metzger  <markus.t.metzger@intel.com>

	* gdb.btrace/data.exp: Make test names unique.
	* gdb.btrace/delta.exp: Likewise.
	* gdb.btrace/enable.exp: Likewise.
	* gdb.btrace/function_call_history.exp: Likewise.
	* gdb.btrace/nohist.exp: Likewise.
	* gdb.btrace/non-stop.exp: Likewise.
	* gdb.btrace/rn-dl-bind.exp: Likewise.
	* gdb.btrace/step.exp: Likewise.
	* gdb.btrace/stepi.exp: Likewise.
	* gdb.btrace/tailcall.exp: Likewise.
---
 gdb/testsuite/gdb.btrace/data.exp             |  24 ++-
 gdb/testsuite/gdb.btrace/delta.exp            |  30 ++--
 gdb/testsuite/gdb.btrace/enable.exp           |   2 +-
 .../gdb.btrace/function_call_history.exp      |   2 +-
 gdb/testsuite/gdb.btrace/nohist.exp           |   8 +-
 gdb/testsuite/gdb.btrace/non-stop.exp         |  72 +++++----
 gdb/testsuite/gdb.btrace/rn-dl-bind.exp       |  14 +-
 gdb/testsuite/gdb.btrace/step.exp             |  26 ++--
 gdb/testsuite/gdb.btrace/stepi.exp            | 146 +++++++++++-------
 gdb/testsuite/gdb.btrace/tailcall.exp         |  30 ++--
 10 files changed, 215 insertions(+), 139 deletions(-)

diff --git a/gdb/testsuite/gdb.btrace/data.exp b/gdb/testsuite/gdb.btrace/data.exp
index b10970f6760..28ac1ad46ad 100644
--- a/gdb/testsuite/gdb.btrace/data.exp
+++ b/gdb/testsuite/gdb.btrace/data.exp
@@ -40,16 +40,24 @@ gdb_test "next" ".*main\.3.*"
 gdb_test "reverse-step" ".*test\.4.*"
 
 # we can't read memory while we're replaying
-gdb_test "print glob" "unavailable\[^\\\r\\\n\]*"
-gdb_test "print loc" "unavailable\[^\\\r\\\n\]*"
+with_test_prefix "replay" {
+    with_test_prefix "default" {
+	gdb_test "print glob" "unavailable\[^\\\r\\\n\]*"
+	gdb_test "print loc" "unavailable\[^\\\r\\\n\]*"
+    }
 
-# we can read memory if we explicitly allow it.
-gdb_test_no_output "set record btrace replay-memory-access read-write"
-gdb_test "print glob" "1"
+    # we can read memory if we explicitly allow it.
+    with_test_prefix "read-write" {
+	gdb_test_no_output "set record btrace replay-memory-access read-write"
+	gdb_test "print glob" "1"
+    }
 
-# we can't if we don't explicitly allow it.
-gdb_test_no_output "set record btrace replay-memory-access read-only"
-gdb_test "print glob" "unavailable\[^\\\r\\\n\]*"
+    # we can't if we don't explicitly allow it.
+    with_test_prefix "read-only" {
+	gdb_test_no_output "set record btrace replay-memory-access read-only"
+	gdb_test "print glob" "unavailable\[^\\\r\\\n\]*"
+    }
+}
 
 # stop replaying and try again
 gdb_test "record goto end" ".*main\.3.*"
diff --git a/gdb/testsuite/gdb.btrace/delta.exp b/gdb/testsuite/gdb.btrace/delta.exp
index 0fe0e99f9ac..54891214653 100644
--- a/gdb/testsuite/gdb.btrace/delta.exp
+++ b/gdb/testsuite/gdb.btrace/delta.exp
@@ -73,18 +73,22 @@ with_test_prefix "twice" {
 }
 
 # check that we can reverse-stepi that instruction
-gdb_test "reverse-stepi"
-gdb_test "info record" [multi_line \
-  "Active record target: record-btrace" \
-  "Recording format: .*" \
-  "Recorded 1 instructions in 1 functions \\\(0 gaps\\\) for .*" \
-  "Replay in progress\.  At instruction 1\." \
-  ] "reverse-stepi"
+with_test_prefix "reverse" {
+    gdb_test "reverse-stepi"
+    gdb_test "info record" [multi_line \
+      "Active record target: record-btrace" \
+      "Recording format: .*" \
+      "Recorded 1 instructions in 1 functions \\\(0 gaps\\\) for .*" \
+      "Replay in progress\.  At instruction 1\." \
+    ]
+}
 
 # and back
-gdb_test "stepi"
-gdb_test "info record" [multi_line \
-  "Active record target: record-btrace" \
-  "Recording format: .*" \
-  "Recorded 1 instructions in 1 functions \\\(0 gaps\\\) for .*" \
-  ] "and back"
+with_test_prefix "forward" {
+    gdb_test "stepi"
+    gdb_test "info record" [multi_line \
+      "Active record target: record-btrace" \
+      "Recording format: .*" \
+      "Recorded 1 instructions in 1 functions \\\(0 gaps\\\) for .*" \
+    ]
+}
diff --git a/gdb/testsuite/gdb.btrace/enable.exp b/gdb/testsuite/gdb.btrace/enable.exp
index 8ce1dab076f..d147e90874a 100644
--- a/gdb/testsuite/gdb.btrace/enable.exp
+++ b/gdb/testsuite/gdb.btrace/enable.exp
@@ -100,7 +100,7 @@ if ![runto_main] {
     untested "failed to run to main"
     return -1
 }
-gdb_test_no_output "record btrace"
+gdb_test_no_output "record btrace" "enable after restart"
 if ![runto_main] {
     untested "failed to run to main"
     return -1
diff --git a/gdb/testsuite/gdb.btrace/function_call_history.exp b/gdb/testsuite/gdb.btrace/function_call_history.exp
index bc28ec7cccd..51536ff6035 100644
--- a/gdb/testsuite/gdb.btrace/function_call_history.exp
+++ b/gdb/testsuite/gdb.btrace/function_call_history.exp
@@ -240,7 +240,7 @@ gdb_continue_to_breakpoint "cont to fib.3"
 gdb_continue_to_breakpoint "cont to fib.4"
 
 # start tracing
-gdb_test_no_output "record btrace"
+gdb_test_no_output "record btrace" "start recording after rerun"
 
 # continue until line 30 above
 delete_breakpoints
diff --git a/gdb/testsuite/gdb.btrace/nohist.exp b/gdb/testsuite/gdb.btrace/nohist.exp
index 0273ceef815..90883c06973 100644
--- a/gdb/testsuite/gdb.btrace/nohist.exp
+++ b/gdb/testsuite/gdb.btrace/nohist.exp
@@ -42,8 +42,12 @@ proc check_not_replaying {} {
 
 gdb_test_no_output "record btrace"
 
-check_not_replaying
+with_test_prefix "forward" {
+    check_not_replaying
+}
 
 gdb_test "reverse-continue" "No more reverse-execution history\.\r\n.*"
 
-check_not_replaying
+with_test_prefix "backward" {
+    check_not_replaying
+}
diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp b/gdb/testsuite/gdb.btrace/non-stop.exp
index 1637fab888a..f3bf005f9eb 100644
--- a/gdb/testsuite/gdb.btrace/non-stop.exp
+++ b/gdb/testsuite/gdb.btrace/non-stop.exp
@@ -52,7 +52,7 @@ set loop_line "$loop\[^\\\r\\\n\]*/\\\* loop \\\*/"
 
 # make sure $line matches the full expected output per thread.
 # and let's hope that GDB never mixes the output from different threads.
-proc gdb_cont_to { threads cmd line nthreads } {
+proc gdb_cont_to { threads cmd line nthreads message } {
     global gdb_prompt
     set full_cmd "thread apply $threads $cmd"
 
@@ -60,13 +60,13 @@ proc gdb_cont_to { threads cmd line nthreads } {
     # the prompt precedes any further output except some errors.
     gdb_test_multiple "$full_cmd &" "$full_cmd: prompt" {
         -re "$gdb_prompt " {
-            pass "$full_cmd: prompt"
+            pass "$full_cmd ($message): prompt"
         }
     }
 
     # now check for the expected line - one per thread.
     for {set i 0} {$i < $nthreads} {incr i} {
-        set test "$full_cmd: thread $i"
+        set test "$full_cmd ($message): thread $i"
 
         gdb_test_multiple "" $test {
             -re "$line\[^\\\r\\\n\]*\r\n" {
@@ -82,7 +82,7 @@ proc gdb_cont_to_bp_line { line threads nthreads } {
              "Breakpoint\[^\\\r\\\n\]*$line" \
              "\[^\\\r\\\n\]*" \
             ] \
-        $nthreads
+        $nthreads "to $line"
 }
 
 proc gdb_cont_to_no_history { threads cmd nthreads } {
@@ -92,7 +92,7 @@ proc gdb_cont_to_no_history { threads cmd nthreads } {
              "\[^\\\r\\\n\]*" \
              "\[^\\\r\\\n\]*" \
             ] \
-        $nthreads
+        $nthreads "to end of history"
 }
 
 # trace the code between the two breakpoints
@@ -112,15 +112,15 @@ with_test_prefix "navigate" {
     gdb_test "thread apply 1 record goto 2" "$loop_line"
     gdb_test "thread apply 2 record goto 4" "$loop_line"
     gdb_test "thread apply 1 info record" \
-        ".*Replay in progress\.  At instruction 2\."
+        ".*Replay in progress\.  At instruction 2\." "thread 1 at insn 2"
     gdb_test "thread apply 2 info record" \
-        ".*Replay in progress\.  At instruction 4\."
+        ".*Replay in progress\.  At instruction 4\." "thread 2 at insn 4"
 
     gdb_test "thread apply all record goto 5" "$loop_line"
     gdb_test "thread apply 1 info record" \
-        ".*Replay in progress\.  At instruction 5\."
+        ".*Replay in progress\.  At instruction 5\." "thread 1 at insn 5"
     gdb_test "thread apply 2 info record" \
-        ".*Replay in progress\.  At instruction 5\."
+        ".*Replay in progress\.  At instruction 5\." "thread 2 at insn 5"
 }
 
 with_test_prefix "step" {
@@ -141,7 +141,7 @@ with_test_prefix "step" {
     }
 
     with_test_prefix "all" {
-        gdb_cont_to all "stepi 4" "$loop_line" 2
+        gdb_cont_to all "stepi 4" "$loop_line" 2 "to $srcfile:$loop"
         gdb_test "thread apply 1 info record" \
             ".*Replay in progress\.  At instruction 11\."
         gdb_test "thread apply 2 info record" \
@@ -167,7 +167,7 @@ with_test_prefix "reverse-step" {
     }
 
     with_test_prefix "all" {
-        gdb_cont_to all "reverse-stepi 4" "$loop_line" 2
+        gdb_cont_to all "reverse-stepi 4" "$loop_line" 2 "to $srcfile:$loop"
         gdb_test "thread apply 1 info record" \
             ".*Replay in progress\.  At instruction 5\."
         gdb_test "thread apply 2 info record" \
@@ -177,31 +177,37 @@ with_test_prefix "reverse-step" {
 
 with_test_prefix "continue" {
     with_test_prefix "thread 1" {
-        gdb_cont_to_no_history 1 "continue" 1
-        gdb_test "thread apply 1 info record" \
-            ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*"
-        gdb_test "thread apply 2 info record" \
-            ".*Replay in progress\.  At instruction 5\."
-
-        gdb_cont_to_no_history 1 "reverse-continue" 1
-        gdb_test "thread apply 1 info record" \
-            ".*Replay in progress\.  At instruction 1\."
-        gdb_test "thread apply 2 info record" \
-            ".*Replay in progress\.  At instruction 5\."
+	with_test_prefix "continue" {
+	    gdb_cont_to_no_history 1 "continue" 1
+	    gdb_test "thread apply 1 info record" \
+		".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*"
+	    gdb_test "thread apply 2 info record" \
+		".*Replay in progress\.  At instruction 5\."
+	}
+	with_test_prefix "reverse-continue" {
+	    gdb_cont_to_no_history 1 "reverse-continue" 1
+	    gdb_test "thread apply 1 info record" \
+		".*Replay in progress\.  At instruction 1\."
+	    gdb_test "thread apply 2 info record" \
+		".*Replay in progress\.  At instruction 5\."
+	}
     }
 
     with_test_prefix "thread 2" {
-        gdb_cont_to_no_history 2 "continue" 1
-        gdb_test "thread apply 1 info record" \
-            ".*Replay in progress\.  At instruction 1\."
-        gdb_test "thread apply 2 info record" \
-            ".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*"
-
-        gdb_cont_to_no_history 2 "reverse-continue" 1
-        gdb_test "thread apply 1 info record" \
-            ".*Replay in progress\.  At instruction 1\."
-        gdb_test "thread apply 2 info record" \
-            ".*Replay in progress\.  At instruction 1\."
+	with_test_prefix "continue" {
+	    gdb_cont_to_no_history 2 "continue" 1
+	    gdb_test "thread apply 1 info record" \
+		".*Replay in progress\.  At instruction 1\."
+	    gdb_test "thread apply 2 info record" \
+		".*Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*"
+	}
+	with_test_prefix "reverse-continue" {
+	    gdb_cont_to_no_history 2 "reverse-continue" 1
+	    gdb_test "thread apply 1 info record" \
+		".*Replay in progress\.  At instruction 1\."
+	    gdb_test "thread apply 2 info record" \
+		".*Replay in progress\.  At instruction 1\."
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.btrace/rn-dl-bind.exp b/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
index ab2dda413ef..46804864026 100644
--- a/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
+++ b/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
@@ -38,18 +38,18 @@ if ![runto_main] {
 
 # trace the code for the call to test
 gdb_test_no_output "record btrace"
-gdb_test "next" ".*main\.2.*"
+gdb_test "next" ".*main\.2.*" "next.1"
 
 # just dump the function-call-history to help debugging
 gdb_test_no_output "set record function-call-history-size 0"
 gdb_test "record function-call-history /cli 1" ".*"
 
 # check that we can reverse-next and next
-gdb_test "reverse-next" ".*main\.1.*"
-gdb_test "next" ".*main\.2.*"
+gdb_test "reverse-next" ".*main\.1.*" "reverse-next.1"
+gdb_test "next" ".*main\.2.*" "next.2"
 
 # now go into test and try to reverse-next and next over the library call
-gdb_test "reverse-step" ".*test\.3.*"
-gdb_test "reverse-step" ".*test\.2.*"
-gdb_test "reverse-next" ".*test\.1.*"
-gdb_test "next" ".*test\.2.*"
+gdb_test "reverse-step" ".*test\.3.*" "reverse-step.1"
+gdb_test "reverse-step" ".*test\.2.*" "reverse-step.2"
+gdb_test "reverse-next" ".*test\.1.*" "reverse-next.2"
+gdb_test "next" ".*test\.2.*" "next.3"
diff --git a/gdb/testsuite/gdb.btrace/step.exp b/gdb/testsuite/gdb.btrace/step.exp
index 19850256590..a71c46dde6b 100644
--- a/gdb/testsuite/gdb.btrace/step.exp
+++ b/gdb/testsuite/gdb.btrace/step.exp
@@ -33,16 +33,20 @@ if ![runto_main] {
 }
 
 # trace the call to the test function
-gdb_test_no_output "record btrace"
-gdb_test "next"
+with_test_prefix "record" {
+    gdb_test_no_output "record btrace"
+    gdb_test "next"
+}
 
 # let's step around a bit
-gdb_test "reverse-next" ".*main\.2.*"
-gdb_test "step" ".*fun4\.2.*"
-gdb_test "next" ".*fun4\.3.*"
-gdb_test "step" ".*fun2\.2.*"
-gdb_test "finish" ".*fun4\.4.*"
-gdb_test "reverse-step" ".*fun2\.3.*"
-gdb_test "reverse-finish" ".*fun4\.3.*"
-gdb_test "reverse-next" ".*fun4\.2.*"
-gdb_test "reverse-finish" ".*main\.2.*"
+with_test_prefix "replay" {
+    gdb_test "reverse-next" ".*main\.2.*" "reverse-next.1"
+    gdb_test "step" ".*fun4\.2.*" "step.1"
+    gdb_test "next" ".*fun4\.3.*" "next.1"
+    gdb_test "step" ".*fun2\.2.*" "step.2"
+    gdb_test "finish" ".*fun4\.4.*" "finish.1"
+    gdb_test "reverse-step" ".*fun2\.3.*" "reverse-step.1"
+    gdb_test "reverse-finish" ".*fun4\.3.*" "reverse-finish.1"
+    gdb_test "reverse-next" ".*fun4\.2.*" "reverse-next.2"
+    gdb_test "reverse-finish" ".*main\.2.*" "reverse-finish.2"
+}
diff --git a/gdb/testsuite/gdb.btrace/stepi.exp b/gdb/testsuite/gdb.btrace/stepi.exp
index 1ab1b9adfa1..4e2bf1a8e81 100644
--- a/gdb/testsuite/gdb.btrace/stepi.exp
+++ b/gdb/testsuite/gdb.btrace/stepi.exp
@@ -61,82 +61,122 @@ proc check_replay_at { insn } {
     ".*" \
     "Recorded 40 instructions in 16 functions \\\(0 gaps\\\) for .*" \
     "Replay in progress\.  At instruction $insn\." \
-    ]
+    ] "check replay at $insn"
 }
 
 # trace the call to the test function
-gdb_test_no_output "record btrace"
-gdb_test "next"
+with_test_prefix "record" {
+    gdb_test_no_output "record btrace"
+    gdb_test "next" ".*" "next.1"
+}
 
 # we start with stepping to make sure that the trace is fetched automatically
-gdb_test "reverse-stepi" ".*fun4\.5.*"
-gdb_test "reverse-stepi" ".*fun4\.5.*"
+with_test_prefix "fetch" {
+    gdb_test "reverse-stepi" ".*fun4\.5.*" "reverse-stepi.1"
+    gdb_test "reverse-stepi" ".*fun4\.5.*" "reverse-stepi.2"
 
-# let's check where we are in the trace
-with_test_prefix "reverse-stepi to 39" { check_replay_at 39 }
+    # let's check where we are in the trace
+    check_replay_at 39
+}
 
 # let's step forward and check again
-gdb_test "stepi" ".*fun4\.5.*"
-with_test_prefix "stepi to 40" { check_replay_at 40 }
+with_test_prefix "stepi" {
+    gdb_test "stepi" ".*fun4\.5.*"
+    check_replay_at 40
+}
 
 # with the next step, we stop replaying
-gdb_test "stepi" ".*main\.3.*"
-gdb_test "info record" [multi_line \
-  "Active record target: record-btrace" \
-  ".*" \
-  "Recorded 40 instructions in 16 functions \\\(0 gaps\\\) for \[^\\\r\\\n\]*" \
-  ] "stepi to live"
+with_test_prefix "end" {
+    gdb_test "stepi" ".*main\.3.*"
+    gdb_test "info record" [multi_line \
+      "Active record target: record-btrace" \
+      ".*" \
+      "Recorded 40 instructions in 16 functions \\\(0 gaps\\\) for \[^\\\r\\\n\]*" \
+			       ]
+}
 
 # let's try nexti
-gdb_test "reverse-nexti" ".*main\.2.*"
-with_test_prefix "reverse-nexti - 1" { check_replay_at 1 }
+with_test_prefix "reverse-nexti.1" {
+    gdb_test "reverse-nexti" ".*main\.2.*"
+    check_replay_at 1
+}
 
 # we can't reverse-nexti any further
-gdb_test "reverse-nexti" "No more reverse-execution history\.\r\n.*main\.2.*"
-with_test_prefix "reverse-nexti - 1" { check_replay_at 1 }
+with_test_prefix "reverse-nexti.2" {
+    gdb_test "reverse-nexti" \
+	"No more reverse-execution history\.\r\n.*main\.2.*" \
+	"reverse-nexti.2"
+    check_replay_at 1
+}
 
 # but we can step back again
-gdb_test "nexti" ".*main\.3.*" "next, 1.5"
-gdb_test "info record" [multi_line \
-  "Active record target: record-btrace" \
-  ".*" \
-  "Recorded 40 instructions in 16 functions \\\(0 gaps\\\) for \[^\\\r\\\n\]*" \
-  ] "nexti back"
+with_test_prefix "nexti" {
+    gdb_test "nexti" ".*main\.3.*"
+    gdb_test "info record" [multi_line \
+      "Active record target: record-btrace" \
+      ".*" \
+      "Recorded 40 instructions in 16 functions \\\(0 gaps\\\) for \[^\\\r\\\n\]*" \
+			       ]
+}
 
 # let's step from a goto position somewhere in the middle
-gdb_test "record goto 22" ".*fun3\.2.*"
-with_test_prefix "goto 22" { check_replay_at 22 }
+with_test_prefix "goto" {
+    gdb_test "record goto 22" ".*fun3\.2.*"
+    with_test_prefix "goto 22" { check_replay_at 22 }
 
-gdb_test "stepi" ".*fun1\.1.*"
-with_test_prefix "stepi to 23" { check_replay_at 23 }
+    gdb_test "stepi" ".*fun1\.1.*" "stepi.3"
+    with_test_prefix "stepi to 23" { check_replay_at 23 }
 
-gdb_test "reverse-stepi" ".*fun3\.2.*"
-with_test_prefix "reverse-stepi to 22" { check_replay_at 22 }
+    gdb_test "reverse-stepi" ".*fun3\.2.*" "reverse-stepi.3"
+    with_test_prefix "reverse-stepi to 22" { check_replay_at 22 }
 
-gdb_test "nexti" ".*fun3\.3.*"
-with_test_prefix "nexti to 27" { check_replay_at 27 }
+    gdb_test "nexti" ".*fun3\.3.*"
+    with_test_prefix "nexti to 27" { check_replay_at 27 }
 
-gdb_test "reverse-nexti" ".*fun3\.2.*"
-with_test_prefix "reverse-nexti to 22" { check_replay_at 22 }
+    gdb_test "reverse-nexti" ".*fun3\.2.*" "reverse-nexti.3"
+    with_test_prefix "reverse-nexti to 22" { check_replay_at 22 }
+}
 
 # let's try to step off the left end
-gdb_test "record goto begin" ".*main\.2.*"
-with_test_prefix "goto begin" { check_replay_at 1 }
-
-gdb_test "reverse-stepi" "No more reverse-execution history\.\r\n.*main\.2.*"
-gdb_test "reverse-stepi" "No more reverse-execution history\.\r\n.*main\.2.*"
-with_test_prefix "reverse-stepi at begin" { check_replay_at 1 }
-
-gdb_test "reverse-nexti" "No more reverse-execution history\.\r\n.*main\.2.*"
-gdb_test "reverse-nexti" "No more reverse-execution history\.\r\n.*main\.2.*"
-with_test_prefix "reverse-nexti at begin" { check_replay_at 1 }
-
-# we can step forward, though
-gdb_test "stepi" ".*fun4\.1.*"
-with_test_prefix "stepi to 2" { check_replay_at 2 }
+with_test_prefix "goto begin" {
+    gdb_test "record goto begin" ".*main\.2.*"
+    check_replay_at 1
+
+    with_test_prefix "reverse-stepi" {
+	gdb_test "reverse-stepi" \
+	    "No more reverse-execution history\.\r\n.*main\.2.*" \
+	    "reverse-stepi.1"
+	gdb_test "reverse-stepi" \
+	    "No more reverse-execution history\.\r\n.*main\.2.*" \
+	    "reverse-stepi.2"
+	check_replay_at 1
+    }
+
+    with_test_prefix "reverse-nexti" {
+	gdb_test "reverse-nexti" \
+	    "No more reverse-execution history\.\r\n.*main\.2.*" \
+	    "reverse-nexti.1"
+	gdb_test "reverse-nexti" \
+	    "No more reverse-execution history\.\r\n.*main\.2.*" \
+	    "reverse-nexti.2"
+	check_replay_at 1
+    }
+
+    # we can step forward, though
+    with_test_prefix "stepi" {
+	gdb_test "stepi" ".*fun4\.1.*"
+	check_replay_at 2
+    }
+}
 
 # let's try to step off the left end again
-gdb_test "reverse-stepi" ".*main\.2.*"
-gdb_test "reverse-stepi" "No more reverse-execution history\.\r\n.*main\.2.*"
-gdb_test "reverse-stepi" "No more reverse-execution history\.\r\n.*main\.2.*"
-with_test_prefix "reverse-stepi at begin" { check_replay_at 1 }
+with_test_prefix "reverse-stepi" {
+    gdb_test "reverse-stepi" ".*main\.2.*" "reverse-stepi.1"
+    gdb_test "reverse-stepi" \
+	"No more reverse-execution history\.\r\n.*main\.2.*" \
+	"reverse-stepi.2"
+    gdb_test "reverse-stepi" \
+	"No more reverse-execution history\.\r\n.*main\.2.*" \
+	"reverse-stepi.3"
+    check_replay_at 1
+}
diff --git a/gdb/testsuite/gdb.btrace/tailcall.exp b/gdb/testsuite/gdb.btrace/tailcall.exp
index 2937a3b4fe9..4f86a6f2088 100644
--- a/gdb/testsuite/gdb.btrace/tailcall.exp
+++ b/gdb/testsuite/gdb.btrace/tailcall.exp
@@ -93,13 +93,23 @@ gdb_test "up" "#2\[^\r\n\]*main \\(\\) at tailcall.c:37\r\n.*" "up to main"
 gdb_test "down" "#1\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*" "down to foo"
 
 # test stepping into and out of tailcalls.
-gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*"
-gdb_test "reverse-step" "\[^\r\n\]*bar \\(\\) at tailcall.c:24\r\n.*"
-gdb_test "reverse-finish" "\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*"
-gdb_test "reverse-step" "\[^\r\n\]*main \\(\\) at tailcall.c:37\r\n.*"
-gdb_test "next" "\[^\r\n\]*38.*"
-gdb_test "reverse-next" "\[^\r\n\]*main \\(\\) at tailcall.c:37\r\n.*"
-gdb_test "step" "\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*"
-gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*"
-gdb_test "reverse-step" "\[^\r\n\]*bar \\(\\) at tailcall.c:24\r\n.*"
-gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*"
+gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*" \
+    "finish.1"
+gdb_test "reverse-step" "\[^\r\n\]*bar \\(\\) at tailcall.c:24\r\n.*" \
+    "reverse-step.1"
+gdb_test "reverse-finish" "\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*" \
+    "reverse-finish.1"
+gdb_test "reverse-step" "\[^\r\n\]*main \\(\\) at tailcall.c:37\r\n.*" \
+    "reverse-step.2"
+gdb_test "next" "\[^\r\n\]*38.*" \
+    "next.1"
+gdb_test "reverse-next" "\[^\r\n\]*main \\(\\) at tailcall.c:37\r\n.*" \
+    "reverse-next.1"
+gdb_test "step" "\[^\r\n\]*foo \\(\\) at tailcall.c:29\r\n.*" \
+    "step.1"
+gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*" \
+    "finish.2"
+gdb_test "reverse-step" "\[^\r\n\]*bar \\(\\) at tailcall.c:24\r\n.*" \
+    "reverse-step.3"
+gdb_test "finish" "\[^\r\n\]*main \\(\\) at tailcall.c:38\r\n.*" \
+    "finish.3"
-- 
2.26.2

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 2/4] testsuite, gdb.python: make py-record-*.exp test names unique

[Markus Metzger via Gdb-patches]

gdb/testsuite/ChangeLog:
2020-12-14  Markus Metzger  <markus.t.metzger@intel.com>

	* gdb.python/py-record-btrace.exp: Make test names unique.
	* gdb.python/py-record-full.exp: Likewise.
---
 gdb/testsuite/gdb.python/py-record-btrace.exp | 7 +++++--
 gdb/testsuite/gdb.python/py-record-full.exp   | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.python/py-record-btrace.exp b/gdb/testsuite/gdb.python/py-record-btrace.exp
index 3a84cbe7ac4..d113ed640fd 100644
--- a/gdb/testsuite/gdb.python/py-record-btrace.exp
+++ b/gdb/testsuite/gdb.python/py-record-btrace.exp
@@ -44,10 +44,13 @@ with_test_prefix "no or double record" {
     gdb_test "python print(gdb.current_recording())" "None"
 
     gdb_test_no_output "python gdb.start_recording(\"btrace\")"
-    gdb_test "python gdb.start_recording(\"btrace\")" ".*gdb\.error: The process is already being recorded\..*"
+    gdb_test "python gdb.start_recording(\"btrace\")" \
+	".*gdb\.error: The process is already being recorded\..*" \
+	"already recording"
 
     gdb_test_no_output "python gdb.stop_recording()" "first"
-    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No recording is currently active\..*" "second"
+    gdb_test "python gdb.stop_recording()" \
+	".*gdb\.error: No recording is currently active\..*" "second"
 }
 
 with_test_prefix "preopened record btrace" {
diff --git a/gdb/testsuite/gdb.python/py-record-full.exp b/gdb/testsuite/gdb.python/py-record-full.exp
index a1f24a3634f..0b67788d4fd 100644
--- a/gdb/testsuite/gdb.python/py-record-full.exp
+++ b/gdb/testsuite/gdb.python/py-record-full.exp
@@ -44,10 +44,13 @@ with_test_prefix "no or double record" {
     gdb_test "python print(gdb.current_recording())" "None"
 
     gdb_test_no_output "python gdb.start_recording(\"full\")"
-    gdb_test "python gdb.start_recording(\"full\")" ".*gdb\.error: The process is already being recorded\..*"
+    gdb_test "python gdb.start_recording(\"full\")" \
+	".*gdb\.error: The process is already being recorded\..*" \
+	"already recording"
 
     gdb_test_no_output "python gdb.stop_recording()" "first"
-    gdb_test "python gdb.stop_recording()" ".*gdb\.error: No recording is currently active\..*" "second"
+    gdb_test "python gdb.stop_recording()" \
+	".*gdb\.error: No recording is currently active\..*" "second"
 }
 
 with_test_prefix "preopened record full" {
-- 
2.26.2

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 3/4] testsuite, gdb.btrace: skip multi-inferior.exp on gdbserver targets

[Markus Metzger via Gdb-patches]

The gdb.btrace/multi-inferior.exp test creates multiple inferiors to check
that recording is per-inferior.

When run with the native-gdbserver board, this test hangs when trying to
add the second inferior over the remote connection.  Skip the test.

Note that the test runs fine with the native-extended-gdbserver board file
and we want to keep testing that configuration.

gdb/testsuite/ChangeLog:
2020-12-11  Markus Metzger  <markus.t.metzger@intel.com>

	* gdb.btrace/multi-inferior.exp: Skip for gdbserver targets.
---
 gdb/testsuite/gdb.btrace/multi-inferior.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
index 23bc852c565..015c46634e5 100644
--- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
+++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
@@ -27,6 +27,13 @@ if { [skip_btrace_tests] } {
     return -1
 }
 
+# This test creates multiple inferiors.  This won't work with all
+# gdbserver board files.
+if { [use_gdb_stub] } {
+    unsupported "test creates multiple inferiors"
+    return -1
+}
+
 standard_testfile
 if [prepare_for_testing "failed to prepare" $testfile {} {debug}] {
     return -1
-- 
2.26.2

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 4/4] testsuite, gdb.btrace: fix 32-bit PIE false positives

[Markus Metzger via Gdb-patches]

For 32-bit position independent executables, GCC generates an extra call to

    __x86.get_pc_thunk.<reg>

which appears in the function call history.  It is correct to appear there
but this confuses the tests, which check for an expected sequence of
functions.

Build with -no-pie -fno-pie to avoid this complication.

gdb/testsuite/ChangeLog:
2020-12-04  Markus Metzger  <markus.t.metzger@intel.com>

	* gdb.btrace/exception.exp: Build with -no-pie -fno-pie.
	* gdb.btrace/function_call_history.exp: Likewise.
	* gdb.btrace/unknown_functions.exp: Likewise.
---
 gdb/testsuite/gdb.btrace/exception.exp             | 5 ++++-
 gdb/testsuite/gdb.btrace/function_call_history.exp | 5 ++++-
 gdb/testsuite/gdb.btrace/unknown_functions.exp     | 9 ++++++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.btrace/exception.exp b/gdb/testsuite/gdb.btrace/exception.exp
index 0d242227938..b515de87b01 100755
--- a/gdb/testsuite/gdb.btrace/exception.exp
+++ b/gdb/testsuite/gdb.btrace/exception.exp
@@ -22,8 +22,11 @@ if { [skip_btrace_tests] } {
     return -1
 }
 
+# We expect a specific function call history.  This gets messed up with
+# PIE on 32-bit.
 standard_testfile exception.cc
-if [prepare_for_testing "failed to prepare" $testfile $srcfile {c++ debug}] {
+if [prepare_for_testing "failed to prepare" $testfile $srcfile \
+	{additional_flags=-fno-pie ldflags=-no-pie c++ debug}] {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.btrace/function_call_history.exp b/gdb/testsuite/gdb.btrace/function_call_history.exp
index 51536ff6035..ea22692bb47 100644
--- a/gdb/testsuite/gdb.btrace/function_call_history.exp
+++ b/gdb/testsuite/gdb.btrace/function_call_history.exp
@@ -22,8 +22,11 @@ if { [skip_btrace_tests] } {
     return -1
 }
 
+# We expect a specific function call history.  This gets messed up with
+# PIE on 32-bit.
 standard_testfile
-if [prepare_for_testing "failed to prepare" $testfile {} {debug}] {
+if [prepare_for_testing "failed to prepare" $testfile {} \
+	{additional_flags=-fno-pie ldflags=-no-pie debug}] {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.btrace/unknown_functions.exp b/gdb/testsuite/gdb.btrace/unknown_functions.exp
index 4db9c70f496..76803319d9f 100644
--- a/gdb/testsuite/gdb.btrace/unknown_functions.exp
+++ b/gdb/testsuite/gdb.btrace/unknown_functions.exp
@@ -24,9 +24,12 @@ if { [skip_btrace_tests] } {
 
 standard_testfile
 
-# discard local symbols
-set ldflags "additional_flags=-Wl,-x"
-if [prepare_for_testing "failed to prepare" $testfile $srcfile $ldflags] {
+# We expect a specific function call history.  This gets messed up with
+# PIE on 32-bit.
+#
+# Also discard local symbols.
+if [prepare_for_testing "failed to prepare" $testfile $srcfile \
+	{additional_flags=-fno-pie additional_flags=-Wl,-x ldflags=-no-pie}] {
     return -1
 }
 
-- 
2.26.2

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] gdb, record: rephrase the 'not recording' error message

[Simon Marchi via Gdb-patches]

On 2020-12-17 10:12 a.m., Markus Metzger via Gdb-patches wrote:
> When trying to use one of the record commands without having enabled
> recording first, GDB gives the error message:
> 
>     (gdb) record function-call-history
>     No record target is currently active.
>     Use one of the "target record-<TAB><TAB>" commands first.
> 
> In the record help, however, we say:
> 
>     (gdb) help record
>     record, rec
>     Start recording.
> 
>     List of record subcommands:
> 
>     record btrace, record b -- Start branch trace recording.
>     record delete, record del, record d -- Delete the rest of execution log and start recording it anew.
>     record full -- Start full execution recording.
>     record function-call-history -- Prints the execution history at function granularity.
>     record goto -- Restore the program to its state at instruction number N.
>     record instruction-history -- Print disassembled instructions stored in the execution log.
>     record save -- Save the execution log to a file.
>     record stop, record s -- Stop the record/replay target.
> 
> Change the above error message to
> 
>     (gdb) record function-call-history
>     No recording is currently active.
>     Use the "record full" or "record btrace" command first.
> 
> to align with the help text.
> 
> gdb/ChangeLog:
> 2020-12-03  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* record.c (require_record_target): Rephrase error message.
> 	(info_record_command): Likewise.
> 
> gdb/testsuite/ChangeLog:
> 2020-12-03  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* gdb.btrace/enable.exp: Update error message.
> 	* gdb.btrace/multi-inferior.exp: Likewise.
> 	* gdb.btrace/reconnect.exp: Likewise.
> 	* gdb.python/py-record-btrace.exp: Likewise.
> 	* gdb.python/py-record-full.exp: Likewise.

Thanks, that LGTM.

Simon

Re: [PATCH 1/4] testsuite, gdb.btrace: make test names unique

[Simon Marchi via Gdb-patches]

> @@ -60,13 +60,13 @@ proc gdb_cont_to { threads cmd line nthreads } {
>      # the prompt precedes any further output except some errors.
>      gdb_test_multiple "$full_cmd &" "$full_cmd: prompt" {
>          -re "$gdb_prompt " {
> -            pass "$full_cmd: prompt"
> +            pass "$full_cmd ($message): prompt"
>          }
>      }

It would be preferable to use

  gdb_test_multiple "$full_cmd &" "$full_cmd ($message): prompt" {

and $gdb_test_name inside the "-re" clause, to make sure the test name
is the same regardless of the outcome.

And just a nit, I would prefer if the message was used for a
"with_test_prefix" for the whole proc, instead of putting the context
in parenthesis.

Otherwise, that all looks good to me, thanks for doing this.

Simon

Re: [PATCH 2/4] testsuite, gdb.python: make py-record-*.exp test names unique

[Simon Marchi via Gdb-patches]

On 2020-12-17 10:12 a.m., Markus Metzger via Gdb-patches wrote:
> gdb/testsuite/ChangeLog:
> 2020-12-14  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* gdb.python/py-record-btrace.exp: Make test names unique.
> 	* gdb.python/py-record-full.exp: Likewise.

LGTM, thanks.

Simon

Re: [PATCH 3/4] testsuite, gdb.btrace: skip multi-inferior.exp on gdbserver targets

[Simon Marchi via Gdb-patches]

On 2020-12-17 10:12 a.m., Markus Metzger via Gdb-patches wrote:
> The gdb.btrace/multi-inferior.exp test creates multiple inferiors to check
> that recording is per-inferior.
> 
> When run with the native-gdbserver board, this test hangs when trying to
> add the second inferior over the remote connection.  Skip the test.
> 
> Note that the test runs fine with the native-extended-gdbserver board file
> and we want to keep testing that configuration.
> 
> gdb/testsuite/ChangeLog:
> 2020-12-11  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* gdb.btrace/multi-inferior.exp: Skip for gdbserver targets.
> ---
>  gdb/testsuite/gdb.btrace/multi-inferior.exp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
> index 23bc852c565..015c46634e5 100644
> --- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
> +++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
> @@ -27,6 +27,13 @@ if { [skip_btrace_tests] } {
>      return -1
>  }
>  
> +# This test creates multiple inferiors.  This won't work with all
> +# gdbserver board files.

The comment sounds a bit confusing.  It may be interpreted as "this test
doesn't work with any gdbserver board files", but you mean "this test works
with some gdbserver board files but not all".

I think just saying "Test test creates multiple inferiors." is enough to
explain the [use_gdb_stub] check.  Stub configurations can't spawn inferiors.

Although I think it wouldn't hurt to improve the documentation of the
use_gdb_stub procedure to explain what it means, as it's a pretty opaque
concept if you don't already know it.

Otherwise, the patch LGTM.

Simon

Re: [PATCH 4/4] testsuite, gdb.btrace: fix 32-bit PIE false positives

[Simon Marchi via Gdb-patches]

On 2020-12-17 10:12 a.m., Markus Metzger via Gdb-patches wrote:
> For 32-bit position independent executables, GCC generates an extra call to
> 
>     __x86.get_pc_thunk.<reg>
> 
> which appears in the function call history.  It is correct to appear there
> but this confuses the tests, which check for an expected sequence of
> functions.
> 
> Build with -no-pie -fno-pie to avoid this complication.
> 
> gdb/testsuite/ChangeLog:
> 2020-12-04  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* gdb.btrace/exception.exp: Build with -no-pie -fno-pie.
> 	* gdb.btrace/function_call_history.exp: Likewise.
> 	* gdb.btrace/unknown_functions.exp: Likewise.
> ---
>  gdb/testsuite/gdb.btrace/exception.exp             | 5 ++++-
>  gdb/testsuite/gdb.btrace/function_call_history.exp | 5 ++++-
>  gdb/testsuite/gdb.btrace/unknown_functions.exp     | 9 ++++++---
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.btrace/exception.exp b/gdb/testsuite/gdb.btrace/exception.exp
> index 0d242227938..b515de87b01 100755
> --- a/gdb/testsuite/gdb.btrace/exception.exp
> +++ b/gdb/testsuite/gdb.btrace/exception.exp
> @@ -22,8 +22,11 @@ if { [skip_btrace_tests] } {
>      return -1
>  }
>  
> +# We expect a specific function call history.  This gets messed up with
> +# PIE on 32-bit.
>  standard_testfile exception.cc
> -if [prepare_for_testing "failed to prepare" $testfile $srcfile {c++ debug}] {
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile \
> +	{additional_flags=-fno-pie ldflags=-no-pie c++ debug}] {
>      return -1
>  }

There's the "nopie" option you can pass to gdb_compile to disable PIE.  Although from
what I see, it doesn't add -fno-pie to compilation options, just adds -no-pie to link
options.  I suppose you need both?  If so, it could be preferable in the long run to
teach gdb_compile to pass -fno-pie to the compilation step as well.

Simon

RE: [PATCH 1/4] testsuite, gdb.btrace: make test names unique

[Metzger, Markus T via Gdb-patches]

Hello Simon,

Thanks for your review.

> > @@ -60,13 +60,13 @@ proc gdb_cont_to { threads cmd line nthreads } {
> >      # the prompt precedes any further output except some errors.
> >      gdb_test_multiple "$full_cmd &" "$full_cmd: prompt" {
> >          -re "$gdb_prompt " {
> > -            pass "$full_cmd: prompt"
> > +            pass "$full_cmd ($message): prompt"
> >          }
> >      }
> 
> It would be preferable to use
> 
>   gdb_test_multiple "$full_cmd &" "$full_cmd ($message): prompt" {
> 
> and $gdb_test_name inside the "-re" clause, to make sure the test name
> is the same regardless of the outcome.
> 
> And just a nit, I would prefer if the message was used for a
> "with_test_prefix" for the whole proc, instead of putting the context
> in parenthesis.

I changed gdb_cont_to to [1].  The rest of the patch already added
with_test_prefix to most users, which left only two for which I added them.

Thanks,
Markus.

[1]
---
diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp b/gdb/testsuite/gdb.btrace/non-stop.exp
index 1637fab888a..0a9b753eca4 100644
--- a/gdb/testsuite/gdb.btrace/non-stop.exp
+++ b/gdb/testsuite/gdb.btrace/non-stop.exp
@@ -60,17 +60,15 @@ proc gdb_cont_to { threads cmd line nthreads } {
     # the prompt precedes any further output except some errors.
     gdb_test_multiple "$full_cmd &" "$full_cmd: prompt" {
         -re "$gdb_prompt " {
-            pass "$full_cmd: prompt"
+            pass $gdb_test_name
         }
     }
 
     # now check for the expected line - one per thread.
     for {set i 0} {$i < $nthreads} {incr i} {
-        set test "$full_cmd: thread $i"
-
-        gdb_test_multiple "" $test {
+        gdb_test_multiple "" "$full_cmd: thread $i" {
             -re "$line\[^\\\r\\\n\]*\r\n" {
-                pass $test
+                pass $gdb_test_name
             }
         }
     }
@@ -96,9 +94,13 @@ proc gdb_cont_to_no_history { threads cmd nthreads } {
 }
 
 # trace the code between the two breakpoints
-gdb_cont_to_bp_line "$srcfile:$bp_1" all 2
-gdb_test_no_output "record btrace"
-gdb_cont_to_bp_line "$srcfile:$bp_2" all 2
+with_test_prefix "prepare" {
+    gdb_cont_to_bp_line "$srcfile:$bp_1" all 2
+}
+with_test_prefix "record" {
+    gdb_test_no_output "record btrace"
+    gdb_cont_to_bp_line "$srcfile:$bp_2" all 2
+}
 
 # we don't need those breakpoints any longer.
 # they will only disturb our stepping.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 3/4] testsuite, gdb.btrace: skip multi-inferior.exp on gdbserver targets

[Metzger, Markus T via Gdb-patches]

Hello Simon,

Thanks for your review.

> > --- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
> > +++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
> > @@ -27,6 +27,13 @@ if { [skip_btrace_tests] } {
> >      return -1
> >  }
> >
> > +# This test creates multiple inferiors.  This won't work with all
> > +# gdbserver board files.
> 
> The comment sounds a bit confusing.  It may be interpreted as "this test
> doesn't work with any gdbserver board files", but you mean "this test works
> with some gdbserver board files but not all".
> 
> I think just saying "Test test creates multiple inferiors." is enough to
> explain the [use_gdb_stub] check.  Stub configurations can't spawn inferiors.

I removed the comment altogether given that it now merely duplicates the code.


> Although I think it wouldn't hurt to improve the documentation of the
> use_gdb_stub procedure to explain what it means, as it's a pretty opaque
> concept if you don't already know it.

How about this [1]?  I'll send a separate patch for it.

Regards,
Markus.

[1]
---

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e812237d67a..21c822f26a6 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3589,6 +3589,9 @@ proc gdb_is_target_native { } {
 # This is the preferred way of checking use_gdb_stub, since it allows to check
 # the value before the gdb has been spawned and it will return the correct value
 # even when it was overriden by the test.
+#
+# Note that stub targets are not able to spawn new inferiors.  Use this
+# check for skipping respective tests.
 
 proc use_gdb_stub {} {
   global use_gdb_stub
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 4/4] testsuite, gdb.btrace: fix 32-bit PIE false positives

[Metzger, Markus T via Gdb-patches]

Hello Simon,

Thanks for your review.

> > +# We expect a specific function call history.  This gets messed up with
> > +# PIE on 32-bit.
> >  standard_testfile exception.cc
> > -if [prepare_for_testing "failed to prepare" $testfile $srcfile {c++ debug}] {
> > +if [prepare_for_testing "failed to prepare" $testfile $srcfile \
> > +	{additional_flags=-fno-pie ldflags=-no-pie c++ debug}] {
> >      return -1
> >  }
> 
> There's the "nopie" option you can pass to gdb_compile to disable PIE.  Although
> from
> what I see, it doesn't add -fno-pie to compilation options, just adds -no-pie to link
> options.  I suppose you need both?  If so, it could be preferable in the long run to
> teach gdb_compile to pass -fno-pie to the compilation step as well.

Indeed, older GCC require -fno-pie in addition to -no-pie, e.g. GCC 7.5 on Ubuntu 18.04.

I changed the test to use nopie and I'll submit a new patch to add -fno-pie to the
nopie handling of gdb_compile.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 3/4] testsuite, gdb.btrace: skip multi-inferior.exp on gdbserver targets

[Simon Marchi via Gdb-patches]

On 2020-12-21 2:27 a.m., Metzger, Markus T wrote:
> Hello Simon,
> 
> Thanks for your review.
> 
>>> --- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
>>> +++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
>>> @@ -27,6 +27,13 @@ if { [skip_btrace_tests] } {
>>>      return -1
>>>  }
>>>
>>> +# This test creates multiple inferiors.  This won't work with all
>>> +# gdbserver board files.
>>
>> The comment sounds a bit confusing.  It may be interpreted as "this test
>> doesn't work with any gdbserver board files", but you mean "this test works
>> with some gdbserver board files but not all".
>>
>> I think just saying "Test test creates multiple inferiors." is enough to
>> explain the [use_gdb_stub] check.  Stub configurations can't spawn inferiors.
> 
> I removed the comment altogether given that it now merely duplicates the code.
> 
> 
>> Although I think it wouldn't hurt to improve the documentation of the
>> use_gdb_stub procedure to explain what it means, as it's a pretty opaque
>> concept if you don't already know it.
> 
> How about this [1]?  I'll send a separate patch for it.
> 
> Regards,
> Markus.
> 
> [1]
> ---
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index e812237d67a..21c822f26a6 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3589,6 +3589,9 @@ proc gdb_is_target_native { } {
>  # This is the preferred way of checking use_gdb_stub, since it allows to check
>  # the value before the gdb has been spawned and it will return the correct value
>  # even when it was overriden by the test.
> +#
> +# Note that stub targets are not able to spawn new inferiors.  Use this
> +# check for skipping respective tests.

That's good, thanks.

Simon