public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Some gdbserver test suite fixes
@ 2018-03-14 16:12 Andreas Arnez
  2018-03-14 16:13 ` [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver Andreas Arnez
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Andreas Arnez @ 2018-03-14 16:12 UTC (permalink / raw)
  To: gdb-patches

These are fixes for a few test suite problems I encountered when running
with --target_board=native-gdbserver or native-extended-gdbserver.  The
patches are just loosely bundled in this series, but really independent
from each other.

Andreas Arnez (3):
  Fix tspeed test case: copy libinproctrace to target
  Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  Fix a FAIL in attach.exp under native-extended-gdbserver

 gdb/testsuite/gdb.base/attach.exp       |  7 +++++++
 gdb/testsuite/gdb.trace/trace-break.c   |  4 ++--
 gdb/testsuite/gdb.trace/trace-break.exp | 12 ++++++------
 gdb/testsuite/gdb.trace/trace-mt.c      |  4 ++--
 gdb/testsuite/gdb.trace/trace-mt.exp    |  2 +-
 gdb/testsuite/gdb.trace/tspeed.exp      |  1 +
 gdb/testsuite/gdb.trace/unavailable.cc  |  8 ++++----
 gdb/testsuite/gdb.trace/unavailable.exp | 20 ++++++++++----------
 8 files changed, 33 insertions(+), 25 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target
  2018-03-14 16:12 [PATCH 0/3] Some gdbserver test suite fixes Andreas Arnez
  2018-03-14 16:13 ` [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver Andreas Arnez
@ 2018-03-14 16:13 ` Andreas Arnez
  2018-03-15 21:40   ` Simon Marchi
  2018-03-14 16:13 ` [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash Andreas Arnez
  2 siblings, 1 reply; 13+ messages in thread
From: Andreas Arnez @ 2018-03-14 16:13 UTC (permalink / raw)
  To: gdb-patches

The tspeed test case does not execute correctly because libinproctrace.so
is not copied to the target.  This is fixed.

gdb/testsuite/ChangeLog:

	* gdb.trace/tspeed.exp: Add invocation of gdb_load_shlib to ensure
	that libinproctrace is copied to the target.
---
 gdb/testsuite/gdb.trace/tspeed.exp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/testsuite/gdb.trace/tspeed.exp b/gdb/testsuite/gdb.trace/tspeed.exp
index d53608c51b..ecd36d2d9b 100644
--- a/gdb/testsuite/gdb.trace/tspeed.exp
+++ b/gdb/testsuite/gdb.trace/tspeed.exp
@@ -19,6 +19,7 @@ standard_testfile
 set executable $testfile
 
 set ipalib [get_in_proc_agent]
+gdb_load_shlib $ipalib
 
 if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
 	  executable [concat {debug nowarnings c} shlib=$ipalib]] != "" } {
-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver
  2018-03-14 16:12 [PATCH 0/3] Some gdbserver test suite fixes Andreas Arnez
@ 2018-03-14 16:13 ` Andreas Arnez
  2018-03-15 22:17   ` Simon Marchi
  2018-03-14 16:13 ` [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target Andreas Arnez
  2018-03-14 16:13 ` [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash Andreas Arnez
  2 siblings, 1 reply; 13+ messages in thread
From: Andreas Arnez @ 2018-03-14 16:13 UTC (permalink / raw)
  To: gdb-patches

The attach.exp test case yields a FAIL with native-extended-gdbserver when
trying to start a new process.  This is because gdbserver does not support
starting new processes.  And since the gdbserver-base board file sets the
GDB command line option -ex "set auto-connect-native-target off", the
process is not started on the native target either.  An error message
results instead:

  Don't know how to run.  Try "help target".

Thus just accept this error when not running on a native target.

gdb/testsuite/ChangeLog:

	* gdb.base/attach.exp: Accept the error message "don't know how to
	run" when not running on a native target.
---
 gdb/testsuite/gdb.base/attach.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index efec49e385..1651dc40a7 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -421,11 +421,18 @@ proc test_command_line_attach_run {} {
 
 	send_gdb "y\n"
 
+	set cantrun 0
 	set test "run to main"
 	gdb_test_multiple "" $test {
 	    -re "Temporary breakpoint .* main .*$gdb_prompt $" {
 		pass $test
 	    }
+	    -re "Don't know how to run..*$gdb_prompt $" {
+		set cantrun 1
+	    }
+	}
+	if { $cantrun && [gdb_is_target_native] } {
+	    fail $test
 	}
 
 	# Get rid of the process
-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-14 16:12 [PATCH 0/3] Some gdbserver test suite fixes Andreas Arnez
  2018-03-14 16:13 ` [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver Andreas Arnez
  2018-03-14 16:13 ` [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target Andreas Arnez
@ 2018-03-14 16:13 ` Andreas Arnez
  2018-03-15 21:58   ` Simon Marchi
  2 siblings, 1 reply; 13+ messages in thread
From: Andreas Arnez @ 2018-03-14 16:13 UTC (permalink / raw)
  To: gdb-patches

Some of GDB's trace test cases define a function end() and place a
breakpoint there with "break end".  However, when libinproctrace is linked
to the binary, there are multiple methods named "end", such as
std::string::end() from the C++ library or format_pieces::end() from
common/format.h.  GDB then creates multiple breakpoints instead of just a
single one, and some FAILs result, such as these:

  FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
  FAIL: gdb.trace/trace-mt.exp: ftrace off: break end

This is fixed by renaming end() to my_end().  For consistency, where end()
was paired with a previous begin(), the latter is renamed to my_begin() as
well.

gdb/testsuite/ChangeLog:

	* gdb.trace/trace-break.c (end): Rename to...
	(my_end): ...this.
	(main): Adjust call.
	* gdb.trace/trace-mt.c (end): Rename to...
	(my_end): ...this.
	(main): Adjust call.
	* gdb.trace/unavailable.cc (begin): Rename to...
	(my_begin): ...this.
	(end): Rename to...
	(my_end): ...this.
	(main): Adjust calls.
	* gdb.trace/trace-break.exp: Reflect function name change.
	* gdb.trace/trace-mt.exp: Likewise.
	* gdb.trace/unavailable.exp: Likewise.
---
 gdb/testsuite/gdb.trace/trace-break.c   |  4 ++--
 gdb/testsuite/gdb.trace/trace-break.exp | 12 ++++++------
 gdb/testsuite/gdb.trace/trace-mt.c      |  4 ++--
 gdb/testsuite/gdb.trace/trace-mt.exp    |  2 +-
 gdb/testsuite/gdb.trace/unavailable.cc  |  8 ++++----
 gdb/testsuite/gdb.trace/unavailable.exp | 20 ++++++++++----------
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/trace-break.c b/gdb/testsuite/gdb.trace/trace-break.c
index 6aa9d09214..3349066dcc 100644
--- a/gdb/testsuite/gdb.trace/trace-break.c
+++ b/gdb/testsuite/gdb.trace/trace-break.c
@@ -31,13 +31,13 @@ marker (void)
 }
 
 static void
-end (void)
+my_end (void)
 {}
 
 int
 main ()
 {
   marker ();
-  end ();
+  my_end ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index 539fcb5218..602b2e23ec 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -53,7 +53,7 @@ proc break_trace_same_addr_1 { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
@@ -88,7 +88,7 @@ proc break_trace_same_addr_2 { trace_type1 trace_type2 option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "${trace_type1} set_point" \
 	"\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
@@ -127,7 +127,7 @@ proc break_trace_same_addr_3 { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
     gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
@@ -165,7 +165,7 @@ proc break_trace_same_addr_4 { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
     gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
@@ -208,7 +208,7 @@ proc break_trace_same_addr_5 { trace1 trace2 trace3 trace3_at_first_loc } \
     }
 
     gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "${trace1} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
 	"${trace1} set_point 1"
@@ -282,7 +282,7 @@ proc break_trace_same_addr_6 { trace1 enable1 trace2 enable2 } \
     }
 
     gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "continue" "Continuing\\.\[ \r\n\]+(Thread .* hit )?Breakpoint.*" \
 	"continue to marker"
diff --git a/gdb/testsuite/gdb.trace/trace-mt.c b/gdb/testsuite/gdb.trace/trace-mt.c
index 6f04ba7775..37d1ee0ea5 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.c
+++ b/gdb/testsuite/gdb.trace/trace-mt.c
@@ -25,7 +25,7 @@ thread_function(void *arg)
 }
 
 static void
-end (void)
+my_end (void)
 {}
 
 int
@@ -40,7 +40,7 @@ main (int argc, char *argv[], char *envp[])
   for (i = 0; i < 2; i++)
     pthread_join (threads[i], NULL);
 
-  end ();
+  my_end ();
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.trace/trace-mt.exp b/gdb/testsuite/gdb.trace/trace-mt.exp
index f327406a13..bd89c9bec2 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.exp
+++ b/gdb/testsuite/gdb.trace/trace-mt.exp
@@ -81,7 +81,7 @@ proc break_trace_same_addr { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break my_end" "Breakpoint \[0-9\] at $hex: file.*"
 
     gdb_test "break set_point1" "Breakpoint \[0-9\] at $hex: file.*"
     gdb_test "${trace_type} set_point1" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
diff --git a/gdb/testsuite/gdb.trace/unavailable.cc b/gdb/testsuite/gdb.trace/unavailable.cc
index 56e5541259..aa4aeef65f 100644
--- a/gdb/testsuite/gdb.trace/unavailable.cc
+++ b/gdb/testsuite/gdb.trace/unavailable.cc
@@ -174,12 +174,12 @@ Virtual *virtualp = &virtual_partial;
 /* Test functions.  */
 
 static void
-begin ()	/* called before anything else */
+my_begin ()	/* called before anything else */
 {
 }
 
 static void
-end ()		/* called after everything else */
+my_end ()		/* called after everything else */
 {
 }
 
@@ -325,7 +325,7 @@ main (int argc, char **argv, char **envp)
   test_struct mystruct;
   int         myarray[4];
 
-  begin ();
+  my_begin ();
   /* Assign collectable values to global variables.  */
   globalc = 71;
   globali = 72;
@@ -395,6 +395,6 @@ main (int argc, char **argv, char **envp)
   g_structref.clear ();
   g_structref_p = NULL;
 
-  end ();
+  my_end ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.trace/unavailable.exp b/gdb/testsuite/gdb.trace/unavailable.exp
index 79aa8ef1b0..7e8fc8b303 100644
--- a/gdb/testsuite/gdb.trace/unavailable.exp
+++ b/gdb/testsuite/gdb.trace/unavailable.exp
@@ -61,21 +61,21 @@ proc prepare_for_trace_test {} {
 
     runto_main
 
-    gdb_test "break begin" ".*" ""
-    gdb_test "break end" ".*" ""
+    gdb_test "break my_begin" ".*" ""
+    gdb_test "break my_end" ".*" ""
 }
 
 proc run_trace_experiment { test_func } {
     global gdb_prompt
 
     gdb_test "continue" \
-	".*Breakpoint \[0-9\]+, begin .*" \
-	"advance to begin"
+	".*Breakpoint \[0-9\]+, my_begin .*" \
+	"advance to my_begin"
 
     gdb_test_no_output "tstart" "start trace experiment"
 
     gdb_test "continue" \
-	    "Continuing.*Breakpoint \[0-9\]+, end.*" \
+	    "Continuing.*Breakpoint \[0-9\]+, my_end.*" \
 	    "run trace experiment"
     gdb_test "tstop" \
 	    "\[\r\n\]+" \
@@ -189,7 +189,7 @@ proc gdb_collect_args_test {} {
 	gdb_collect_args_test_1
 
 	gdb_test "tfind none" \
-	    "#0  end .*" \
+	    "#0  my_end .*" \
 	    "cease trace debugging"
 
 	set tracefile [standard_output_file ${testfile}]
@@ -273,7 +273,7 @@ proc gdb_collect_locals_test { func msg } {
 	gdb_collect_locals_test_1 $func
 
 	gdb_test "tfind none" \
-	    "#0  end .*" \
+	    "#0  my_end .*" \
 	    "cease trace debugging"
 
 	set tracefile [standard_output_file ${testfile}]
@@ -357,7 +357,7 @@ proc gdb_unavailable_registers_test { } {
 
 	gdb_unavailable_registers_test_1
 
-	gdb_test "tfind none" "#0  end .*" "cease trace debugging"
+	gdb_test "tfind none" "#0  my_end .*" "cease trace debugging"
 
 	set tracefile [standard_output_file ${testfile}]
 	gdb_test "tsave ${tracefile}.registers.tfile" \
@@ -421,7 +421,7 @@ proc gdb_unavailable_floats { } {
 
 	gdb_unavailable_floats_1
 
-	gdb_test "tfind none" "#0  end .*" "cease trace debugging"
+	gdb_test "tfind none" "#0  my_end .*" "cease trace debugging"
 
 	set tracefile [standard_output_file ${testfile}]
 	gdb_test "tsave ${tracefile}.floats.tfile" \
@@ -688,7 +688,7 @@ proc gdb_collect_globals_test { } {
 	gdb_collect_globals_test_1
 
 	gdb_test "tfind none" \
-	    "#0  end .*" \
+	    "#0  my_end .*" \
 	    "cease trace debugging"
 
 	set tracefile [standard_output_file ${testfile}]
-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target
  2018-03-14 16:13 ` [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target Andreas Arnez
@ 2018-03-15 21:40   ` Simon Marchi
  2018-03-16 19:41     ` Andreas Arnez
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2018-03-15 21:40 UTC (permalink / raw)
  To: Andreas Arnez, gdb-patches

On 2018-03-14 12:11 PM, Andreas Arnez wrote:
> The tspeed test case does not execute correctly because libinproctrace.so
> is not copied to the target.  This is fixed.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/tspeed.exp: Add invocation of gdb_load_shlib to ensure
> 	that libinproctrace is copied to the target.
> ---
>  gdb/testsuite/gdb.trace/tspeed.exp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdb/testsuite/gdb.trace/tspeed.exp b/gdb/testsuite/gdb.trace/tspeed.exp
> index d53608c51b..ecd36d2d9b 100644
> --- a/gdb/testsuite/gdb.trace/tspeed.exp
> +++ b/gdb/testsuite/gdb.trace/tspeed.exp
> @@ -19,6 +19,7 @@ standard_testfile
>  set executable $testfile
>  
>  set ipalib [get_in_proc_agent]
> +gdb_load_shlib $ipalib
>  
>  if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
>  	  executable [concat {debug nowarnings c} shlib=$ipalib]] != "" } {
> 

LGTM.

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-14 16:13 ` [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash Andreas Arnez
@ 2018-03-15 21:58   ` Simon Marchi
  2018-03-16 19:47     ` Andreas Arnez
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2018-03-15 21:58 UTC (permalink / raw)
  To: Andreas Arnez, gdb-patches

On 2018-03-14 12:11 PM, Andreas Arnez wrote:
> Some of GDB's trace test cases define a function end() and place a
> breakpoint there with "break end".  However, when libinproctrace is linked
> to the binary, there are multiple methods named "end", such as
> std::string::end() from the C++ library or format_pieces::end() from
> common/format.h.  GDB then creates multiple breakpoints instead of just a
> single one, and some FAILs result, such as these:
> 
>   FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
>   FAIL: gdb.trace/trace-mt.exp: ftrace off: break end
> 
> This is fixed by renaming end() to my_end().  For consistency, where end()
> was paired with a previous begin(), the latter is renamed to my_begin() as
> well.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/trace-break.c (end): Rename to...
> 	(my_end): ...this.
> 	(main): Adjust call.
> 	* gdb.trace/trace-mt.c (end): Rename to...
> 	(my_end): ...this.
> 	(main): Adjust call.
> 	* gdb.trace/unavailable.cc (begin): Rename to...
> 	(my_begin): ...this.
> 	(end): Rename to...
> 	(my_end): ...this.
> 	(main): Adjust calls.
> 	* gdb.trace/trace-break.exp: Reflect function name change.
> 	* gdb.trace/trace-mt.exp: Likewise.
> 	* gdb.trace/unavailable.exp: Likewise.

Hi Andreas,

Another way would have been to pass "-qualified" to the break command,
or use gdb_breakpoint and pass the "qualified" arg.  There are also
other instances of the same "problem" in tests that don't check the
output of the break command at all...

  gdb_test "break end" ".*" ""

In any case, this patch LGTM.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver
  2018-03-14 16:13 ` [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver Andreas Arnez
@ 2018-03-15 22:17   ` Simon Marchi
  2018-04-25  9:32     ` Andreas Arnez
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2018-03-15 22:17 UTC (permalink / raw)
  To: Andreas Arnez, gdb-patches

Hi Andreas,

On 2018-03-14 12:11 PM, Andreas Arnez wrote:
> The attach.exp test case yields a FAIL with native-extended-gdbserver when
> trying to start a new process.  This is because gdbserver does not support
> starting new processes.  And since the gdbserver-base board file sets the
> GDB command line option -ex "set auto-connect-native-target off", the
> process is not started on the native target either.  An error message
> results instead:
> 
>   Don't know how to run.  Try "help target".
> 
> Thus just accept this error when not running on a native target.

It is not true that gdbserver can't run a new process, in fact it can in the
extended-remote protocol (as opposed to remote).  Start gdbserver with:

 $ ./gdbserver/gdbserver --once --multi :1234

And

 $ ./gdb --data-directory=data-directory
 (gdb) tar ext :1234
 (gdb) set remote exec-file /usr/bin/gnome-calculator
 (gdb) run

Still, I first thought this test wouldn't be applicable to the extended-remote
protocol because I though you couldn't mix -p with connecting to a remote target
on startup, but it seems like this works:

 $ pidof gnome-calculator
 20001
 $ ./gdb --data-directory=data-directory -iex "tar ext :1234" -p 20001  -ex "set remote exec-file /usr/bin/gnome-calculator" -ex "start"

So it might be possible to tweak the test case to adapt it to the extended-remote
protocol (add the -iex and "set remote exec-file" bits).

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target
  2018-03-15 21:40   ` Simon Marchi
@ 2018-03-16 19:41     ` Andreas Arnez
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Arnez @ 2018-03-16 19:41 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Thu, Mar 15 2018, Simon Marchi wrote:

> On 2018-03-14 12:11 PM, Andreas Arnez wrote:
>> The tspeed test case does not execute correctly because libinproctrace.so
>> is not copied to the target.  This is fixed.
>> 
>> gdb/testsuite/ChangeLog:
>> 
>> 	* gdb.trace/tspeed.exp: Add invocation of gdb_load_shlib to ensure
>> 	that libinproctrace is copied to the target.
>> ---
>>  gdb/testsuite/gdb.trace/tspeed.exp | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/gdb/testsuite/gdb.trace/tspeed.exp b/gdb/testsuite/gdb.trace/tspeed.exp
>> index d53608c51b..ecd36d2d9b 100644
>> --- a/gdb/testsuite/gdb.trace/tspeed.exp
>> +++ b/gdb/testsuite/gdb.trace/tspeed.exp
>> @@ -19,6 +19,7 @@ standard_testfile
>>  set executable $testfile
>>  
>>  set ipalib [get_in_proc_agent]
>> +gdb_load_shlib $ipalib
>>  
>>  if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
>>  	  executable [concat {debug nowarnings c} shlib=$ipalib]] != "" } {
>> 
>
> LGTM.

Thanks, pushed.

--
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-15 21:58   ` Simon Marchi
@ 2018-03-16 19:47     ` Andreas Arnez
  2018-03-16 21:57       ` Simon Marchi
  2018-03-26 14:44       ` Pedro Alves
  0 siblings, 2 replies; 13+ messages in thread
From: Andreas Arnez @ 2018-03-16 19:47 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Thu, Mar 15 2018, Simon Marchi wrote:

> On 2018-03-14 12:11 PM, Andreas Arnez wrote:
>> Some of GDB's trace test cases define a function end() and place a
>> breakpoint there with "break end".  However, when libinproctrace is linked
>> to the binary, there are multiple methods named "end", such as
>> std::string::end() from the C++ library or format_pieces::end() from
>> common/format.h.  GDB then creates multiple breakpoints instead of just a
>> single one, and some FAILs result, such as these:
>> 
>>   FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
>>   FAIL: gdb.trace/trace-mt.exp: ftrace off: break end
>> 
>> This is fixed by renaming end() to my_end().  For consistency, where end()
>> was paired with a previous begin(), the latter is renamed to my_begin() as
>> well.
>> 
>> gdb/testsuite/ChangeLog:
>> 
>> 	* gdb.trace/trace-break.c (end): Rename to...
>> 	(my_end): ...this.
>> 	(main): Adjust call.
>> 	* gdb.trace/trace-mt.c (end): Rename to...
>> 	(my_end): ...this.
>> 	(main): Adjust call.
>> 	* gdb.trace/unavailable.cc (begin): Rename to...
>> 	(my_begin): ...this.
>> 	(end): Rename to...
>> 	(my_end): ...this.
>> 	(main): Adjust calls.
>> 	* gdb.trace/trace-break.exp: Reflect function name change.
>> 	* gdb.trace/trace-mt.exp: Likewise.
>> 	* gdb.trace/unavailable.exp: Likewise.
>
> Hi Andreas,
>
> Another way would have been to pass "-qualified" to the break command,
> or use gdb_breakpoint and pass the "qualified" arg.  There are also
> other instances of the same "problem" in tests that don't check the
> output of the break command at all...
>
>   gdb_test "break end" ".*" ""
>
> In any case, this patch LGTM.

Hm, actually the use of "-qualified" is a good point.  I'd rather go
with that.  How about the patch below?

--
Andreas

-- >8 --
Subject: [PATCH] Testsuite: Fix ambiguous "break" due to libinproctrace

Some of GDB's trace test cases define a function end() and place a
breakpoint there with "break end".  However, when libinproctrace is linked
to the binary, there are multiple methods named "end", such as
std::string::end() from the C++ library or format_pieces::end() from
common/format.h.  GDB then creates multiple breakpoints instead of just a
single one, and some FAILs result, such as these:

  FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
  FAIL: gdb.trace/trace-mt.exp: ftrace off: break end

Fix this by adding the "-qualified" option to the break commands.  For
consistency, change all occurrences of "break end" (and similar) in all
trace test cases, even if the current behavior does not cause problems.
Also, consequently use the gdb_breakpoint convenience proc.

gdb/testsuite/ChangeLog:

	* gdb.trace/actions-changed.exp: Call gdb_breakpoint with the
	"qualified" option when setting breakpoints.
	* gdb.trace/backtrace.exp: Likewise.
	* gdb.trace/circ.exp: Likewise.
	* gdb.trace/collection.exp: Likewise.
	* gdb.trace/disconnected-tracing.exp: Likewise.
	* gdb.trace/ftrace-lock.exp: Likewise.
	* gdb.trace/ftrace.exp: Likewise.
	* gdb.trace/infotrace.exp: Likewise.
	* gdb.trace/packetlen.exp: Likewise.
	* gdb.trace/passc-dyn.exp: Likewise.
	* gdb.trace/qtro.exp: Likewise.
	* gdb.trace/read-memory.exp: Likewise.
	* gdb.trace/report.exp: Likewise.
	* gdb.trace/signal.exp: Likewise.
	* gdb.trace/status-stop.exp: Likewise.
	* gdb.trace/strace.exp: Likewise.
	* gdb.trace/tfind.exp: Likewise.
	* gdb.trace/trace-break.exp: Likewise.
	* gdb.trace/trace-condition.exp: Likewise.
	* gdb.trace/trace-mt.exp: Likewise.
	* gdb.trace/tstatus.exp: Likewise.
	* gdb.trace/tsv.exp: Likewise.
	* gdb.trace/unavailable-dwarf-piece.exp: Likewise.
	* gdb.trace/unavailable.exp: Likewise.
	* gdb.trace/while-dyn.exp: Likewise.
---
 gdb/testsuite/gdb.trace/actions-changed.exp        |  2 +-
 gdb/testsuite/gdb.trace/backtrace.exp              |  2 +-
 gdb/testsuite/gdb.trace/circ.exp                   |  4 ++--
 gdb/testsuite/gdb.trace/collection.exp             |  4 ++--
 gdb/testsuite/gdb.trace/disconnected-tracing.exp   |  4 ++--
 gdb/testsuite/gdb.trace/ftrace-lock.exp            |  4 ++--
 gdb/testsuite/gdb.trace/ftrace.exp                 |  4 ++--
 gdb/testsuite/gdb.trace/infotrace.exp              |  2 +-
 gdb/testsuite/gdb.trace/packetlen.exp              |  2 +-
 gdb/testsuite/gdb.trace/passc-dyn.exp              |  2 +-
 gdb/testsuite/gdb.trace/qtro.exp                   |  2 +-
 gdb/testsuite/gdb.trace/read-memory.exp            |  2 +-
 gdb/testsuite/gdb.trace/report.exp                 |  2 +-
 gdb/testsuite/gdb.trace/signal.exp                 |  6 ++---
 gdb/testsuite/gdb.trace/status-stop.exp            |  4 ++--
 gdb/testsuite/gdb.trace/strace.exp                 |  6 ++---
 gdb/testsuite/gdb.trace/tfind.exp                  |  2 +-
 gdb/testsuite/gdb.trace/trace-break.exp            | 26 +++++++++++-----------
 gdb/testsuite/gdb.trace/trace-condition.exp        |  4 ++--
 gdb/testsuite/gdb.trace/trace-mt.exp               |  2 +-
 gdb/testsuite/gdb.trace/tstatus.exp                |  4 ++--
 gdb/testsuite/gdb.trace/tsv.exp                    |  2 +-
 .../gdb.trace/unavailable-dwarf-piece.exp          |  2 +-
 gdb/testsuite/gdb.trace/unavailable.exp            |  4 ++--
 gdb/testsuite/gdb.trace/while-dyn.exp              |  2 +-
 25 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/actions-changed.exp b/gdb/testsuite/gdb.trace/actions-changed.exp
index 5482291c01..102f04f734 100644
--- a/gdb/testsuite/gdb.trace/actions-changed.exp
+++ b/gdb/testsuite/gdb.trace/actions-changed.exp
@@ -22,7 +22,7 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
 }
 
 proc test_actions_changed { } {
-    gdb_breakpoint "end"
+    gdb_breakpoint "end" qualified
 
     gdb_test "trace subr" "Tracepoint .*" \
 	"tracepoint at subr"
diff --git a/gdb/testsuite/gdb.trace/backtrace.exp b/gdb/testsuite/gdb.trace/backtrace.exp
index 1842627046..6b41f8945b 100644
--- a/gdb/testsuite/gdb.trace/backtrace.exp
+++ b/gdb/testsuite/gdb.trace/backtrace.exp
@@ -145,7 +145,7 @@ gdb_trace_setactions "8.6: setup TP to collect stack mem cast expr" \
 
 gdb_test_no_output "tstart" ""
 
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/circ.exp b/gdb/testsuite/gdb.trace/circ.exp
index f5312bdd3f..5af84feb24 100644
--- a/gdb/testsuite/gdb.trace/circ.exp
+++ b/gdb/testsuite/gdb.trace/circ.exp
@@ -66,7 +66,7 @@ proc run_trace_experiment { } {
     global decimal
 
     setup_tracepoints
-    gdb_test "break end" "Breakpoint $decimal.*" "breakpoint at end"
+    gdb_breakpoint "end" qualified
     gdb_test "tstart" "\[\r\n\]*" "start trace experiment"
     gdb_test "continue" "Continuing.*Breakpoint \[0-9\]+, end.*" \
 	"run to end"
@@ -174,7 +174,7 @@ set frame_size -1
 with_test_prefix "frame size" {
     set_a_tracepoint func0
 
-    gdb_test "break end" "Breakpoint $decimal.*" "breakpoint at end"
+    gdb_breakpoint "end" qualified
 
     gdb_test "tstart" "\[\r\n\]*" "start trace"
 
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index 4d3dc6bdab..b7eee6dbf4 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -64,8 +64,8 @@ proc prepare_for_trace_test {} {
 
     runto_main
 
-    gdb_test "break begin" ".*" ""
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "begin" qualified
+    gdb_breakpoint "end" qualified
 }
 
 proc run_trace_experiment { msg test_func } {
diff --git a/gdb/testsuite/gdb.trace/disconnected-tracing.exp b/gdb/testsuite/gdb.trace/disconnected-tracing.exp
index c806482fc6..7e77699d0c 100644
--- a/gdb/testsuite/gdb.trace/disconnected-tracing.exp
+++ b/gdb/testsuite/gdb.trace/disconnected-tracing.exp
@@ -62,7 +62,7 @@ proc disconnected_tracing {  } {
 	gdb_test "trace start" ".*"
 	gdb_trace_setactions "collect on tracepoint 2" "2" \
 	    "collect foo" "^$"
-	gdb_test "break end" "Breakpoint ${decimal} at .*"
+	gdb_breakpoint "end" qualified
 
 	gdb_test_no_output "tstart"
 
@@ -125,7 +125,7 @@ proc disconnected_tfind {  } {
 	gdb_test "trace start" ".*"
 	gdb_test_no_output "tstart"
 
-	gdb_test "break end" "Breakpoint ${decimal} at .*"
+	gdb_breakpoint "end" qualified
 	gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*"
 	gdb_test_no_output "tstop"
 
diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.exp b/gdb/testsuite/gdb.trace/ftrace-lock.exp
index 4675e190b2..3e5f0b8712 100644
--- a/gdb/testsuite/gdb.trace/ftrace-lock.exp
+++ b/gdb/testsuite/gdb.trace/ftrace-lock.exp
@@ -69,8 +69,8 @@ if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 }
     return 1
 }
 
-gdb_test "break end" ""
-gdb_test "break fail" ""
+gdb_breakpoint "end" qualified
+gdb_breakpoint "fail" qualified
 
 gdb_test "ftrace set_point" "Fast tracepoint .*" \
     "fast tracepoint at a long insn"
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index 6389924cb8..5c494c6bb4 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -104,9 +104,9 @@ proc test_fast_tracepoints {} {
 
     set fourgood 0
 
-    gdb_test "break begin" ".*" ""
+    gdb_breakpoint "begin" qualified
 
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "end" qualified
 
     gdb_test "print gdb_agent_gdb_trampoline_buffer_error" ".*" ""
 
diff --git a/gdb/testsuite/gdb.trace/infotrace.exp b/gdb/testsuite/gdb.trace/infotrace.exp
index 053dafb0d7..b01365822c 100644
--- a/gdb/testsuite/gdb.trace/infotrace.exp
+++ b/gdb/testsuite/gdb.trace/infotrace.exp
@@ -111,7 +111,7 @@ if { ![gdb_target_supports_trace] } then {
     return 1
 }
 
-gdb_test "break end" "Breakpoint \[0-9\] at .*"
+gdb_breakpoint "end" qualified
 gdb_test_no_output "tstart"
 gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
     "continue to end"
diff --git a/gdb/testsuite/gdb.trace/packetlen.exp b/gdb/testsuite/gdb.trace/packetlen.exp
index 3304904c13..ca54cfe159 100644
--- a/gdb/testsuite/gdb.trace/packetlen.exp
+++ b/gdb/testsuite/gdb.trace/packetlen.exp
@@ -61,7 +61,7 @@ gdb_trace_setactions "setup collect actions" \
 	"end" ""
 
 gdb_test_no_output "tstart" "survive the long packet send"
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/passc-dyn.exp b/gdb/testsuite/gdb.trace/passc-dyn.exp
index 26ba79a5d0..1738b46234 100644
--- a/gdb/testsuite/gdb.trace/passc-dyn.exp
+++ b/gdb/testsuite/gdb.trace/passc-dyn.exp
@@ -86,7 +86,7 @@ gdb_test "passcount 3 $tdp4" "Setting tracepoint $tdp4's passcount to 3" \
 
 gdb_test "tstart" ".*" ""
 
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
index 5fe1db4afb..830d25f437 100644
--- a/gdb/testsuite/gdb.trace/qtro.exp
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -48,7 +48,7 @@ if ![gdb_target_supports_trace] {
 # frame (IOW, returns while tfind mode is active).
 proc prepare_for_trace_disassembly { } {
     global gdb_prompt
-    gdb_breakpoint "end"
+    gdb_breakpoint "end" qualified
 
     gdb_test "trace subr" "Tracepoint .*" \
 	"tracepoint at subr"
diff --git a/gdb/testsuite/gdb.trace/read-memory.exp b/gdb/testsuite/gdb.trace/read-memory.exp
index 27b23eab06..e91c7fdf74 100644
--- a/gdb/testsuite/gdb.trace/read-memory.exp
+++ b/gdb/testsuite/gdb.trace/read-memory.exp
@@ -42,7 +42,7 @@ proc set_tracepoint_and_collect { } {
 	fail "can't run to main"
 	return -1
     }
-    gdb_test "break end" "Breakpoint \[0-9\] at .*"
+    gdb_breakpoint "end" qualified
     gdb_test "trace start" "Tracepoint \[0-9\] at .*"
     gdb_trace_setactions "set action for tracepoint"  "" \
 	"collect testglob" "^$" \
diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
index 75a9e1c355..5155165b69 100644
--- a/gdb/testsuite/gdb.trace/report.exp
+++ b/gdb/testsuite/gdb.trace/report.exp
@@ -160,7 +160,7 @@ gdb_trace_setactions "9.x: setup TP to collect expressions" \
 
 gdb_test "tstart" ".*" ""
 
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/signal.exp b/gdb/testsuite/gdb.trace/signal.exp
index b2337685da..b860b12d63 100644
--- a/gdb/testsuite/gdb.trace/signal.exp
+++ b/gdb/testsuite/gdb.trace/signal.exp
@@ -66,7 +66,7 @@ if ![runto_main] {
     return -1
 }
 
-gdb_test "break kill" "Breakpoint $decimal at .*"
+gdb_breakpoint "kill" qualified
 gdb_test "handle SIGABRT nostop noprint pass" ".*" "pass SIGABRT"
 
 # Hit the breakpoint on $syscall for the first time.  In this time,
@@ -119,7 +119,7 @@ gdb_test_multiple $test $test {
 }
 
 delete_breakpoints
-gdb_test "break start" "Breakpoint $decimal at .*"
+gdb_breakpoint "start" qualified
 gdb_continue_to_breakpoint "continue to start"
 
 gdb_assert { 0 == [get_integer_valueof "counter" "1"] } "counter is zero"
@@ -135,7 +135,7 @@ set tpnum [get_integer_valueof "\$bpnum" 0]
 gdb_test "trace *$syscall_insn_next" "Tracepoint $decimal at .*" \
     "tracepoint on instruction following syscall instruction"
 
-gdb_test "break end" "Breakpoint $decimal at .*"
+gdb_breakpoint "end" qualified
 
 gdb_test_no_output "tstart"
 gdb_test "continue" ".*Breakpoint.* end .*at.*$srcfile.*" \
diff --git a/gdb/testsuite/gdb.trace/status-stop.exp b/gdb/testsuite/gdb.trace/status-stop.exp
index 551c018b74..378bf2dba9 100644
--- a/gdb/testsuite/gdb.trace/status-stop.exp
+++ b/gdb/testsuite/gdb.trace/status-stop.exp
@@ -51,7 +51,7 @@ proc test_tstart_tstop_tstart { } {
 	gdb_test "trace func1" "Tracepoint \[0-9\] at $hex: file.*"
 	gdb_test_no_output "tstart"
 
-	gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+	gdb_breakpoint "end" qualified
 	gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
 	    "continue to end"
 
@@ -108,7 +108,7 @@ proc test_buffer_full_tstart { } {
 	    "collect buf" "^$"
 
 	gdb_test_no_output "tstart"
-	gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+	gdb_breakpoint "end" qualified
 	gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
 
 	gdb_test "tstatus" ".*buffer was full.*"
diff --git a/gdb/testsuite/gdb.trace/strace.exp b/gdb/testsuite/gdb.trace/strace.exp
index 124deb6f7f..4b70674491 100644
--- a/gdb/testsuite/gdb.trace/strace.exp
+++ b/gdb/testsuite/gdb.trace/strace.exp
@@ -185,7 +185,7 @@ proc strace_probe_marker { } {
 	gdb_test "info static-tracepoint-markers" \
 	    "ust/bar\[\t \]+y\[\t \]+$hex .*ust/bar2\[\t \]+y\[\t \]+$hex.*"
 
-	gdb_test "break end" "Breakpoint \[0-9\]+ at.*"
+	gdb_breakpoint "end" qualified
 
 	gdb_test_no_output "tstart"
 	gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
@@ -274,7 +274,7 @@ proc strace_trace_on_same_addr { type } {
 	    }
 	}
 
-	gdb_test "break end" "Breakpoint \[0-9\]+ at.*"
+	gdb_breakpoint "end" qualified
 
 	if [string equal $type "break"] {
 	    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
@@ -350,7 +350,7 @@ proc strace_trace_on_diff_addr { } {
 	gdb_test "trace *${marker_bar2_addr}" \
 	    "Tracepoint \[0-9\]+ at ${hex}: file.*"
 
-	gdb_test "break end" "Breakpoint \[0-9\]+ at.*"
+	gdb_breakpoint "end" qualified
 
 	gdb_test_no_output "tstart"
 	gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" \
diff --git a/gdb/testsuite/gdb.trace/tfind.exp b/gdb/testsuite/gdb.trace/tfind.exp
index 251877af37..c466862333 100644
--- a/gdb/testsuite/gdb.trace/tfind.exp
+++ b/gdb/testsuite/gdb.trace/tfind.exp
@@ -127,7 +127,7 @@ if { $return_me == 1 } then {
 # test tstatus (when trace on)
 gdb_test "tstatus" "\[Tt\]race is running.*" "test tstatus on"
 
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index 539fcb5218..84c4780ba2 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -53,9 +53,9 @@ proc break_trace_same_addr_1 { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "end" qualified
 
-    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "set_point" qualified
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
 
     gdb_test_no_output "tstart"
@@ -88,7 +88,7 @@ proc break_trace_same_addr_2 { trace_type1 trace_type2 option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "end" qualified
 
     gdb_test "${trace_type1} set_point" \
 	"\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
@@ -126,10 +126,10 @@ proc break_trace_same_addr_3 { trace_type option } \
     }
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
-    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "marker" qualified
+    gdb_breakpoint "end" qualified
 
-    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "set_point" qualified
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
 
     gdb_test_no_output "tstart"
@@ -164,10 +164,10 @@ proc break_trace_same_addr_4 { trace_type option } \
     }
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
-    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "marker" qualified
+    gdb_breakpoint "end" qualified
 
-    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "set_point" qualified
     gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
 
     gdb_test "continue" "Continuing\\.\[ \r\n\]+(Thread .* hit )?Breakpoint.*" \
@@ -207,8 +207,8 @@ proc break_trace_same_addr_5 { trace1 trace2 trace3 trace3_at_first_loc } \
 	return -1
     }
 
-    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "marker" qualified
+    gdb_breakpoint "end" qualified
 
     gdb_test "${trace1} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
 	"${trace1} set_point 1"
@@ -281,8 +281,8 @@ proc break_trace_same_addr_6 { trace1 enable1 trace2 enable2 } \
 	return -1
     }
 
-    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "marker" qualified
+    gdb_breakpoint "end" qualified
 
     gdb_test "continue" "Continuing\\.\[ \r\n\]+(Thread .* hit )?Breakpoint.*" \
 	"continue to marker"
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index 12a42ffb92..d23e6a3f5b 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -71,9 +71,9 @@ proc test_tracepoints { trace_command condition num_frames { kfail_proc 0 } } {
 	return 0
     }
 
-    gdb_test "break begin" ".*" ""
+    gdb_breakpoint "begin" qualified
 
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "end" qualified
 
     with_test_prefix "${trace_command}: ${condition}" {
 
diff --git a/gdb/testsuite/gdb.trace/trace-mt.exp b/gdb/testsuite/gdb.trace/trace-mt.exp
index f327406a13..6e18666111 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.exp
+++ b/gdb/testsuite/gdb.trace/trace-mt.exp
@@ -81,7 +81,7 @@ proc break_trace_same_addr { trace_type option } \
 
     gdb_test_no_output "set breakpoint always-inserted ${option}"
 
-    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_breakpoint "end" qualified
 
     gdb_test "break set_point1" "Breakpoint \[0-9\] at $hex: file.*"
     gdb_test "${trace_type} set_point1" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
diff --git a/gdb/testsuite/gdb.trace/tstatus.exp b/gdb/testsuite/gdb.trace/tstatus.exp
index d24c03738a..3498b1c13c 100644
--- a/gdb/testsuite/gdb.trace/tstatus.exp
+++ b/gdb/testsuite/gdb.trace/tstatus.exp
@@ -119,9 +119,9 @@ proc run_trace_experiment {} {
 proc test_tracepoints {} {
     global gdb_prompt
 
-    gdb_test "break begin" ".*" ""
+    gdb_breakpoint "begin" qualified
 
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "end" qualified
 
     gdb_test "trace gdb_c_test" "Tracepoint .*" \
 	"tracepoint at gdb_c_test"
diff --git a/gdb/testsuite/gdb.trace/tsv.exp b/gdb/testsuite/gdb.trace/tsv.exp
index e08c668c50..a86919a75d 100644
--- a/gdb/testsuite/gdb.trace/tsv.exp
+++ b/gdb/testsuite/gdb.trace/tsv.exp
@@ -136,7 +136,7 @@ gdb_test "print \$tvar5" " = 15" \
     "Print a trace state variable at start of run"
 
 # Be sure not to fall off the end of the program.
-gdb_test "break end" ".*" ""
+gdb_breakpoint "end" qualified
 gdb_test "continue" \
     "Continuing.*Breakpoint $decimal, end.*" \
     "run trace experiment"
diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index 99f4fbe05c..9fd3a739a7 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -303,7 +303,7 @@ if ![gdb_target_supports_trace] {
     return -1
 }
 
-gdb_breakpoint "end"
+gdb_breakpoint "end" qualified
 
 with_test_prefix "tracing foo" {
     gdb_test "trace *foo_start_lbl" ".*"
diff --git a/gdb/testsuite/gdb.trace/unavailable.exp b/gdb/testsuite/gdb.trace/unavailable.exp
index 79aa8ef1b0..181c9af8aa 100644
--- a/gdb/testsuite/gdb.trace/unavailable.exp
+++ b/gdb/testsuite/gdb.trace/unavailable.exp
@@ -61,8 +61,8 @@ proc prepare_for_trace_test {} {
 
     runto_main
 
-    gdb_test "break begin" ".*" ""
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "begin" qualified
+    gdb_breakpoint "end" qualified
 }
 
 proc run_trace_experiment { test_func } {
diff --git a/gdb/testsuite/gdb.trace/while-dyn.exp b/gdb/testsuite/gdb.trace/while-dyn.exp
index 6728aeda64..0fb6b47b0b 100644
--- a/gdb/testsuite/gdb.trace/while-dyn.exp
+++ b/gdb/testsuite/gdb.trace/while-dyn.exp
@@ -72,7 +72,7 @@ proc test_while_stepping { while_stepping } {
 
     gdb_test "tstart" ".*" ""
 
-    gdb_test "break end" ".*" ""
+    gdb_breakpoint "end" qualified
     gdb_test "continue" \
 	"Continuing.*Breakpoint $decimal, end.*" \
 	"$while_stepping: run trace experiment"
-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-16 19:47     ` Andreas Arnez
@ 2018-03-16 21:57       ` Simon Marchi
  2018-03-19 12:15         ` Andreas Arnez
  2018-03-26 14:44       ` Pedro Alves
  1 sibling, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2018-03-16 21:57 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches

On 2018-03-16 03:47 PM, Andreas Arnez wrote:
> On Thu, Mar 15 2018, Simon Marchi wrote:
> 
>> On 2018-03-14 12:11 PM, Andreas Arnez wrote:
>>> Some of GDB's trace test cases define a function end() and place a
>>> breakpoint there with "break end".  However, when libinproctrace is linked
>>> to the binary, there are multiple methods named "end", such as
>>> std::string::end() from the C++ library or format_pieces::end() from
>>> common/format.h.  GDB then creates multiple breakpoints instead of just a
>>> single one, and some FAILs result, such as these:
>>>
>>>   FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
>>>   FAIL: gdb.trace/trace-mt.exp: ftrace off: break end
>>>
>>> This is fixed by renaming end() to my_end().  For consistency, where end()
>>> was paired with a previous begin(), the latter is renamed to my_begin() as
>>> well.
>>>
>>> gdb/testsuite/ChangeLog:
>>>
>>> 	* gdb.trace/trace-break.c (end): Rename to...
>>> 	(my_end): ...this.
>>> 	(main): Adjust call.
>>> 	* gdb.trace/trace-mt.c (end): Rename to...
>>> 	(my_end): ...this.
>>> 	(main): Adjust call.
>>> 	* gdb.trace/unavailable.cc (begin): Rename to...
>>> 	(my_begin): ...this.
>>> 	(end): Rename to...
>>> 	(my_end): ...this.
>>> 	(main): Adjust calls.
>>> 	* gdb.trace/trace-break.exp: Reflect function name change.
>>> 	* gdb.trace/trace-mt.exp: Likewise.
>>> 	* gdb.trace/unavailable.exp: Likewise.
>>
>> Hi Andreas,
>>
>> Another way would have been to pass "-qualified" to the break command,
>> or use gdb_breakpoint and pass the "qualified" arg.  There are also
>> other instances of the same "problem" in tests that don't check the
>> output of the break command at all...
>>
>>   gdb_test "break end" ".*" ""
>>
>> In any case, this patch LGTM.
> 
> Hm, actually the use of "-qualified" is a good point.  I'd rather go
> with that.  How about the patch below?
> 
> --
> Andreas
> 
> -- >8 --
> Subject: [PATCH] Testsuite: Fix ambiguous "break" due to libinproctrace
> 
> Some of GDB's trace test cases define a function end() and place a
> breakpoint there with "break end".  However, when libinproctrace is linked
> to the binary, there are multiple methods named "end", such as
> std::string::end() from the C++ library or format_pieces::end() from
> common/format.h.  GDB then creates multiple breakpoints instead of just a
> single one, and some FAILs result, such as these:
> 
>   FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
>   FAIL: gdb.trace/trace-mt.exp: ftrace off: break end
> 
> Fix this by adding the "-qualified" option to the break commands.  For
> consistency, change all occurrences of "break end" (and similar) in all
> trace test cases, even if the current behavior does not cause problems.
> Also, consequently use the gdb_breakpoint convenience proc.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/actions-changed.exp: Call gdb_breakpoint with the
> 	"qualified" option when setting breakpoints.
> 	* gdb.trace/backtrace.exp: Likewise.
> 	* gdb.trace/circ.exp: Likewise.
> 	* gdb.trace/collection.exp: Likewise.
> 	* gdb.trace/disconnected-tracing.exp: Likewise.
> 	* gdb.trace/ftrace-lock.exp: Likewise.
> 	* gdb.trace/ftrace.exp: Likewise.
> 	* gdb.trace/infotrace.exp: Likewise.
> 	* gdb.trace/packetlen.exp: Likewise.
> 	* gdb.trace/passc-dyn.exp: Likewise.
> 	* gdb.trace/qtro.exp: Likewise.
> 	* gdb.trace/read-memory.exp: Likewise.
> 	* gdb.trace/report.exp: Likewise.
> 	* gdb.trace/signal.exp: Likewise.
> 	* gdb.trace/status-stop.exp: Likewise.
> 	* gdb.trace/strace.exp: Likewise.
> 	* gdb.trace/tfind.exp: Likewise.
> 	* gdb.trace/trace-break.exp: Likewise.
> 	* gdb.trace/trace-condition.exp: Likewise.
> 	* gdb.trace/trace-mt.exp: Likewise.
> 	* gdb.trace/tstatus.exp: Likewise.
> 	* gdb.trace/tsv.exp: Likewise.
> 	* gdb.trace/unavailable-dwarf-piece.exp: Likewise.
> 	* gdb.trace/unavailable.exp: Likewise.
> 	* gdb.trace/while-dyn.exp: Likewise.

Hi Andreas,

Since the gdb_breakpoint proc accepts the multi-location printout, we may
not even need the "qualified", just using gdb_breakpoint would be sufficient.
But I think it's not bad either to use "qualified", as it makes the test more
robust.  It makes sure the test doesn't stop on another random end function/method.

So this LGTM, thanks.

Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-16 21:57       ` Simon Marchi
@ 2018-03-19 12:15         ` Andreas Arnez
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Arnez @ 2018-03-19 12:15 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Fri, Mar 16 2018, Simon Marchi wrote:

> Since the gdb_breakpoint proc accepts the multi-location printout, we may
> not even need the "qualified", just using gdb_breakpoint would be sufficient.
> But I think it's not bad either to use "qualified", as it makes the test more
> robust.  It makes sure the test doesn't stop on another random end function/method.

Right, these random breakpoints could hit us badly in the future.

> So this LGTM, thanks.

Thanks, pushed.

--
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash
  2018-03-16 19:47     ` Andreas Arnez
  2018-03-16 21:57       ` Simon Marchi
@ 2018-03-26 14:44       ` Pedro Alves
  1 sibling, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2018-03-26 14:44 UTC (permalink / raw)
  To: Andreas Arnez, Simon Marchi; +Cc: gdb-patches

On 03/16/2018 07:47 PM, Andreas Arnez wrote:

> Subject: [PATCH] Testsuite: Fix ambiguous "break" due to libinproctrace
> 
> Some of GDB's trace test cases define a function end() and place a
> breakpoint there with "break end".  However, when libinproctrace is linked
> to the binary, there are multiple methods named "end", such as
> std::string::end() from the C++ library or format_pieces::end() from
> common/format.h.  GDB then creates multiple breakpoints instead of just a
> single one, and some FAILs result, such as these:
> 
>   FAIL: gdb.trace/trace-mt.exp: ftrace on: break end
>   FAIL: gdb.trace/trace-mt.exp: ftrace off: break end
> 
> Fix this by adding the "-qualified" option to the break commands.  For
> consistency, change all occurrences of "break end" (and similar) in all
> trace test cases, even if the current behavior does not cause problems.
> Also, consequently use the gdb_breakpoint convenience proc.
I agree this is the right patch for now.  I actually posted something
like this a while ago, but forgot to push it in:

 https://sourceware.org/ml/gdb-patches/2017-12/msg00123.html

Note that I think it's a design flaw that users can run into this, though.
I had filed:

  https://sourceware.org/bugzilla/show_bug.cgi?id=22560

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver
  2018-03-15 22:17   ` Simon Marchi
@ 2018-04-25  9:32     ` Andreas Arnez
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Arnez @ 2018-04-25  9:32 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Sorry for the delay...

On Thu, Mar 15 2018, Simon Marchi wrote:

> Hi Andreas,
>
> On 2018-03-14 12:11 PM, Andreas Arnez wrote:
>> The attach.exp test case yields a FAIL with native-extended-gdbserver when
>> trying to start a new process.  This is because gdbserver does not support
>> starting new processes.  And since the gdbserver-base board file sets the
>> GDB command line option -ex "set auto-connect-native-target off", the
>> process is not started on the native target either.  An error message
>> results instead:
>> 
>>   Don't know how to run.  Try "help target".
>> 
>> Thus just accept this error when not running on a native target.
>
> It is not true that gdbserver can't run a new process, in fact it can in the
> extended-remote protocol (as opposed to remote).  Start gdbserver with:
>
>  $ ./gdbserver/gdbserver --once --multi :1234
>
> And
>
>  $ ./gdb --data-directory=data-directory
>  (gdb) tar ext :1234
>  (gdb) set remote exec-file /usr/bin/gnome-calculator
>  (gdb) run

Thanks, you're right, of course.

What really happens here is that attach.exp tries to start GDB with the
command line options "--pid=<pid>" and "-ex start", using
gdb_spawn_with_cmdline_opts.  However, gdb_spawn_with_cmdline_opts is
not overridden in native-extended-gdbserver.exp.  Thus we just fire up
GDB without setting an extended-remote target, and without having
started gdbserver.  And due to "auto-connect-native-target off", the
"start" command then responds with the error message above.

The test suite contains some other "gdb_spawn*" invocations, all having
the same problem.  To fix this, we should override these procs.

>
> Still, I first thought this test wouldn't be applicable to the extended-remote
> protocol because I though you couldn't mix -p with connecting to a remote target
> on startup, but it seems like this works:
>
>  $ pidof gnome-calculator
>  20001
>  $ ./gdb --data-directory=data-directory -iex "tar ext :1234" -p 20001  -ex "set remote exec-file /usr/bin/gnome-calculator" -ex "start"
>
> So it might be possible to tweak the test case to adapt it to the extended-remote
> protocol (add the -iex and "set remote exec-file" bits).

Actually, after investigating this a bit further I wonder why "set
remote exec-file" should even be necessary.  In the native case GDB
takes care of this for the user, if possible.  And this is exactly what
the test case tries to verify here.  IMHO extended-remote should behave
the same.  So I think the current behavior is a usability bug and should
be fixed.

I've been working on some patches for this.  I'll send them around
later.

--
Andreas

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-04-25  9:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 16:12 [PATCH 0/3] Some gdbserver test suite fixes Andreas Arnez
2018-03-14 16:13 ` [PATCH 3/3] Fix a FAIL in attach.exp under native-extended-gdbserver Andreas Arnez
2018-03-15 22:17   ` Simon Marchi
2018-04-25  9:32     ` Andreas Arnez
2018-03-14 16:13 ` [PATCH 1/3] Fix tspeed test case: copy libinproctrace to target Andreas Arnez
2018-03-15 21:40   ` Simon Marchi
2018-03-16 19:41     ` Andreas Arnez
2018-03-14 16:13 ` [PATCH 2/3] Testsuite: Rename "end()" to avoid libinproctrace C++ symbol clash Andreas Arnez
2018-03-15 21:58   ` Simon Marchi
2018-03-16 19:47     ` Andreas Arnez
2018-03-16 21:57       ` Simon Marchi
2018-03-19 12:15         ` Andreas Arnez
2018-03-26 14:44       ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).