public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Make gdb_test's question non-optional if specified
@ 2022-03-30 19:29 Pedro Alves
  2022-03-30 19:29 ` [PATCH 1/5] Remove gdb_test questions that GDB doesn't ask Pedro Alves
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

gdb_test supports handling scenarios where GDB asks a question before
finishing handling some command.  The full prototype of gdb_test is:

  # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE

However, QUESTION is a question that GDB _may_ ask, not one that GDB
_must_ ask:

 # QUESTION is a question GDB may ask in response to COMMAND, like
 #   "are you sure?"
 # RESPONSE is the response to send if QUESTION appears.

If GDB doesn't raise the question, the test still passes.

I think that this is a misfeature.  If GDB regresses and stops asking
a question, the testsuite won't notice.  So I think that if a QUESTION
is specified, gdb_test should ensure it comes out of GDB.

Running the testsuite exposes a number of tests that pass
QUESTION/RESPONSE to GDB, but no question comes out.  This series
fixes them all.

A related issue is that gdb_test doesn't enforce that if you specify
QUESTION, that you also specify RESPONSE.  I.e., you should pass 1, 2,
3, or 5 arguments to gdb_test, but never 4, or more than 5.  Making
gdb_test detect bogus arguments actually regresses some testcases,
exposing test bugs, also fixed in this series.

gdb_test's behavior is changed in the last patch of the series.

Pedro Alves (5):
  Remove gdb_test questions that GDB doesn't ask
  gdb.base/scope.exp: Remove bogus gdb_test questions
  Fix bogus gdb_test invocations
  Avoid having to unload file in
    gdb.server/connect-with-no-symbol-file.exp
  Make gdb_test's question non-optional if specified

 gdb/testsuite/gdb.base/auxv.exp               |  3 +-
 gdb/testsuite/gdb.base/catch-fork-kill.exp    |  3 +-
 gdb/testsuite/gdb.base/default.exp            |  2 +-
 gdb/testsuite/gdb.base/pointers.exp           |  2 +-
 gdb/testsuite/gdb.base/scope.exp              | 60 +++++++++----------
 gdb/testsuite/gdb.base/style.exp              |  4 +-
 gdb/testsuite/gdb.multi/tids.exp              |  2 +-
 gdb/testsuite/gdb.python/py-parameter.exp     |  2 +-
 gdb/testsuite/gdb.python/py-xmethods.exp      |  1 +
 .../connect-with-no-symbol-file.exp           | 17 +++---
 gdb/testsuite/gdb.server/solib-list.exp       |  3 +-
 gdb/testsuite/lib/gdb.exp                     | 25 ++++++--
 12 files changed, 67 insertions(+), 57 deletions(-)


base-commit: c50e54825bfea1ab6afbd984a4d2e78e9306e70f
-- 
2.26.2


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

* [PATCH 1/5] Remove gdb_test questions that GDB doesn't ask
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
@ 2022-03-30 19:29 ` Pedro Alves
  2022-03-30 19:29 ` [PATCH 2/5] gdb.base/scope.exp: Remove bogus gdb_test questions Pedro Alves
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

Change-Id: Ib2616dc883e9dc9ee100f6c86d83a921a0113c16
---
 gdb/testsuite/gdb.base/auxv.exp            | 3 +--
 gdb/testsuite/gdb.base/catch-fork-kill.exp | 3 +--
 gdb/testsuite/gdb.base/default.exp         | 2 +-
 gdb/testsuite/gdb.base/style.exp           | 4 +---
 gdb/testsuite/gdb.multi/tids.exp           | 2 +-
 gdb/testsuite/gdb.server/solib-list.exp    | 3 +--
 6 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.base/auxv.exp b/gdb/testsuite/gdb.base/auxv.exp
index c5384a51b93..390f559a4a3 100644
--- a/gdb/testsuite/gdb.base/auxv.exp
+++ b/gdb/testsuite/gdb.base/auxv.exp
@@ -158,8 +158,7 @@ proc do_core_test {works corefile test1 test2} {
 	unsupported $test2
     } else {
 	gdb_test "core $corefile" "Core was generated by.*" \
-	    "load core file for $test1" \
-	    "A program is being debugged already.*" "y"
+	    "load core file for $test1"
 	set core_data [fetch_auxv $test1]
 	global live_data
 	if {$core_data == $live_data} {
diff --git a/gdb/testsuite/gdb.base/catch-fork-kill.exp b/gdb/testsuite/gdb.base/catch-fork-kill.exp
index 98a6c66db3d..67fb84706a0 100644
--- a/gdb/testsuite/gdb.base/catch-fork-kill.exp
+++ b/gdb/testsuite/gdb.base/catch-fork-kill.exp
@@ -74,8 +74,7 @@ proc do_test {fork_kind exit_kind} {
 	    "Catchpoint \[0-9\]* \\(${fork_kind}ed process \[0-9\]*\\),.*" \
 	    "continue to grandchild ${fork_kind}"
 
-	gdb_test "kill inferior 2" "" "kill child" \
-	    "Kill the program being debugged.*y or n. $" "y"
+	gdb_test "kill inferior 2" "" "kill child"
 
 	gdb_test "inferior 1" "Switching to inferior 1 .*" "switch to parent"
 
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index f7859c9a479..23379f6ec01 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -455,7 +455,7 @@ gdb_test "restore" "You can't do that without a process to debug\."
 #test return
 # The middle case accomodated the obsolete a29k, where doing the "ni"
 # above causes an initial stack to be created.
-gdb_test "return" "No selected frame..*" "return"  "Make .* return now.*y or n. $" "y"
+gdb_test "return" "No selected frame..*"
 
 
 #test reverse-search
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 68196d6e3e2..c6d313274d6 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -279,9 +279,7 @@ proc run_style_tests { } {
 	}
 	gdb_test "file $binfile" \
 	    $pass_re \
-	    "filename is styled when loading symbol file" \
-	    "Are you sure you want to change the file.*" \
-	    "y"
+	    "filename is styled when loading symbol file"
 
 	gdb_test "pwd" "Working directory [limited_style .*? file].*"
 
diff --git a/gdb/testsuite/gdb.multi/tids.exp b/gdb/testsuite/gdb.multi/tids.exp
index d8ba3cae97d..fb7c2a29a71 100644
--- a/gdb/testsuite/gdb.multi/tids.exp
+++ b/gdb/testsuite/gdb.multi/tids.exp
@@ -428,7 +428,7 @@ if { ![skip_python_tests] } {
 # Remove the second inferior and confirm that GDB goes back to showing
 # single-number thread IDs.
 with_test_prefix "back to one inferior" {
-    gdb_test "kill inferior 2" "" "kill inferior 2" "Kill the program being debugged.*" "y"
+    gdb_test "kill inferior 2" "" "kill inferior 2"
     gdb_test "thread 1.1" "Switching to thread 1\.1 .*"
     gdb_test "remove-inferior 2" ".*" "remove inferior 2"
 
diff --git a/gdb/testsuite/gdb.server/solib-list.exp b/gdb/testsuite/gdb.server/solib-list.exp
index 117cb682216..db549d1ce15 100644
--- a/gdb/testsuite/gdb.server/solib-list.exp
+++ b/gdb/testsuite/gdb.server/solib-list.exp
@@ -92,8 +92,7 @@ foreach nonstop { 0 1 } { with_test_prefix "non-stop $nonstop" {
     # but before "target remote" below so that qSymbol data get already
     # initialized from BINFILE (and not from ld.so first needing a change to
     # BINFILE later).
-    gdb_test "file ${binfile}" {Reading symbols from .*\.\.\..*} "file binfile" \
-	     {(Are you sure you want to change the file|Load new symbol table from ".*")\? \(y or n\) } "y"
+    gdb_test "file ${binfile}" {Reading symbols from .*\.\.\..*} "file binfile"
 
     set test "target $gdbserver_protocol"
     set ok 0
-- 
2.26.2


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

* [PATCH 2/5] gdb.base/scope.exp: Remove bogus gdb_test questions
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
  2022-03-30 19:29 ` [PATCH 1/5] Remove gdb_test questions that GDB doesn't ask Pedro Alves
@ 2022-03-30 19:29 ` Pedro Alves
  2022-03-30 19:29 ` [PATCH 3/5] Fix bogus gdb_test invocations Pedro Alves
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

This test is abusing the QUESTION/RESPONSE feature to send an
alternative command to GDB if the first command fails.  Like so:

   gdb_test "print 'scope0.c'::filelocal" \
	    "\\\$$decimal = 1" "print 'scope0.c'::filelocal at main" \
	    "No symbol \"scope0.c\" in current context.*" \
	    "print '$srcdir/$subdir/scope0.c'::filelocal"

So if 'scope0.c' doesn't work, we try again with
'$srcdir/$subdir/scope0.c'.  I strongly suspect this is really an
obsolete test.  I think that if '$srcdir/$subdir/scope0.c' works, then
'scope0.c' should have worked too, thus I'd think that if we pass due
to the question path, then it's a bug.  So just remove the question
part passed to gdb_test.

Change-Id: I2acc99285f1d519284051b49693b5441fbdfe3cd
---
 gdb/testsuite/gdb.base/scope.exp | 60 ++++++++++++++++----------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/gdb/testsuite/gdb.base/scope.exp b/gdb/testsuite/gdb.base/scope.exp
