[PATCH 0/2] Fix remaining gdb.trace/ duplicate test names

[PATCH 0/2] Fix remaining gdb.trace/ duplicate test names

[Andrew Burgess]

I went ahead and pushed a couple of patches to fix some duplicate test
names.  They were all the "obvious" changes.  This series contains the
last few fixes for gdb.trace/*, but these are heading towards non-obvious.

If there's no feedback I'll go ahead and push these some time next week.

---

Andrew Burgess (2):
  gdb/testsuite: fix some more duplicate test names in gdb.trace/
  gdb/testsuite: fix duplicate test names in gdb.trace/circ.exp

 gdb/testsuite/gdb.trace/circ.exp        | 76 +++++++++++++++----------
 gdb/testsuite/gdb.trace/ftrace-lock.exp | 45 +++++++--------
 gdb/testsuite/gdb.trace/trace-mt.exp    | 56 ++++++++----------
 3 files changed, 91 insertions(+), 86 deletions(-)


base-commit: 05d1b4b4ad7d74a64cc71c53d621241fc393fcb6
-- 
2.25.4

[PATCH 1/2] gdb/testsuite: fix some more duplicate test names in gdb.trace/

[Andrew Burgess]

This commit fixes some duplicate test names in the gdb.trace/
directory when run with the native-gdbserver and
native-extended-gdbserver boards.  In this case the duplicates relate
to the calls to gdb_compile_pthreads which emits a fixed PASS message,
as there are two calls to gdb_compile_pthreads we get a duplicate PASS
message.

In both cases the problem is fixed by adding a with_test_prefix around
one of the compilations, however, I've made additional changes to
clean up the tests a little while I was working on them:

  1. Switch to use prepare_for_testing instead of
  gdb_compile_pthreads.  By passing the 'pthreads' option this does
  call gdb_compile_pthreads under the hood, but using the standard
  compile function is cleaner,

  2. Using prepare_for_testing removes the need to call clean_restart
  immediately afterwards, so those calls are removed,

  3. I removed the unneeded $executable and $expfile globals, where
  the $executable global was used I've replaced this with $binfile,

  4. When we compile two executables I've now given these different
  names so that both exist at the end of the test run,

  5. Removed a gdb_reinitialize_dir call, this is covered by
  clean_restart,

  6. Use gdb_test_no_output where it makes sense.

I now see no duplicate test names when running these test scripts.
There should be no change in what is being tested after this commit.
---
 gdb/testsuite/gdb.trace/ftrace-lock.exp | 45 ++++++++++----------
 gdb/testsuite/gdb.trace/trace-mt.exp    | 56 +++++++++++--------------
 2 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.exp b/gdb/testsuite/gdb.trace/ftrace-lock.exp
index eb0d0f02933..ce2b890229a 100644
--- a/gdb/testsuite/gdb.trace/ftrace-lock.exp
+++ b/gdb/testsuite/gdb.trace/ftrace-lock.exp
@@ -16,9 +16,10 @@ load_lib "trace-support.exp"
 
 require allow_shlib_tests
 
+# Check that the target supports trace.
+require gdb_trace_common_supports_arch
+
 standard_testfile
-set executable $testfile
-set expfile $testfile.exp
 
 # make check RUNTESTFLAGS='gdb.trace/ftrace-lock.exp NUM_THREADS=2'
 if ![info exists NUM_THREADS] {
@@ -27,25 +28,23 @@ if ![info exists NUM_THREADS] {
 
 # Some targets have leading underscores on assembly symbols.
 set additional_flags [gdb_target_symbol_prefix_flags]
-set options [list debug [gdb_target_symbol_prefix_flags] \
+set options [list debug pthreads [gdb_target_symbol_prefix_flags] \
 	     additional_flags=-DNUM_THREADS=$NUM_THREADS]
 
-# Check that the target supports trace.
-require gdb_trace_common_supports_arch
-if { [gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } {
-    untested "failed to compile"
-    return -1
-}
-
-clean_restart ${testfile}
+with_test_prefix "runtime trace support check" {
+    if { [prepare_for_testing "prepare for testing" ${binfile}-check \
+	      $srcfile $options] } {
+	return
+    }
 
-if ![runto_main] {
-    return -1
-}
+    if ![runto_main] {
+	return -1
+    }
 
-if ![gdb_target_supports_trace] {
-    unsupported "target does not support trace"
-    return -1
+    if ![gdb_target_supports_trace] {
+	unsupported "target does not support trace"
+	return -1
+    }
 }
 
 # Compile the test case with the in-process agent library.
@@ -54,13 +53,11 @@ set remote_libipa [gdb_load_shlib $libipa]
 
 lappend options shlib=$libipa
 
-if { [gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } {
-    untested "failed to compile with in-process agent library"
-    return -1
+if { [prepare_for_testing "prepare for testing with libipa" \
+	  $binfile $srcfile $options] } {
+    return
 }
 
-clean_restart ${executable}
-
 if ![runto_main] {
     return -1
 }
@@ -76,7 +73,7 @@ gdb_breakpoint "fail" qualified
 gdb_test "ftrace set_point" "Fast tracepoint .*" \
     "fast tracepoint at a long insn"
 
-gdb_test "tstart" ""
+gdb_test_no_output "tstart"
 
 # If NUM_THREADS is high then this test case may timeout.  Increase the
 # timeout temporarily.
@@ -86,4 +83,4 @@ with_timeout_factor $NUM_THREADS {
 	"do not hit the fail function"
 }
 
-gdb_test "tstop" ""
+gdb_test_no_output "tstop"
diff --git a/gdb/testsuite/gdb.trace/trace-mt.exp b/gdb/testsuite/gdb.trace/trace-mt.exp
index bfa66dd88ee..e56064bbe8b 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.exp
+++ b/gdb/testsuite/gdb.trace/trace-mt.exp
@@ -15,39 +15,36 @@
 load_lib "trace-support.exp"
 
 standard_testfile
-set executable $testfile
-set expfile $testfile.exp
 
 # Some targets have leading underscores on assembly symbols.
 set additional_flags [gdb_target_symbol_prefix_flags]
 
 require gdb_trace_common_supports_arch
 
-if { [gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile \
-	  executable [list debug $additional_flags] ] != "" } {
-    # gdb_compile_pthreads provides an appropriate unsupported message.
-    return -1
-}
-
-clean_restart ${testfile}
+with_test_prefix "runtime trace support check" {
+    if { [prepare_for_testing "prepare for testing" ${binfile} $srcfile \
+	      [list debug pthreads $additional_flags]] } {
+	return
+    }
 
-if ![runto_main] {
-    return -1
-}
+    if ![runto_main] {
+	return -1
+    }
 
-if ![gdb_target_supports_trace] {
-    unsupported "target does not support trace"
-    return -1
+    if ![gdb_target_supports_trace] {
+	unsupported "target does not support trace"
+	return -1
+    }
 }
 
-proc step_over_tracepoint { trace_type } \
+proc step_over_tracepoint { binfile trace_type } \
 {with_test_prefix "step over $trace_type" \
 {
-    global executable
     global hex
 
     # Start with a fresh gdb.
-    clean_restart ${executable}
+    clean_restart $binfile
+
     # Make sure inferior is running in all-stop mode.
     gdb_test_no_output "set non-stop 0"
     if ![runto_main] {
@@ -66,14 +63,13 @@ proc step_over_tracepoint { trace_type } \
 
 # Set breakpoint and tracepoint at the same address.
 
-proc break_trace_same_addr { trace_type option } \
+proc break_trace_same_addr { binfile trace_type option } \
 {with_test_prefix "$trace_type $option" \
 {
-    global executable
     global hex
 
     # Start with a fresh gdb.
-    clean_restart ${executable}
+    clean_restart $binfile
     if ![runto_main] {
 	return -1
     }
@@ -100,10 +96,10 @@ proc break_trace_same_addr { trace_type option } \
 }}
 
 foreach break_always_inserted { "on" "off" } {
-    break_trace_same_addr "trace" ${break_always_inserted}
+    break_trace_same_addr $binfile "trace" ${break_always_inserted}
 }
 
-step_over_tracepoint "trace"
+step_over_tracepoint $binfile "trace"
 
 require allow_shlib_tests
 
@@ -111,24 +107,22 @@ set libipa [get_in_proc_agent]
 set remote_libipa [gdb_load_shlib $libipa]
 
 # Compile test case again with IPA.
-if { [gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile \
-	  executable [list debug $additional_flags shlib=$libipa] ] != "" } {
-    untested "failed to compile with in-process agent library"
-    return -1
+set binfile_ipa ${binfile}-ipa
+if { [prepare_for_testing "prepare for testing" $binfile_ipa $srcfile \
+	  [list debug pthreads $additional_flags shlib=$libipa]] } {
+    return
 }
-clean_restart ${executable}
 
 if ![runto_main] {
     return 0
 }
 
-gdb_reinitialize_dir $srcdir/$subdir
 if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 } {
     untested "could not find IPA lib loaded"
 } else {
     foreach break_always_inserted { "on" "off" } {
-	break_trace_same_addr "ftrace" ${break_always_inserted}
+	break_trace_same_addr $binfile_ipa "ftrace" ${break_always_inserted}
     }
 
-    step_over_tracepoint "ftrace"
+    step_over_tracepoint $binfile_ipa "ftrace"
 }
-- 
2.25.4

[PATCH 2/2] gdb/testsuite: fix duplicate test names in gdb.trace/circ.exp

[Andrew Burgess]

This fixes some duplicate test names in gdb.trace/circ.exp when using
native-gdbserver and native-extended-gdbserver boards.

In this test we set the trace buffer size twice.  The same test name
was used each time the size was adjusted.

I've fixed this issue by:

  1. Creating a new proc, set_trace_buffer_size, which factors out the
  code to change the buffer size, and uses test names based on the
  size we're setting the buffer too,

  2. Calling the new proc each time we want to adjust the buffer size.

After this the duplicate test names are resolved.  There should be no
change in what is tested after this commit.
---
 gdb/testsuite/gdb.trace/circ.exp | 76 +++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/circ.exp b/gdb/testsuite/gdb.trace/circ.exp
index 66bb64852fd..d123a1e7686 100644
--- a/gdb/testsuite/gdb.trace/circ.exp
+++ b/gdb/testsuite/gdb.trace/circ.exp
@@ -115,45 +115,59 @@ gdb_test "show circular-trace-buffer" \
     "Target's use of circular trace buffer is on." \
     "show circular-trace-buffer (on)"
 
-# Check if changing the trace buffer size is supported.  This step is
-# repeated twice.  This helps in case the trace buffer size is 100.
-set test_size 100
-set test "change buffer size to $test_size"
-gdb_test_multiple "set trace-buffer-size $test_size" $test {
-    -re ".*Target does not support this command.*$gdb_prompt $" {
-	unsupported "target does not support changing trace buffer size"
-	return 1
+# Use 'set trace-buffer-size' to change the trace buffer size to
+# REQUIRED_SIZE.  Return -1 if the current target doesn't support
+# adjusting the trace buffer size.  Return 0 if the adjustment failed
+# for some other reason.  Return 1 if the trace buffer size was
+# correctly adjusted.
+#
+# If this proc returns -1 then a suitable call to the unsupported proc
+# will have already been made.
+proc set_trace_buffer_size { required_size } {
+    set test_passed false
+    gdb_test_multiple "set trace-buffer-size $required_size" "" {
+	-re -wrap ".*Target does not support this command.*" {
+	    unsupported "target does not support changing trace buffer size"
+	    return -1
+	}
+	-re -wrap "" {
+	    pass $gdb_test_name
+	    set test_passed true
+	}
     }
-    -re "$gdb_prompt $" {
-	pass $test
+
+    if { !$test_passed } {
+	return 0
     }
-}
 
-set test "check whether setting trace buffer size is supported"
-gdb_test_multiple "tstatus" $test {
-    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_prompt $" {
-	set total_size $expect_out(2,string)
-	if { $test_size != $total_size } {
-	    unsupported "target does not support changing trace buffer size"
-	    return 1
+    set test_passed false
+    gdb_test_multiple "tstatus" "check trace-buffer-size is $required_size" {
+	-re -wrap ".*Trace buffer has ($::decimal) bytes of ($::decimal) bytes free.*" {
+	    set total_size $expect_out(2,string)
+	    if { $required_size != $total_size } {
+		unsupported "target does not support changing trace buffer size"
+		return -1
+	    }
+	    pass $gdb_test_name
+	    set test_passed true
 	}
-	pass $test
     }
+
+    if { !$test_passed } {
+	return 0
+    }
+
+    return 1
 }
 
-set test_size 400
-gdb_test_no_output "set trace-buffer-size $test_size" \
-    "change buffer size to $test_size"
+# Check if changing the trace buffer size is supported.  This step is
+# repeated twice.  This helps in case the trace buffer size is 100.
+if {[set_trace_buffer_size 100] < 0} {
+    return
+}
 
-gdb_test_multiple "tstatus" $test {
-    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_prompt $" {
-	set total_size $expect_out(2,string)
-	if { $test_size != $total_size } {
-	    unsupported "target does not support changing trace buffer size"
-	    return 1
-	}
-	pass $test
-    }
+if {[set_trace_buffer_size 400] < 0} {
+    return
 }
 
 gdb_test_no_output "set circular-trace-buffer off" \
-- 
2.25.4

Re: [PATCH 0/2] Fix remaining gdb.trace/ duplicate test names

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> I went ahead and pushed a couple of patches to fix some duplicate test
> names.  They were all the "obvious" changes.  This series contains the
> last few fixes for gdb.trace/*, but these are heading towards non-obvious.
>
> If there's no feedback I'll go ahead and push these some time next
> week.

I've now pushed these fixes.  Let me know if there are any problems.

Thanks,
Andrew


>
> ---
>
> Andrew Burgess (2):
>   gdb/testsuite: fix some more duplicate test names in gdb.trace/
>   gdb/testsuite: fix duplicate test names in gdb.trace/circ.exp
>
>  gdb/testsuite/gdb.trace/circ.exp        | 76 +++++++++++++++----------
>  gdb/testsuite/gdb.trace/ftrace-lock.exp | 45 +++++++--------
>  gdb/testsuite/gdb.trace/trace-mt.exp    | 56 ++++++++----------
>  3 files changed, 91 insertions(+), 86 deletions(-)
>
>
> base-commit: 05d1b4b4ad7d74a64cc71c53d621241fc393fcb6
> -- 
> 2.25.4