index 19a85bcd244..a8ddb61350a 100644
--- a/gdb/testsuite/gdb.base/scope.exp
+++ b/gdb/testsuite/gdb.base/scope.exp
@@ -49,11 +49,11 @@ proc_with_prefix test_at_main {} {
 
     # Print scope0.c::filelocal, which is 1
     gdb_test "print filelocal" "\\\$$decimal = 1"
-    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at main"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal"
+    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at main"
 
     # Print scope0.c::filelocal_bss, which is 101
     gdb_test "print filelocal_bss" "\\\$$decimal = 101"
-    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_main"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_bss"
+    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_main"
 
     # Print scope0.c::filelocal_ro, which is 201
 
@@ -62,37 +62,37 @@ proc_with_prefix test_at_main {} {
     gdb_test "print filelocal_ro" "\\\$$decimal = 201" "print filelocal_ro in test_at_main"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_ro"
+    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro"
 
     # Print scope1.c::filelocal, which is 2
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal"
+    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal"
 
     # Print scope1.c::filelocal_bss, which is 102
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_bss"
+    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss"
 
     # Print scope1.c::filelocal_ro, which is 202
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_ro"
+    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro"
 
     # Print scope1.c::foo::funclocal, which is 3
     gdb_test "print foo::funclocal" "\\\$$decimal = 3"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal"
+    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal"
 
     # Print scope1.c::foo::funclocal_ro, which is 203
     gdb_test "print foo::funclocal_ro" "\\\$$decimal = 203"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal_ro"
+    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro"
 
     # Print scope1.c::bar::funclocal, which is 4
     gdb_test "print bar::funclocal" "\\\$$decimal = 4"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::bar::funclocal"
+    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal"
 }
 
 proc_with_prefix test_at_foo {} {
@@ -105,34 +105,34 @@ proc_with_prefix test_at_foo {} {
     gdb_test "next" ".*bar \\(\\);"
 
     # Print scope0.c::filelocal, which is 1
-    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at foo"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal"
+    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at foo"
 
     # Print scope0.c::filelocal_bss, which is 101
-    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_foo"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_bss"
+    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_foo"
 
     # Print scope0.c::filelocal_ro, which is 201
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_ro"
+    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro"
 
 
     # Print scope1.c::filelocal, which is 2
     gdb_test "print filelocal" "\\\$$decimal = 2" "print filelocal at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal"
+    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal at foo"
 
     gdb_test "print filelocal_bss" "\\\$$decimal = 102" \
 	"print filelocal_bss at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_bss"
+    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss at foo"
 
 
     gdb_test "print filelocal_ro" "\\\$$decimal = 202" \
 	"print filelocal_ro at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_ro"
+    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro at foo"
 
 
     # Print scope1.c::foo::funclocal, which is 3
@@ -143,7 +143,7 @@ proc_with_prefix test_at_foo {} {
 	"print foo::funclocal at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal"
+    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal at foo"
 
 
     # Print scope1.c::foo::funclocal_bss, which is 103
@@ -155,7 +155,7 @@ proc_with_prefix test_at_foo {} {
 	"print foo::funclocal_bss at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal_bss" "\\\$$decimal = 103" "print 'scope1.c'::foo::funclocal_bss at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal_bss"
+    gdb_test "print 'scope1.c'::foo::funclocal_bss" "\\\$$decimal = 103" "print 'scope1.c'::foo::funclocal_bss at foo"
 
 
     # Print scope1.c::foo::funclocal_ro, which is 203
@@ -167,7 +167,7 @@ proc_with_prefix test_at_foo {} {
 	"print foo::funclocal_ro at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal_ro"
+    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro at foo"
 
 
     # Print scope1.c::bar::funclocal, which is 4
@@ -176,7 +176,7 @@ proc_with_prefix test_at_foo {} {
 	"print bar::funclocal at foo"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal at foo"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::bar::funclocal"
+    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal at foo"
 
 }
 
@@ -190,64 +190,64 @@ proc_with_prefix test_at_bar {} {
     gdb_test "next"
 
     # Print scope0.c::filelocal, which is 1
-    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at bar"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal"
+    gdb_test "print 'scope0.c'::filelocal" "\\\$$decimal = 1" "print 'scope0.c'::filelocal at bar"
 
     # Print scope0.c::filelocal_bss, which is 101
-    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_bar"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_bss"
+    gdb_test "print 'scope0.c'::filelocal_bss" "\\\$$decimal = 101" "print 'scope0.c'::filelocal_bss in test_at_bar"
 
     # Print scope0.c::filelocal_ro, which is 201
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro at bar"  "No symbol \"scope0.c\" in current context.*" "print '$srcdir/$subdir/scope0.c'::filelocal_ro"
+    gdb_test "print 'scope0.c'::filelocal_ro" "\\\$$decimal = 201" "print 'scope0.c'::filelocal_ro at bar"
 
     # Print scope1.c::filelocal, which is 2
     gdb_test "print filelocal" "\\\$$decimal = 2" "print filelocal at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal"
+    gdb_test "print 'scope1.c'::filelocal" "\\\$$decimal = 2" "print 'scope1.c'::filelocal at bar"
 
     # Print scope1.c::filelocal_bss, which is 102
     gdb_test "print filelocal_bss" "\\\$$decimal = 102" "print filelocal_bss at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_bss"
+    gdb_test "print 'scope1.c'::filelocal_bss" "\\\$$decimal = 102" "print 'scope1.c'::filelocal_bss at bar"
 
     # Print scope1.c::filelocal_ro, which is 202
     gdb_test "print filelocal_ro" "\\\$$decimal = 202" "print filelocal_ro in test_at_bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::filelocal_ro"
+    gdb_test "print 'scope1.c'::filelocal_ro" "\\\$$decimal = 202" "print 'scope1.c'::filelocal_ro at bar"
 
     # Print scope1.c::foo::funclocal, which is 3
     gdb_test "print foo::funclocal" "\\\$$decimal = 3" "print foo::funclocal at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal"
+    gdb_test "print 'scope1.c'::foo::funclocal" "\\\$$decimal = 3" "print 'scope1.c'::foo::funclocal at bar"
 
     # Print scope1.c::foo::funclocal_bss, which is 103
     gdb_test "print foo::funclocal_bss" "\\\$$decimal = 103" "print foo::funclocal_bss at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal_bss" "\\\$$decimal = 103" "print 'scope1.c'::foo::funclocal_bss at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal_bss"
+    gdb_test "print 'scope1.c'::foo::funclocal_bss" "\\\$$decimal = 103" "print 'scope1.c'::foo::funclocal_bss at bar"
 
     # Print scope1.c::foo::funclocal_ro, which is 203
     gdb_test "print foo::funclocal_ro" "\\\$$decimal = 203" "print foo::funclocal_ro at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::foo::funclocal_ro"
+    gdb_test "print 'scope1.c'::foo::funclocal_ro" "\\\$$decimal = 203" "print 'scope1.c'::foo::funclocal_ro at bar"
 
     # Print scope1.c::bar::funclocal, which is 4
     gdb_test "print funclocal" "\\\$$decimal = 4" "print funclocal at bar"
     gdb_test "print bar::funclocal" "\\\$$decimal = 4" "print bar::funclocal at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::bar::funclocal"
+    gdb_test "print 'scope1.c'::bar::funclocal" "\\\$$decimal = 4" "print 'scope1.c'::bar::funclocal at bar"
 
     # Print scope1.c::bar::funclocal_bss, which is 104
     gdb_test "print funclocal_bss" "\\\$$decimal = 104" "print funclocal_bss at bar"
     gdb_test "print bar::funclocal_bss" "\\\$$decimal = 104" "print bar::funclocal_bss at bar"
 
     if { [test_compiler_info gcc-*-*] } then { setup_xfail "rs6000-*-*" }
-    gdb_test "print 'scope1.c'::bar::funclocal_bss" "\\\$$decimal = 104" "print 'scope1.c'::bar::funclocal_bss at bar"  "No symbol \"scope1.c\" in current context.*" "print '$srcdir/$subdir/scope1.c'::bar::funclocal_bss"
+    gdb_test "print 'scope1.c'::bar::funclocal_bss" "\\\$$decimal = 104" "print 'scope1.c'::bar::funclocal_bss at bar"
 }
 
 # This test has little to do with local scopes, but it is in scope.exp anyway.
-- 
2.26.2


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

* [PATCH 3/5] Fix bogus gdb_test invocations
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
  2022-03-30 19:29 ` [PATCH 1/5] Remove gdb_test questions that GDB doesn't ask Pedro Alves
  2022-03-30 19:29 ` [PATCH 2/5] gdb.base/scope.exp: Remove bogus gdb_test questions Pedro Alves
@ 2022-03-30 19:29 ` Pedro Alves
  2022-03-30 19:29 ` [PATCH 4/5] Avoid having to unload file in gdb.server/connect-with-no-symbol-file.exp Pedro Alves
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

A following patch will make gdb_test error out if bogus arguments are
passed, which exposed bugs in a few testcases:

 - gdb.python/py-parameter.exp, passing a spurious "1" as extra
   parameter, resulting in:

   ERROR: Unexpected arguments: {set test-file-param bar.txt} {The name of the file has been changed to bar.txt} {set new file parameter} 1

 - gdb.python/py-xmethods.exp, a missing test message, resulting in
   the next gdb_test being interpreted as message, question and
   response!  With the enforcing patch, this was caught with:

   ERROR: Unexpected arguments: {p g.mul<char>('a')} {From Python G<>::mul.*} gdb_test {p g_ptr->mul<char>('a')} {From Python G<>::mul.*} {after: g_ptr->mul<char>('a')}

 - gdb.base/pointers.exp, missing a quote.

Change-Id: I66f2db4412025a64121db7347dfb0b48240d46d4
---
 gdb/testsuite/gdb.base/pointers.exp       | 2 +-
 gdb/testsuite/gdb.python/py-parameter.exp | 2 +-
 gdb/testsuite/gdb.python/py-xmethods.exp  | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/pointers.exp b/gdb/testsuite/gdb.base/pointers.exp
index bca8377637b..db60b2bf540 100644
--- a/gdb/testsuite/gdb.base/pointers.exp
+++ b/gdb/testsuite/gdb.base/pointers.exp
@@ -209,7 +209,7 @@ gdb_test "print **ptr_to_ptr_to_float" " = 100" \
 
 gdb_test "break marker1" ".*" ""
 gdb_test "cont" "Break.* marker1 \\(\\) at .*:$decimal.*" \
-    continue to marker1"
+    "continue to marker1"
 gdb_test "up" "more_code.*" "up from marker1"
 
 gdb_test "print *pUC" " = 21 \'.025\'.*" "print value of *pUC"
diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
index 98d4b2d4684..199d3bc16ec 100644
--- a/gdb/testsuite/gdb.python/py-parameter.exp
+++ b/gdb/testsuite/gdb.python/py-parameter.exp
@@ -199,7 +199,7 @@ proc_with_prefix test_file_parameter { } {
 	"The name of the file is foo.txt.*" "show initial file value"
     gdb_test "set test-file-param bar.txt" \
 	"The name of the file has been changed to bar.txt" \
-	"set new file parameter" 1
+	"set new file parameter"
     gdb_test "show test-file-param" \
 	"The name of the file is bar.txt.*" "show new file value"
     gdb_test "python print (test_file_param.value)" \
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index f879e9c15a2..18057782143 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -130,6 +130,7 @@ gdb_test "p g.size_mul<  5  >()" "From Python G<>::size_mul.*" \
 gdb_test "p g.mul<double>(2.0)" "From Python G<>::mul.*" \
   "after: g.mul<double>(2.0)"
 gdb_test "p g.mul<char>('a')" "From Python G<>::mul.*" \
+  "after: g.mul<char>('a')"
 gdb_test "p g_ptr->mul<char>('a')" "From Python G<>::mul.*" \
   "after: g_ptr->mul<char>('a')"
 
-- 
2.26.2


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

* [PATCH 4/5] Avoid having to unload file in gdb.server/connect-with-no-symbol-file.exp
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
                   ` (2 preceding siblings ...)
  2022-03-30 19:29 ` [PATCH 3/5] Fix bogus gdb_test invocations Pedro Alves
@ 2022-03-30 19:29 ` Pedro Alves
  2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
  2022-05-16 16:02 ` [PATCH 0/5] " Tom Tromey
  5 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

gdb.server/connect-with-no-symbol-file.exp's connect_no_symbol_file
does:

	gdb_test "file" ".*" "discard symbol table" \
	    {Discard symbol table from `.*'\? \(y or n\) } "y"

A following patch will make gdb_test expect the question out of GDB if
one is passed down as argument to gdb_test.  With that, this test
starts failing.  This is because connect_no_symbol_file is called in a
loop, and the first time around, there's a loaded file, so "file" asks
the "Discard symbol table ... ?" question, while in the following
iterations there's no file, so there's no question.

Fix this by not loading a file into GDB in the first place.

Change-Id: I810c036b57842c4c5b47faf340466b0d446d1abc
---
 .../gdb.server/connect-with-no-symbol-file.exp  | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp
index af5917d9018..6c480c8c0cf 100644
--- a/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp
+++ b/gdb/testsuite/gdb.server/connect-with-no-symbol-file.exp
@@ -29,7 +29,7 @@ if { [skip_gdbserver_tests] } {
     return 0
 }
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+if { [build_executable "failed to prepare" $testfile $srcfile debug] } {
     return -1
 }
 
@@ -46,22 +46,16 @@ proc connect_no_symbol_file { sysroot action } {
 
     with_test_prefix "setup" {
 	# Copy the symbol file to the target.
-	gdb_remote_download target $binfile.bak $binfile 
+	set target_exec [gdb_remote_download target $binfile.bak $binfile]
 
 	# Make sure we're disconnected, in case we're testing with an
 	# extended-remote board, therefore already connected.
 	gdb_test "disconnect" ".*"
 
-	# Discard any symbol files that we have opened.
-	gdb_test "file" ".*" "discard symbol table" \
-	    {Discard symbol table from `.*'\? \(y or n\) } "y"
-
 	# Set sysroot to something non-target and possibly also invalid so that
 	# GDB is unable to open the symbol file.
 	gdb_test_no_output "set sysroot $sysroot" "adjust sysroot"
 
-	set target_exec [gdbserver_download_current_prog]
-
 	# Start GDBserver.
 	set res [gdbserver_start "" $target_exec]
 
@@ -70,9 +64,9 @@ proc connect_no_symbol_file { sysroot action } {
 
 	# Perform test actions to the symbol file on the target.
 	if { $action == "delete" } then {
-	  remote_file target delete $binfile
+	  remote_file target delete $target_exec
 	} elseif { $action == "permission" } {
-	  remote_spawn target "chmod 000 $binfile"
+	  remote_spawn target "chmod 000 $target_exec"
 	}
        
 	# Connect to GDBserver.
@@ -89,6 +83,9 @@ proc connect_no_symbol_file { sysroot action } {
 # Make sure we have the original symbol file in a safe place to copy from.
 gdb_remote_download host $binfile $binfile.bak
 
+# Start with no executable loaded.
+clean_restart
+
 # Run the test with different permutations.
 foreach_with_prefix sysroot {"" "target:"} {
     foreach_with_prefix action {"permission" "delete"} {
-- 
2.26.2


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

* [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
                   ` (3 preceding siblings ...)
  2022-03-30 19:29 ` [PATCH 4/5] Avoid having to unload file in gdb.server/connect-with-no-symbol-file.exp Pedro Alves
@ 2022-03-30 19:29 ` Pedro Alves
  2022-04-07 20:31   ` Bruno Larsen
                     ` (2 more replies)
  2022-05-16 16:02 ` [PATCH 0/5] " Tom Tromey
  5 siblings, 3 replies; 26+ messages in thread
From: Pedro Alves @ 2022-03-30 19:29 UTC (permalink / raw)
  To: gdb-patches

gdb_test supports handling scenarios where GDB asks a question before
finishing handling some command.  The full prototype of gdb_test is:

  # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE

However, QUESTION is a question that GDB _may_ ask, not one that GDB
_must_ ask:

 # QUESTION is a question GDB may ask in response to COMMAND, like
 #   "are you sure?"
 # RESPONSE is the response to send if QUESTION appears.

If GDB doesn't raise the question, the test still passes.

I think that this is a misfeature.  If GDB regresses and stops asking
a question, the testsuite won't notice.  So I think that if a QUESTION
is specified, gdb_test should ensure it comes out of GDB.

Running the testsuite exposed a number of tests that pass
QUESTION/RESPONSE to GDB, but no question comes out.  The previous
commits fixed them all, so this commit changes gdb_test's behavior.

A related issue is that gdb_test doesn't enforce that if you specify
QUESTION, that you also specify RESPONSE.  I.e., you should pass 1, 2,
3, or 5 arguments to gdb_test, but never 4, or more than 5.  Making
gdb_test detect bogus arguments actually regressed some testcases,
also all fixed in previous commits.

Change-Id: I47c39c9034e6a6841129312037a5ca4c5811f0db
---
 gdb/testsuite/lib/gdb.exp | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0b242b64992..e9cbbe77fa4 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1324,9 +1324,10 @@ proc gdb_test_multiline { name args } {
 #   omitted, then the pass/fail messages use the command string as the
 #   message.  (If this is the empty string, then sometimes we don't
 #   call pass or fail at all; I don't understand this at all.)
-# QUESTION is a question GDB may ask in response to COMMAND, like
-#   "are you sure?"
-# RESPONSE is the response to send if QUESTION appears.
+# QUESTION is a question GDB should ask in response to COMMAND, like
+#   "are you sure?"  If this is specified, the test fails if GDB
+#   doesn't print the question.
+# RESPONSE is the response to send when QUESTION appears.
 #
 # Returns:
 #    1 if the test failed,
@@ -1337,6 +1338,11 @@ proc gdb_test { args } {
     global gdb_prompt
     upvar timeout timeout
 
+    # Can't have a question without a response.
+    if { [llength $args] == 4 || [llength $args] > 5 } {
+	error "Unexpected arguments: $args"
+    }
+
     if [llength $args]>2 then {
 	set message [lindex $args 2]
     } else {
@@ -1345,11 +1351,21 @@ proc gdb_test { args } {
     set command [lindex $args 0]
     set pattern [lindex $args 1]
 
+    set must_see_question 0
+    if { [llength $args] == 5 } {
+	set must_see_question 1
+	set saw_question 0
+    }
+
     set user_code {}
     lappend user_code {
 	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$gdb_prompt $" {
 	    if ![string match "" $message] then {
-		pass "$message"
+		if {$must_see_question} {
+		    gdb_assert $saw_question "$message"
+		} else {
+		    pass "$message"
+		}
             }
         }
     }
@@ -1359,6 +1375,7 @@ proc gdb_test { args } {
 	set response_string [lindex $args 4]
 	lappend user_code {
 	    -re "(${question_string})$" {
+		set saw_question 1
 		send_gdb "$response_string\n"
 		exp_continue
 	    }
-- 
2.26.2


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
@ 2022-04-07 20:31   ` Bruno Larsen
  2022-04-08 12:18     ` Pedro Alves
  2022-05-16 16:01   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Tom Tromey
  2022-05-17 11:41   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Simon Marchi
  2 siblings, 1 reply; 26+ messages in thread
From: Bruno Larsen @ 2022-04-07 20:31 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 3/30/22 16:29, Pedro Alves wrote:
> gdb_test supports handling scenarios where GDB asks a question before
> finishing handling some command.  The full prototype of gdb_test is:
> 
>    # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
> 
> However, QUESTION is a question that GDB _may_ ask, not one that GDB
> _must_ ask:
> 
>   # QUESTION is a question GDB may ask in response to COMMAND, like
>   #   "are you sure?"
>   # RESPONSE is the response to send if QUESTION appears.
> 
> If GDB doesn't raise the question, the test still passes.
> 
> I think that this is a misfeature.  If GDB regresses and stops asking
> a question, the testsuite won't notice.  So I think that if a QUESTION
> is specified, gdb_test should ensure it comes out of GDB.

I just ran into a neat use of this, or possibly a mis-use, depending on how much you like it. gdb.base/skip.exp uses:

gdb_test "step" "desired spot" "go to desired spot" "undesirable spot" "step"

as a simple way to handle a gcc 9.2.0 bug (misfeature) where gdb could stop in an undesirable spot without actually causing a failure in the test. This feels like a neat way to deal with compiler problems, as it would introduce a simple way to deal with clang's lack of epilogue, for instance.

I'm not suggesting that this patch be scrapped, but maybe this could be implemented on purpose, something like gdb_test_optional. The code itself LGTM, but I am not able to approve patches.

-- 
Cheers!
Bruno Larsen
> 
> Running the testsuite exposed a number of tests that pass
> QUESTION/RESPONSE to GDB, but no question comes out.  The previous
> commits fixed them all, so this commit changes gdb_test's behavior.
> 
> A related issue is that gdb_test doesn't enforce that if you specify
> QUESTION, that you also specify RESPONSE.  I.e., you should pass 1, 2,
> 3, or 5 arguments to gdb_test, but never 4, or more than 5.  Making
> gdb_test detect bogus arguments actually regressed some testcases,
> also all fixed in previous commits.
> 
> Change-Id: I47c39c9034e6a6841129312037a5ca4c5811f0db
> ---
>   gdb/testsuite/lib/gdb.exp | 25 +++++++++++++++++++++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0b242b64992..e9cbbe77fa4 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1324,9 +1324,10 @@ proc gdb_test_multiline { name args } {
>   #   omitted, then the pass/fail messages use the command string as the
>   #   message.  (If this is the empty string, then sometimes we don't
>   #   call pass or fail at all; I don't understand this at all.)
> -# QUESTION is a question GDB may ask in response to COMMAND, like
> -#   "are you sure?"
> -# RESPONSE is the response to send if QUESTION appears.
> +# QUESTION is a question GDB should ask in response to COMMAND, like
> +#   "are you sure?"  If this is specified, the test fails if GDB
> +#   doesn't print the question.
> +# RESPONSE is the response to send when QUESTION appears.
>   #
>   # Returns:
>   #    1 if the test failed,
> @@ -1337,6 +1338,11 @@ proc gdb_test { args } {
>       global gdb_prompt
>       upvar timeout timeout
>   
> +    # Can't have a question without a response.
> +    if { [llength $args] == 4 || [llength $args] > 5 } {
> +	error "Unexpected arguments: $args"
> +    }
> +
>       if [llength $args]>2 then {
>   	set message [lindex $args 2]
>       } else {
> @@ -1345,11 +1351,21 @@ proc gdb_test { args } {
>       set command [lindex $args 0]
>       set pattern [lindex $args 1]
>   
> +    set must_see_question 0
> +    if { [llength $args] == 5 } {
> +	set must_see_question 1
> +	set saw_question 0
> +    }
> +
>       set user_code {}
>       lappend user_code {
>   	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$gdb_prompt $" {
>   	    if ![string match "" $message] then {
> -		pass "$message"
> +		if {$must_see_question} {
> +		    gdb_assert $saw_question "$message"
> +		} else {
> +		    pass "$message"
> +		}
>               }
>           }
>       }
> @@ -1359,6 +1375,7 @@ proc gdb_test { args } {
>   	set response_string [lindex $args 4]
>   	lappend user_code {
>   	    -re "(${question_string})$" {
> +		set saw_question 1
>   		send_gdb "$response_string\n"
>   		exp_continue
>   	    }



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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-04-07 20:31   ` Bruno Larsen
@ 2022-04-08 12:18     ` Pedro Alves
  2022-05-17 10:13       ` [PATCH 5/6] gdb.base/skip.exp: Don't abuse gdb_test's question support (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2022-04-08 12:18 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

On 2022-04-07 21:31, Bruno Larsen wrote:
> On 3/30/22 16:29, Pedro Alves wrote:
>> gdb_test supports handling scenarios where GDB asks a question before
>> finishing handling some command.  The full prototype of gdb_test is:
>>
>>    # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
>>
>> However, QUESTION is a question that GDB _may_ ask, not one that GDB
>> _must_ ask:
>>
>>   # QUESTION is a question GDB may ask in response to COMMAND, like
>>   #   "are you sure?"
>>   # RESPONSE is the response to send if QUESTION appears.
>>
>> If GDB doesn't raise the question, the test still passes.
>>
>> I think that this is a misfeature.  If GDB regresses and stops asking
>> a question, the testsuite won't notice.  So I think that if a QUESTION
>> is specified, gdb_test should ensure it comes out of GDB.
> 
> I just ran into a neat use of this, or possibly a mis-use, depending on how much you like it. gdb.base/skip.exp uses:
> 
> gdb_test "step" "desired spot" "go to desired spot" "undesirable spot" "step"
> 
> as a simple way to handle a gcc 9.2.0 bug (misfeature) where gdb could stop in an undesirable spot without actually causing a failure in the test. This feels like a neat way to deal with compiler problems, as it would introduce a simple way to deal with clang's lack of epilogue, for instance.

This is:

    # With gcc 9.2.0 we jump once back to main before entering foo here.
    # If that happens try to step a second time.
    gdb_test "step" "foo \\(\\) at.*" "step 3" \
	"main \\(\\) at .*\r\n$gdb_prompt " "step"

I think this is a misuse.  That case doesn't really handle a GDB confirmation question.
What if you need to issue more steps, or the program may stop elsewhere for
different ports, and thus you need more than one regexp?  For the latter you can
use (REGEX1|REGEX1), but it isn't as nice as separate -re entries, IMHO.

IMHO, writing it with gdb_test_multiple in these cases is OK:

gdb_test_multiple "step" "step 3" {
  -re -wrap "foo \\(\\) at.*" {
     pass $gdb_test_multiple
  }
  -re -wrap "main \\(\\) at .*" {
    # With gcc 9.2.0 we jump once back to main before entering foo.
    # If that happens try another step.
    send_gdb "step\n"
    exp_continue
  }
}

It's a standard pattern we use in many places, I don't think it's bad enough that we need
to hide it.

> 
> I'm not suggesting that this patch be scrapped, but maybe this could be implemented on purpose, something like gdb_test_optional. The code itself LGTM, but I am not able to approve patches.
> 


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
  2022-04-07 20:31   ` Bruno Larsen
@ 2022-05-16 16:01   ` Tom Tromey
  2022-05-17 11:25     ` Pedro Alves
  2022-05-17 11:41   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Simon Marchi
  2 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2022-05-16 16:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> gdb_test supports handling scenarios where GDB asks a question before
Pedro> finishing handling some command.  The full prototype of gdb_test is:

Pedro>   # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE

I wonder if, after this patch, we can rewrite gdb_test to use ordinary
arguments and not parse 'args' by itself:

Like instead of:

    set command [lindex $args 0]
    set pattern [lindex $args 1]

Just write

    proc gdb_test {command pattern {message ""} {question ""} {response ""}}

... and then default 'message' when appropriate.

Tom

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

* Re: [PATCH 0/5] Make gdb_test's question non-optional if specified
  2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
                   ` (4 preceding siblings ...)
  2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
@ 2022-05-16 16:02 ` Tom Tromey
  5 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2022-05-16 16:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Pedro Alves (5):
Pedro>   Remove gdb_test questions that GDB doesn't ask
Pedro>   gdb.base/scope.exp: Remove bogus gdb_test questions
Pedro>   Fix bogus gdb_test invocations
Pedro>   Avoid having to unload file in
Pedro>     gdb.server/connect-with-no-symbol-file.exp
Pedro>   Make gdb_test's question non-optional if specified

I read through these and they look good to me.

Tom

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

* [PATCH 5/6] gdb.base/skip.exp: Don't abuse gdb_test's question support (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-04-08 12:18     ` Pedro Alves
@ 2022-05-17 10:13       ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-17 10:13 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

On 2022-04-08 13:18, Pedro Alves wrote:
> On 2022-04-07 21:31, Bruno Larsen wrote:
>> On 3/30/22 16:29, Pedro Alves wrote:
>>> gdb_test supports handling scenarios where GDB asks a question before
>>> finishing handling some command.  The full prototype of gdb_test is:
>>>
>>>    # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
>>>
>>> However, QUESTION is a question that GDB _may_ ask, not one that GDB
>>> _must_ ask:
>>>
>>>   # QUESTION is a question GDB may ask in response to COMMAND, like
>>>   #   "are you sure?"
>>>   # RESPONSE is the response to send if QUESTION appears.
>>>
>>> If GDB doesn't raise the question, the test still passes.
>>>
>>> I think that this is a misfeature.  If GDB regresses and stops asking
>>> a question, the testsuite won't notice.  So I think that if a QUESTION
>>> is specified, gdb_test should ensure it comes out of GDB.
>>
>> I just ran into a neat use of this, or possibly a mis-use, depending on how much you like it. gdb.base/skip.exp uses:
>>
>> gdb_test "step" "desired spot" "go to desired spot" "undesirable spot" "step"
>>
>> as a simple way to handle a gcc 9.2.0 bug (misfeature) where gdb could stop in an undesirable spot without actually causing a failure in the test. This feels like a neat way to deal with compiler problems, as it would introduce a simple way to deal with clang's lack of epilogue, for instance.
> 
> This is:
> 
>     # With gcc 9.2.0 we jump once back to main before entering foo here.
>     # If that happens try to step a second time.
>     gdb_test "step" "foo \\(\\) at.*" "step 3" \
> 	"main \\(\\) at .*\r\n$gdb_prompt " "step"
> 
> I think this is a misuse.  That case doesn't really handle a GDB confirmation question.
> What if you need to issue more steps, or the program may stop elsewhere for
> different ports, and thus you need more than one regexp?  For the latter you can
> use (REGEX1|REGEX1), but it isn't as nice as separate -re entries, IMHO.
> 
> IMHO, writing it with gdb_test_multiple in these cases is OK:
> 
> gdb_test_multiple "step" "step 3" {
>   -re -wrap "foo \\(\\) at.*" {
>      pass $gdb_test_multiple
>   }
>   -re -wrap "main \\(\\) at .*" {
>     # With gcc 9.2.0 we jump once back to main before entering foo.
>     # If that happens try another step.
>     send_gdb "step\n"
>     exp_continue
>   }
> }
> 
> It's a standard pattern we use in many places, I don't think it's bad enough that we need
> to hide it.
> 
>>
>> I'm not suggesting that this patch be scrapped, but maybe this could be implemented on purpose, something like gdb_test_optional. The code itself LGTM, but I am not able to approve patches.
>>
> 

I'm adding the patch below to the series, before old patch #5, to prevent
gdb.base/skip.exp from starting to fail with older GCCs.

I'm going to merge the whole series now.



From d7440bee9ffa6767e704f226ec28b9aa2fb748d6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 17 May 2022 10:25:12 +0100
Subject: [PATCH] gdb.base/skip.exp: Don't abuse gdb_test's question support

gdb.base/skip.exp abuses gdb_test's support for answering a GDB
question to do this:

    # With gcc 9.2.0 we jump once back to main before entering foo here.
    # If that happens try to step a second time.
    gdb_test "step" "foo \\(\\) at.*" "step 3" \
	"main \\(\\) at .*\r\n$gdb_prompt " "step"

After a patch later in this series, gdb_test will FAIL if GDB does NOT
issue the question, so this test would start failing on older GCCs.

Switch to using gdb_test_multiple instead.  There are three spots in
the file that have the same pattern, and they're actually in a
sequence of commands that is repeated those 3 times.  Factor all that
out to a procedure.

I don't have gcc 9.2 handy, but I do have gcc 6.5, and that one is
affected as well, so update the comment.

Change-Id: If0a7e3cdf5191b4eec95ce0c8845c3a4d801c39e
---
 gdb/testsuite/gdb.base/skip.exp | 55 +++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index 7c71bb07a84..e6b660004d9 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -122,6 +122,32 @@ with_test_prefix "step after deleting 1" {
     gdb_test "step" "main \\(\\) at.*" "step 3"
 }
 
+# Test that we step into foo(), then into bar(), but not into baz().
+proc step_bar_foo_skip_baz {} {
+    gdb_test "step" "bar \\(\\) at.*" "step 1"
+    gdb_test "step" ".*" "step 2"; # Return from bar()
+
+    # With at least gcc 6.5.0 and 9.2.0, we jump once back to main
+    # before entering foo here.  If that happens try to step a second
+    # time.
+    set stepped_again 0
+    gdb_test_multiple "step" "step 3" {
+	-re -wrap "foo \\(\\) at.*" {
+	    pass $gdb_test_name
+	}
+	-re -wrap "main \\(\\) at .*" {
+	    if {!$stepped_again} {
+		set stepped_again 1
+		send_gdb "step\n"
+	    }
+	    exp_continue
+	}
+    }
+
+    gdb_test "step" ".*" "step 4"; # Return from foo()
+    gdb_test "step" "main \\(\\) at.*" "step 5"
+}
+
 # Now disable the skiplist entry for  skip1.c.  We should now
 # step into foo(), then into bar(), but not into baz().
 
@@ -136,14 +162,7 @@ with_test_prefix "step after disabling 3" {
 	return
     }
 
-    gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    step_bar_foo_skip_baz
 }
 
 # Enable skiplist entry 3 and make sure we step over it like before.
@@ -254,14 +273,8 @@ with_test_prefix "step using -fu for baz" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 7"
-    gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+
+    step_bar_foo_skip_baz
 }
 
 with_test_prefix "step using -rfu for baz" {
@@ -271,14 +284,8 @@ with_test_prefix "step using -rfu for baz" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 8"
-    gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+
+    step_bar_foo_skip_baz
 }
 
 # Test -fi + -fu.

-- 
2.36.0


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-05-16 16:01   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Tom Tromey
@ 2022-05-17 11:25     ` Pedro Alves
  2022-05-17 22:48       ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2022-05-17 11:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2022-05-16 17:01, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> gdb_test supports handling scenarios where GDB asks a question before
> Pedro> finishing handling some command.  The full prototype of gdb_test is:
> 
> Pedro>   # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
> 
> I wonder if, after this patch, we can rewrite gdb_test to use ordinary
> arguments and not parse 'args' by itself:
> 
> Like instead of:
> 
>     set command [lindex $args 0]
>     set pattern [lindex $args 1]
> 
> Just write
> 
>     proc gdb_test {command pattern {message ""} {question ""} {response ""}}
> 
> ... and then default 'message' when appropriate.

(Note pattern is optional too.)

Maybe, but OTOH, we could instead switch to using parse_args, which would
let us support other options to gdb_test, like gdb_test_multiple's 
  [ -prompt PROMPT_REGEXP] [ -lbl ],
and whatever other options we come up in future.

We could even support

  gdb_test "command" "pattern" \
     -question "you sure\?" -response "y"

  gdb_test "command" "pattern" \
     -question "you sure\?" -response "y"

which has the advantage of not requiring you to specify $message whenever you need to supply
a question/response.  We could do both, actually, support parse_args, and keep supporting
the positional arguments.

lindex returns an empty string if the index is >= than number of elements, so we can simplify
things by checking if the positional argument is "" instead of looking at whether it was actually
passed down.  I think this removes most of the ugliness.

In the patch below I've added support for -prompt and -lbl, and one example using gdb_test -prompt,
to show better what I mean.


From 2ef20ed4a12a94a8352d16051b8d1920497348f4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 17 May 2022 11:16:01 +0100
Subject: [PATCH] Support -prompt and -lbl in gdb_test

Change-Id: I243e1296d32c05a421ccef30b63d43a89eaeb4a0
---
 gdb/testsuite/gdb.base/ui-redirect.exp | 10 ++---
 gdb/testsuite/lib/gdb.exp              | 56 +++++++++++++++-----------
 2 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
index 13bc964f46c..4ed82ae63bf 100644
--- a/gdb/testsuite/gdb.base/ui-redirect.exp
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -117,12 +117,10 @@ with_test_prefix "debugging" {
     gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Copying debug output to /dev/null\\."
 
-    set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$"
-    gdb_test_multiple "continue" "continue" -prompt $prompt {
-	-re "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*$prompt$" {
-	    pass $gdb_test_name
-	}
-    }
+    gdb_test \
+	-prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$" \
+	"continue" \
+	"Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*"
 
     gdb_test "set debug infrun 0"
     gdb_test "set logging enabled off" "Done logging to /dev/null\\."
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0b1104bd299..071f4994a76 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1313,7 +1313,7 @@ proc gdb_test_multiline { name args } {
 }
 
 
-# gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
+# gdb_test COMMAND [PATTERN] [MESSAGE] [QUESTION RESPONSE] [-prompt PROMPT] [-lbl]
 # Send a command to gdb; test the result.
 #
 # COMMAND is the command to execute, send to GDB with send_gdb.  If
@@ -1335,57 +1335,67 @@ proc gdb_test_multiline { name args } {
 #    1 if the test failed,
 #    0 if the test passes,
 #   -1 if there was an internal error.
-#  
+#
 proc gdb_test { args } {
     global gdb_prompt
     upvar timeout timeout
 
+    parse_args {
+	{prompt ""}
+	{lbl}
+    }
+
+    set command [lindex $args 0]
+    set pattern [lindex $args 1]
+    set message [lindex $args 2]
+    set question [lindex $args 3]
+    set response [lindex $args 4]
+
     # Can't have a question without a response.
-    if { [llength $args] == 4 || [llength $args] > 5 } {
+    if { $question != "" && $response == "" } {
 	error "Unexpected arguments: $args"
     }
 
-    if [llength $args]>2 then {
-	set message [lindex $args 2]
-    } else {
-	set message [lindex $args 0]
+    if { $message == "" } {
+	set message $command
     }
-    set command [lindex $args 0]
-    set pattern [lindex $args 1]
 
-    set must_see_question 0
-    if { [llength $args] == 5 } {
-	set must_see_question 1
-	set saw_question 0
+    if { $prompt == "" } {
+	set prompt "$gdb_prompt $"
     }
 
+    set saw_question 0
+
     set user_code {}
     lappend user_code {
-	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$gdb_prompt $" {
+	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$prompt" {
 	    if ![string match "" $message] then {
-		if {$must_see_question} {
+		if { $question != "" } {
 		    gdb_assert $saw_question "$message"
 		} else {
 		    pass "$message"
 		}
-            }
-        }
+	    }
+	}
     }
 
-    if { [llength $args] == 5 } {
-	set question_string [lindex $args 3]
-	set response_string [lindex $args 4]
+    if { $question != "" } {
 	lappend user_code {
-	    -re "(${question_string})$" {
+	    -re "($question)$" {
 		set saw_question 1
-		send_gdb "$response_string\n"
+		send_gdb "$response\n"
 		exp_continue
 	    }
 	}
      }
 
+    set opts {}
+    lappend "-prompt $prompt"
+    if {$lbl} {
+	lappend opts "-lbl"
+    }
     set user_code [join $user_code]
-    return [gdb_test_multiple $command $message $user_code]
+    return [gdb_test_multiple $command $message {*}$opts $user_code]
 }
 
 # Return 1 if version MAJOR.MINOR is at least AT_LEAST_MAJOR.AT_LEAST_MINOR.

base-commit: ed01945057cfdf048bc025f15b410492e12283f6
-- 
2.36.0


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
  2022-04-07 20:31   ` Bruno Larsen
  2022-05-16 16:01   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Tom Tromey
@ 2022-05-17 11:41   ` Simon Marchi
  2022-05-17 12:04     ` Pedro Alves
  2 siblings, 1 reply; 26+ messages in thread
From: Simon Marchi @ 2022-05-17 11:41 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches



On 2022-03-30 15:29, Pedro Alves wrote:
> gdb_test supports handling scenarios where GDB asks a question before
> finishing handling some command.  The full prototype of gdb_test is:
> 
>   # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
> 
> However, QUESTION is a question that GDB _may_ ask, not one that GDB
> _must_ ask:
> 
>  # QUESTION is a question GDB may ask in response to COMMAND, like
>  #   "are you sure?"
>  # RESPONSE is the response to send if QUESTION appears.
> 
> If GDB doesn't raise the question, the test still passes.
> 
> I think that this is a misfeature.  If GDB regresses and stops asking
> a question, the testsuite won't notice.  So I think that if a QUESTION
> is specified, gdb_test should ensure it comes out of GDB.
> 
> Running the testsuite exposed a number of tests that pass
> QUESTION/RESPONSE to GDB, but no question comes out.  The previous
> commits fixed them all, so this commit changes gdb_test's behavior.
> 
> A related issue is that gdb_test doesn't enforce that if you specify
> QUESTION, that you also specify RESPONSE.  I.e., you should pass 1, 2,
> 3, or 5 arguments to gdb_test, but never 4, or more than 5.  Making
> gdb_test detect bogus arguments actually regressed some testcases,
> also all fixed in previous commits.
> 
> Change-Id: I47c39c9034e6a6841129312037a5ca4c5811f0db

Hi Pedro,

I see this failure starting with this patch:

  $ make check TESTS="gdb.python/py-connection.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
  (gdb) PASS: gdb.python/py-connection.exp: info connections while the connection is still around
  disconnect^M
  Ending remote debugging.^M
  (gdb) FAIL: gdb.python/py-connection.exp: kill the inferior

Simon


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-05-17 11:41   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Simon Marchi
@ 2022-05-17 12:04     ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-17 12:04 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 2022-05-17 12:41, Simon Marchi wrote:

> I see this failure starting with this patch:
> 
>   $ make check TESTS="gdb.python/py-connection.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
>   (gdb) PASS: gdb.python/py-connection.exp: info connections while the connection is still around
>   disconnect^M
>   Ending remote debugging.^M
>   (gdb) FAIL: gdb.python/py-connection.exp: kill the inferior
> 

Thanks, I've pushed the patch below to fix it.

From a1f2ddd38378c8db63e75daa28b7e304c2fd774f Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 17 May 2022 12:53:32 +0100
Subject: [PATCH] Fix gdb.python/py-connection.exp with remote targets

After the patch to make gdb_test's question non-optional when
specified, gdb.python/py-connection.exp started failing like so:

  $ make check TESTS="gdb.python/py-connection.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
  (gdb) PASS: gdb.python/py-connection.exp: info connections while the connection is still around
  disconnect^M
  Ending remote debugging.^M
  (gdb) FAIL: gdb.python/py-connection.exp: kill the inferior

The problem is that "disconnect" when debugging with the native target
asks the user whether to kill the program, while with remote targets,
it doesn't.

Fix it by explicitly killing before disconnecting.

Tested with --target_board unix, native-gdbserver, and native-extended-gdbserver.

Change-Id: Icd85015c76deb84b71894715d43853c1087eba0b
---
 gdb/testsuite/gdb.python/py-connection.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.python/py-connection.exp b/gdb/testsuite/gdb.python/py-connection.exp
index c56aba33e4c..91315eb1af4 100644
--- a/gdb/testsuite/gdb.python/py-connection.exp
+++ b/gdb/testsuite/gdb.python/py-connection.exp
@@ -55,8 +55,8 @@ gdb_test "python print('Same object: %s' % (conn is conn2))" "True"
 # starts to return False.
 gdb_test "info connections" "\r\n\\* 1 .*" \
     "info connections while the connection is still around"
-gdb_test "disconnect" "" "kill the inferior" \
-    "A program is being debugged already\\.  Kill it\\? .*y or n. $" "y"
+gdb_test "with confirm off -- kill" "" "kill inferior"
+gdb_test "disconnect"
 gdb_test "info connections" "No connections\\." \
     "info connections now all the connections have gone"
 gdb_test "python print(conn)" "<gdb.${connection_type} \\(invalid\\)>" \

base-commit: 49a73ab9d345e1cd412a93b34a0b8c166603d80e
-- 
2.36.0


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

* Re: [PATCH 5/5] Make gdb_test's question non-optional if specified
  2022-05-17 11:25     ` Pedro Alves
@ 2022-05-17 22:48       ` Tom Tromey
  2022-05-18 11:01         ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2022-05-17 22:48 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

>> proc gdb_test {command pattern {message ""} {question ""} {response ""}}
>> 
>> ... and then default 'message' when appropriate.

Pedro> (Note pattern is optional too.)

Oh yeah, I forgot about that.

Pedro> In the patch below I've added support for -prompt and -lbl, and
Pedro> one example using gdb_test -prompt, to show better what I mean.

Seems pretty reasonable.

Pedro> +    set command [lindex $args 0]
Pedro> +    set pattern [lindex $args 1]
Pedro> +    set message [lindex $args 2]
Pedro> +    set question [lindex $args 3]
Pedro> +    set response [lindex $args 4]

You can also write:

  lassign $args command pattern message question response

Tom

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

* [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-17 22:48       ` Tom Tromey
@ 2022-05-18 11:01         ` Pedro Alves
  2022-05-18 12:15           ` Tom de Vries
  2022-05-23 10:48           ` Tom de Vries
  0 siblings, 2 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-18 11:01 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2022-05-17 23:48, Tom Tromey wrote:
> 
> Pedro> In the patch below I've added support for -prompt and -lbl, and
> Pedro> one example using gdb_test -prompt, to show better what I mean.
> 
> Seems pretty reasonable.
> 
> Pedro> +    set command [lindex $args 0]
> Pedro> +    set pattern [lindex $args 1]
> Pedro> +    set message [lindex $args 2]
> Pedro> +    set question [lindex $args 3]
> Pedro> +    set response [lindex $args 4]
> 
> You can also write:
> 
>   lassign $args command pattern message question response

Even better!

I've now looked for an example where we could use "gdb_test -lbl", and found it.

Here's what I pushed.

From c76d61da4a65eaadca861bf6c77d579a5cc3f422 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 17 May 2022 11:16:01 +0100
Subject: [PATCH] Support -prompt and -lbl in gdb_test

This teaches gdb_test to forward the -prompt and -lbl options to
gdb_test_multiple.

The option parsing is done with parse_args.

As a cleanup, instead of using llength and lindex to get at the
positional arguments, use lassign, and check whether the corresponding
variable is empty.

Convert gdb.base/ui-redirect.exp and gdb.xml/tdesc-reload.exp to use
gdb_test -prompt/-lbl instead of gdb_test_multiple as examples.

Change-Id: I243e1296d32c05a421ccef30b63d43a89eaeb4a0
---
 gdb/testsuite/gdb.base/ui-redirect.exp | 10 ++---
 gdb/testsuite/gdb.xml/tdesc-reload.exp |  6 +--
 gdb/testsuite/lib/gdb.exp              | 61 ++++++++++++++++----------
 3 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
index 13bc964f46c..4ed82ae63bf 100644
--- a/gdb/testsuite/gdb.base/ui-redirect.exp
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -117,12 +117,10 @@ with_test_prefix "debugging" {
     gdb_test "set logging enabled on" \
     "Copying output to /dev/null.*Copying debug output to /dev/null\\."
 
-    set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$"
-    gdb_test_multiple "continue" "continue" -prompt $prompt {
-	-re "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*$prompt$" {
-	    pass $gdb_test_name
-	}
-    }
+    gdb_test \
+	-prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$" \
+	"continue" \
+	"Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*"
 
     gdb_test "set debug infrun 0"
     gdb_test "set logging enabled off" "Done logging to /dev/null\\."
diff --git a/gdb/testsuite/gdb.xml/tdesc-reload.exp b/gdb/testsuite/gdb.xml/tdesc-reload.exp
index 76dd70fb6d3..6e3fe2f5304 100644
--- a/gdb/testsuite/gdb.xml/tdesc-reload.exp
+++ b/gdb/testsuite/gdb.xml/tdesc-reload.exp
@@ -68,11 +68,7 @@ if ![runto_main] then {
 
 # Run info registers just to check this appears to run fine with the
 # new target description.
-gdb_test_multiple "info all-registers" "Run info registers" -lbl {
-    -re -wrap "" {
-	pass $gdb_test_name
-    }
-}
+gdb_test -lbl "info all-registers" "" "Run info registers"
 
 # Write out the current target description.
 gdb_test_no_output "pipe maint print xml-tdesc | cat > $xml_file_3" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0b1104bd299..97841ca19a1 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1313,7 +1313,8 @@ proc gdb_test_multiline { name args } {
 }
 
 
-# gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
+# gdb_test [-prompt PROMPT_REGEXP] [-lbl]
+#          COMMAND [PATTERN] [MESSAGE] [QUESTION RESPONSE]
 # Send a command to gdb; test the result.
 #
 # COMMAND is the command to execute, send to GDB with send_gdb.  If
@@ -1331,61 +1332,73 @@ proc gdb_test_multiline { name args } {
 #   doesn't print the question.
 # RESPONSE is the response to send when QUESTION appears.
 #
+# -prompt PROMPT_REGEXP specifies a regexp matching the expected prompt
+#   after the command output.  If empty, defaults to "$gdb_prompt $".
+# -lbl specifies that line-by-line matching will be used.
+#
 # Returns:
 #    1 if the test failed,
 #    0 if the test passes,
 #   -1 if there was an internal error.
-#  
+#
 proc gdb_test { args } {
     global gdb_prompt
     upvar timeout timeout
 
+    parse_args {
+	{prompt ""}
+	{lbl}
+    }
+
+    lassign $args command pattern message question response
+
     # Can't have a question without a response.
-    if { [llength $args] == 4 || [llength $args] > 5 } {
+    if { $question != "" && $response == "" || [llength $args] > 5 } {
 	error "Unexpected arguments: $args"
     }
 
-    if [llength $args]>2 then {
-	set message [lindex $args 2]
-    } else {
-	set message [lindex $args 0]
+    if { $message == "" } {
+	set message $command
     }
-    set command [lindex $args 0]
-    set pattern [lindex $args 1]
 
-    set must_see_question 0
-    if { [llength $args] == 5 } {
-	set must_see_question 1
-	set saw_question 0
+    if { $prompt == "" } {
+	set prompt "$gdb_prompt $"
     }
 
+    set saw_question 0
+
     set user_code {}
     lappend user_code {
-	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$gdb_prompt $" {
+	-re "\[\r\n\]*(?:$pattern)\[\r\n\]+$prompt" {
 	    if ![string match "" $message] then {
-		if {$must_see_question} {
+		if { $question != "" } {
 		    gdb_assert $saw_question "$message"
 		} else {
 		    pass "$message"
 		}
-            }
-        }
+	    }
+	}
     }
 
-    if { [llength $args] == 5 } {
-	set question_string [lindex $args 3]
-	set response_string [lindex $args 4]
+    if { $question != "" } {
 	lappend user_code {
-	    -re "(${question_string})$" {
+	    -re "$question$" {
 		set saw_question 1
-		send_gdb "$response_string\n"
+		send_gdb "$response\n"
 		exp_continue
 	    }
 	}
-     }
+    }
 
     set user_code [join $user_code]
-    return [gdb_test_multiple $command $message $user_code]
+
+    set opts {}
+    lappend "-prompt $prompt"
+    if {$lbl} {
+	lappend opts "-lbl"
+    }
+
+    return [gdb_test_multiple $command $message {*}$opts $user_code]
 }
 
 # Return 1 if version MAJOR.MINOR is at least AT_LEAST_MAJOR.AT_LEAST_MINOR.

base-commit: 44b6e8016087c346dc21270d9097f6294ee1e4f6
-- 
2.36.0


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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 11:01         ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
@ 2022-05-18 12:15           ` Tom de Vries
  2022-05-18 12:36             ` Pedro Alves
  2022-05-23 10:48           ` Tom de Vries
  1 sibling, 1 reply; 26+ messages in thread
From: Tom de Vries @ 2022-05-18 12:15 UTC (permalink / raw)
  To: Pedro Alves, Tom Tromey; +Cc: gdb-patches

On 5/18/22 13:01, Pedro Alves wrote:
> -    if [llength $args]>2 then {
> -	set message [lindex $args 2]
> -    } else {
> -	set message [lindex $args 0]
> +    if { $message == "" } {
> +	set message $command
>       }

This seems to cause:
...
PATH: gdb.ada/exec_changed.exp: shell mv 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
PATH: gdb.ada/exec_changed.exp: shell mv 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
PATH: gdb.ada/exec_changed.exp: shell mv 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/second 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
PATH: gdb.ada/exec_changed.exp: shell touch 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
PATH: gdb.ada/exec_changed.exp: shell touch 
/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
...
because the interpretation of message "" changed:
...
gdb_test "shell mv ${binfile} ${common_binfile}" ".*" ""
...

Thanks,
- Tom

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 12:15           ` Tom de Vries
@ 2022-05-18 12:36             ` Pedro Alves
  2022-05-18 14:13               ` Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2022-05-18 12:36 UTC (permalink / raw)
  To: Tom de Vries, Tom Tromey; +Cc: gdb-patches

On 2022-05-18 13:15, Tom de Vries wrote:
> On 5/18/22 13:01, Pedro Alves wrote:
>> -    if [llength $args]>2 then {
>> -    set message [lindex $args 2]
>> -    } else {
>> -    set message [lindex $args 0]
>> +    if { $message == "" } {
>> +    set message $command
>>       }
> 
> This seems to cause:
> ...
> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/second /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
> ...
> because the interpretation of message "" changed:
> ...
> gdb_test "shell mv ${binfile} ${common_binfile}" ".*" ""
> ...

Hmm...  Yeah...  We could revert most of the changes to gdb_test and just keep the parse_args
part.  However, IMO the old behavior is a misfeature, though.  I think tests should
always have a name.  E.g., if such a test hits an internal error, what message would
be used?

The documentation of the option even says that if message is omitted, use the command
string as message:

 # MESSAGE is an optional message to be printed.  If this is
 #   omitted, then the pass/fail messages use the command string as the
 #   message.  (If this is the empty string, then sometimes we don't
 #   call pass or fail at all; I don't understand this at all.)

Also, gdb_test_multiple doesn't a distinction between explicit "" message and
not specified message, the only way to end up with an empty message is if command
is empty as well.  So AFAICS, this change (inadvertently) made gdb_test and
gdb_test_multiple behave the same in this respect.

So how about we just fix the affected gdb_test invocations?

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 12:36             ` Pedro Alves
@ 2022-05-18 14:13               ` Pedro Alves
  2022-05-18 14:49                 ` Tom de Vries
  2022-05-18 20:34                 ` Tom Tromey
  0 siblings, 2 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-18 14:13 UTC (permalink / raw)
  To: Tom de Vries, Tom Tromey; +Cc: gdb-patches

On 2022-05-18 13:36, Pedro Alves wrote:
> On 2022-05-18 13:15, Tom de Vries wrote:
>> On 5/18/22 13:01, Pedro Alves wrote:
>>> -    if [llength $args]>2 then {
>>> -    set message [lindex $args 2]
>>> -    } else {
>>> -    set message [lindex $args 0]
>>> +    if { $message == "" } {
>>> +    set message $command
>>>       }
>>
>> This seems to cause:
>> ...
>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/second /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
>> ...
>> because the interpretation of message "" changed:
>> ...
>> gdb_test "shell mv ${binfile} ${common_binfile}" ".*" ""
>> ...
> 
> Hmm...  Yeah...  We could revert most of the changes to gdb_test and just keep the parse_args
> part.  However, IMO the old behavior is a misfeature, though.  I think tests should
> always have a name.  E.g., if such a test hits an internal error, what message would
> be used?
> 
> The documentation of the option even says that if message is omitted, use the command
> string as message:
> 
>  # MESSAGE is an optional message to be printed.  If this is
>  #   omitted, then the pass/fail messages use the command string as the
>  #   message.  (If this is the empty string, then sometimes we don't
>  #   call pass or fail at all; I don't understand this at all.)
> 
> Also, gdb_test_multiple doesn't a distinction between explicit "" message and
> not specified message, the only way to end up with an empty message is if command
> is empty as well.  So AFAICS, this change (inadvertently) made gdb_test and
> gdb_test_multiple behave the same in this respect.
> 
> So how about we just fix the affected gdb_test invocations?

So I'm diffing testruns from before the patch vs after, and I think the vast
majority of cases that weren't issuing a pass should do so.  With the new
messages, we now get a ton of DUPLICATEs, which I'm fixing.

However, there are a few spots here and there where we really would prefer to
not issue a pass, such as when we're testing something in a loop, and we don't
know how many iterations there will be.

Instead of going back to how it used to be, I'm thinking of adding a new
option to gdb_test, "gdb_test -nopass".  The advantage of this approach is
that we always have a message for the FAIL case this way, and, it's more
explicit.  We could fix the "no message for the FAIL case" in the old implementation
by delaying defaulting message to the command until after the pass case is added
to user_code.  But the explict option still seems better to me, as it let's you
specify a message different from the command, and only print it on FAIL.  The other
approach would force the message to be the same as the command.

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 14:13               ` Pedro Alves
@ 2022-05-18 14:49                 ` Tom de Vries
  2022-05-18 20:34                 ` Tom Tromey
  1 sibling, 0 replies; 26+ messages in thread
From: Tom de Vries @ 2022-05-18 14:49 UTC (permalink / raw)
  To: Pedro Alves, Tom Tromey; +Cc: gdb-patches

On 5/18/22 16:13, Pedro Alves wrote:
> On 2022-05-18 13:36, Pedro Alves wrote:
>> On 2022-05-18 13:15, Tom de Vries wrote:
>>> On 5/18/22 13:01, Pedro Alves wrote:
>>>> -    if [llength $args]>2 then {
>>>> -    set message [lindex $args 2]
>>>> -    } else {
>>>> -    set message [lindex $args 0]
>>>> +    if { $message == "" } {
>>>> +    set message $command
>>>>        }
>>>
>>> This seems to cause:
>>> ...
>>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
>>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/second /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common
>>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first
>>> ...
>>> because the interpretation of message "" changed:
>>> ...
>>> gdb_test "shell mv ${binfile} ${common_binfile}" ".*" ""
>>> ...
>>
>> Hmm...  Yeah...  We could revert most of the changes to gdb_test and just keep the parse_args
>> part.  However, IMO the old behavior is a misfeature, though.  I think tests should
>> always have a name.  E.g., if such a test hits an internal error, what message would
>> be used?
>>
>> The documentation of the option even says that if message is omitted, use the command
>> string as message:
>>
>>   # MESSAGE is an optional message to be printed.  If this is
>>   #   omitted, then the pass/fail messages use the command string as the
>>   #   message.  (If this is the empty string, then sometimes we don't
>>   #   call pass or fail at all; I don't understand this at all.)
>>
>> Also, gdb_test_multiple doesn't a distinction between explicit "" message and
>> not specified message, the only way to end up with an empty message is if command
>> is empty as well.  So AFAICS, this change (inadvertently) made gdb_test and
>> gdb_test_multiple behave the same in this respect.
>>
>> So how about we just fix the affected gdb_test invocations?
> 
> So I'm diffing testruns from before the patch vs after, and I think the vast
> majority of cases that weren't issuing a pass should do so.  With the new
> messages, we now get a ton of DUPLICATEs, which I'm fixing.
> 
> However, there are a few spots here and there where we really would prefer to
> not issue a pass, such as when we're testing something in a loop, and we don't
> know how many iterations there will be.
> 

Ack.  We could do something fancy like this, which would mean not having 
to update the msg or rewrite into gdb_test_multiple
...
message_squash {
    foreach var [seq 1 20] {
      gdb_test <cmd> <pat> <msg>
    }
}
...
and get:
...
PASS: <msg> (20x)
...
but perhaps that'll just confusion.

> Instead of going back to how it used to be, I'm thinking of adding a new
> option to gdb_test, "gdb_test -nopass".  The advantage of this approach is
> that we always have a message for the FAIL case this way, and, it's more
> explicit.  We could fix the "no message for the FAIL case" in the old implementation
> by delaying defaulting message to the command until after the pass case is added
> to user_code.  But the explict option still seems better to me, as it let's you
> specify a message different from the command, and only print it on FAIL.  The other
> approach would force the message to be the same as the command.

Makes sense to me.

Thanks,
- Tom

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 14:13               ` Pedro Alves
  2022-05-18 14:49                 ` Tom de Vries
@ 2022-05-18 20:34                 ` Tom Tromey
  2022-05-19 12:42                   ` Pedro Alves
  1 sibling, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2022-05-18 20:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom de Vries, Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Instead of going back to how it used to be, I'm thinking of adding a new
Pedro> option to gdb_test, "gdb_test -nopass".  The advantage of this approach is
Pedro> that we always have a message for the FAIL case this way, and, it's more
Pedro> explicit.

I like this a lot more than the current approach.

Tom

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 20:34                 ` Tom Tromey
@ 2022-05-19 12:42                   ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-19 12:42 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom de Vries, gdb-patches

On 2022-05-18 21:34, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Instead of going back to how it used to be, I'm thinking of adding a new
> Pedro> option to gdb_test, "gdb_test -nopass".  The advantage of this approach is
> Pedro> that we always have a message for the FAIL case this way, and, it's more
> Pedro> explicit.
> 
> I like this a lot more than the current approach.

Alright, see here:

  https://sourceware.org/pipermail/gdb-patches/2022-May/189231.html

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-18 11:01         ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
  2022-05-18 12:15           ` Tom de Vries
@ 2022-05-23 10:48           ` Tom de Vries
  2022-05-23 12:01             ` Tom de Vries
  1 sibling, 1 reply; 26+ messages in thread
From: Tom de Vries @ 2022-05-23 10:48 UTC (permalink / raw)
  To: Pedro Alves, Tom Tromey; +Cc: gdb-patches

On 5/18/22 13:01, Pedro Alves wrote:
> diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
> index 13bc964f46c..4ed82ae63bf 100644
> --- a/gdb/testsuite/gdb.base/ui-redirect.exp
> +++ b/gdb/testsuite/gdb.base/ui-redirect.exp
> @@ -117,12 +117,10 @@ with_test_prefix "debugging" {
>       gdb_test "set logging enabled on" \
>       "Copying output to /dev/null.*Copying debug output to /dev/null\\."
>   
> -    set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$"
> -    gdb_test_multiple "continue" "continue" -prompt $prompt {
> -	-re "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*$prompt$" {
> -	    pass $gdb_test_name
> -	}
> -    }
> +    gdb_test \
> +	-prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: exit\r\n$" \
> +	"continue" \
> +	"Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint \[0-9\]+, foo.*"
>   
>       gdb_test "set debug infrun 0"
>       gdb_test "set logging enabled off" "Done logging to /dev/null\\."

I'm running into:
...
   [infrun] maybe_set_commit_resumed_all_targets: not requesting 
commit-resumed for target native, no resumed threads^M
(gdb) FAIL: gdb.base/ui-redirect.exp: debugging: continue
[infrun] fetch_inferior_event: exit^M
...

I'm assuming it's related to this change.

Thanks,
- Tom

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-23 10:48           ` Tom de Vries
@ 2022-05-23 12:01             ` Tom de Vries
  2022-05-23 12:50               ` [committed][gdb/testsuite] Fix -prompt handling in gdb_test Tom de Vries
  2022-05-23 12:53               ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
  0 siblings, 2 replies; 26+ messages in thread
From: Tom de Vries @ 2022-05-23 12:01 UTC (permalink / raw)
  To: Pedro Alves, Tom Tromey; +Cc: gdb-patches

On 5/23/22 12:48, Tom de Vries wrote:
> On 5/18/22 13:01, Pedro Alves wrote:
>> diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp 
>> b/gdb/testsuite/gdb.base/ui-redirect.exp
>> index 13bc964f46c..4ed82ae63bf 100644
>> --- a/gdb/testsuite/gdb.base/ui-redirect.exp
>> +++ b/gdb/testsuite/gdb.base/ui-redirect.exp
>> @@ -117,12 +117,10 @@ with_test_prefix "debugging" {
>>       gdb_test "set logging enabled on" \
>>       "Copying output to /dev/null.*Copying debug output to /dev/null\\."
>> -    set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: 
>> exit\r\n$"
>> -    gdb_test_multiple "continue" "continue" -prompt $prompt {
>> -    -re "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint 
>> \[0-9\]+, foo.*$prompt$" {
>> -        pass $gdb_test_name
>> -    }
>> -    }
>> +    gdb_test \
>> +    -prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: 
>> exit\r\n$" \
>> +    "continue" \
>> +    "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint 
>> \[0-9\]+, foo.*"
>>       gdb_test "set debug infrun 0"
>>       gdb_test "set logging enabled off" "Done logging to /dev/null\\."
> 
> I'm running into:
> ...
>    [infrun] maybe_set_commit_resumed_all_targets: not requesting 
> commit-resumed for target native, no resumed threads^M
> (gdb) FAIL: gdb.base/ui-redirect.exp: debugging: continue
> [infrun] fetch_inferior_event: exit^M
> ...
> 
> I'm assuming it's related to this change.

That's confirmed, reproduces reliably with read1, and is fixed by 
reverting the patch.

Looks like this in gdb_test:
...
     lappend "-prompt $prompt"
...
has no effect.

This fixes it:
...
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 720418beac2..a6780d8d634 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1393,7 +1393,7 @@ proc gdb_test { args } {
      set user_code [join $user_code]

      set opts {}
-    lappend "-prompt $prompt"
+    lappend opts "-prompt" "$prompt"
      if {$lbl} {
         lappend opts "-lbl"
      }
...

Thanks,
- Tom

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

* [committed][gdb/testsuite] Fix -prompt handling in gdb_test
  2022-05-23 12:01             ` Tom de Vries
@ 2022-05-23 12:50               ` Tom de Vries
  2022-05-23 12:53               ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
  1 sibling, 0 replies; 26+ messages in thread
From: Tom de Vries @ 2022-05-23 12:50 UTC (permalink / raw)
  To: Pedro Alves, Tom Tromey; +Cc: gdb-patches

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

[ was: Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 
5/5] Make gdb_test's question non-optional if specified) ]

On 5/23/22 14:01, Tom de Vries wrote:
> On 5/23/22 12:48, Tom de Vries wrote:
>> On 5/18/22 13:01, Pedro Alves wrote:
>>> diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp 
>>> b/gdb/testsuite/gdb.base/ui-redirect.exp
>>> index 13bc964f46c..4ed82ae63bf 100644
>>> --- a/gdb/testsuite/gdb.base/ui-redirect.exp
>>> +++ b/gdb/testsuite/gdb.base/ui-redirect.exp
>>> @@ -117,12 +117,10 @@ with_test_prefix "debugging" {
>>>       gdb_test "set logging enabled on" \
>>>       "Copying output to /dev/null.*Copying debug output to 
>>> /dev/null\\."
>>> -    set prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: 
>>> exit\r\n$"
>>> -    gdb_test_multiple "continue" "continue" -prompt $prompt {
>>> -    -re "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint 
>>> \[0-9\]+, foo.*$prompt$" {
>>> -        pass $gdb_test_name
>>> -    }
>>> -    }
>>> +    gdb_test \
>>> +    -prompt "$gdb_prompt \\\[infrun\\\] fetch_inferior_event: 
>>> exit\r\n$" \
>>> +    "continue" \
>>> +    "Continuing.*\\\[infrun\\\] .*\\\[infrun\\\] .*Breakpoint 
>>> \[0-9\]+, foo.*"
>>>       gdb_test "set debug infrun 0"
>>>       gdb_test "set logging enabled off" "Done logging to /dev/null\\."
>>
>> I'm running into:
>> ...
>>    [infrun] maybe_set_commit_resumed_all_targets: not requesting 
>> commit-resumed for target native, no resumed threads^M
>> (gdb) FAIL: gdb.base/ui-redirect.exp: debugging: continue
>> [infrun] fetch_inferior_event: exit^M
>> ...
>>
>> I'm assuming it's related to this change.
> 
> That's confirmed, reproduces reliably with read1, and is fixed by 
> reverting the patch.
> 
> Looks like this in gdb_test:
> ...
>      lappend "-prompt $prompt"
> ...
> has no effect.
> 
> This fixes it:
> ...
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 720418beac2..a6780d8d634 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1393,7 +1393,7 @@ proc gdb_test { args } {
>       set user_code [join $user_code]
> 
>       set opts {}
> -    lappend "-prompt $prompt"
> +    lappend opts "-prompt" "$prompt"
>       if {$lbl} {
>          lappend opts "-lbl"
>       }
> ...
> 

Tested and committed.

Thanks,
- Tom

[-- Attachment #2: 0004-gdb-testsuite-Fix-prompt-handling-in-gdb_test.patch --]
[-- Type: text/x-patch, Size: 1046 bytes --]

[gdb/testsuite] Fix -prompt handling in gdb_test

With check-read1 I run into:
...
   [infrun] maybe_set_commit_resumed_all_targets: not requesting
commit-resumed for target native, no resumed threads^M
(gdb) FAIL: gdb.base/ui-redirect.exp: debugging: continue
[infrun] fetch_inferior_event: exit^M
...

The problem is that proc gdb_test doesn't pass down the -prompt option to proc
gdb_test_multiple, due to a typo making this lappend without effect:
...
    set opts {}
    lappend "-prompt $prompt"
...

Fix this by actually appending to opts.

Tested on x86_64-linux.

---
 gdb/testsuite/lib/gdb.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 720418beac2..a6780d8d634 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1393,7 +1393,7 @@ proc gdb_test { args } {
     set user_code [join $user_code]
 
     set opts {}
-    lappend "-prompt $prompt"
+    lappend opts "-prompt" "$prompt"
     if {$lbl} {
 	lappend opts "-lbl"
     }

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

* Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified)
  2022-05-23 12:01             ` Tom de Vries
  2022-05-23 12:50               ` [committed][gdb/testsuite] Fix -prompt handling in gdb_test Tom de Vries
@ 2022-05-23 12:53               ` Pedro Alves
  1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2022-05-23 12:53 UTC (permalink / raw)
  To: Tom de Vries, Tom Tromey; +Cc: gdb-patches

On 2022-05-23 13:01, Tom de Vries wrote:
> On 5/23/22 12:48, Tom de Vries wrote:

>> I'm assuming it's related to this change.
> 
> That's confirmed, reproduces reliably with read1, and is fixed by reverting the patch.
> 
> Looks like this in gdb_test:
> ...
>     lappend "-prompt $prompt"
> ...
> has no effect.

Whoops...  Sorry about this.

> 
> This fixes it:

Thanks, that's obviously OK.  Please push.

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

end of thread, other threads:[~2022-05-23 12:53 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30 19:29 [PATCH 0/5] Make gdb_test's question non-optional if specified Pedro Alves
2022-03-30 19:29 ` [PATCH 1/5] Remove gdb_test questions that GDB doesn't ask Pedro Alves
2022-03-30 19:29 ` [PATCH 2/5] gdb.base/scope.exp: Remove bogus gdb_test questions Pedro Alves
2022-03-30 19:29 ` [PATCH 3/5] Fix bogus gdb_test invocations Pedro Alves
2022-03-30 19:29 ` [PATCH 4/5] Avoid having to unload file in gdb.server/connect-with-no-symbol-file.exp Pedro Alves
2022-03-30 19:29 ` [PATCH 5/5] Make gdb_test's question non-optional if specified Pedro Alves
2022-04-07 20:31   ` Bruno Larsen
2022-04-08 12:18     ` Pedro Alves
2022-05-17 10:13       ` [PATCH 5/6] gdb.base/skip.exp: Don't abuse gdb_test's question support (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
2022-05-16 16:01   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Tom Tromey
2022-05-17 11:25     ` Pedro Alves
2022-05-17 22:48       ` Tom Tromey
2022-05-18 11:01         ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
2022-05-18 12:15           ` Tom de Vries
2022-05-18 12:36             ` Pedro Alves
2022-05-18 14:13               ` Pedro Alves
2022-05-18 14:49                 ` Tom de Vries
2022-05-18 20:34                 ` Tom Tromey
2022-05-19 12:42                   ` Pedro Alves
2022-05-23 10:48           ` Tom de Vries
2022-05-23 12:01             ` Tom de Vries
2022-05-23 12:50               ` [committed][gdb/testsuite] Fix -prompt handling in gdb_test Tom de Vries
2022-05-23 12:53               ` [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Pedro Alves
2022-05-17 11:41   ` [PATCH 5/5] Make gdb_test's question non-optional if specified Simon Marchi
2022-05-17 12:04     ` Pedro Alves
2022-05-16 16:02 ` [PATCH 0/5] " Tom Tromey

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).