[PATCH 1/4] gdb/testsuite: gdb.base/args.exp: use save_vars

[PATCH 1/4] gdb/testsuite: gdb.base/args.exp: use save_vars

[Simon Marchi]

Use save_vars instead of manually saving/restoring.  This ensures that
if anything throws an error, GDBFLAGS will be correctly restored.

Remove the global GDBFLAGS declaration at the top, it's not necessary.

gdb/testsuite/ChangeLog:

	* gdb.base/args.exp: Use save_vars.

Change-Id: I3a45e4fc1635ec0212de2415040f91eecaf4a057
---
 gdb/testsuite/gdb.base/args.exp | 56 ++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index e848c70a482..bff750dff24 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -15,9 +15,6 @@
 
 # This is a test for the gdb invocation option --args.
 
-
-global GDBFLAGS
-
 # Skip test if target does not support argument passing.
 if [target_info exists noargs] {
     return
@@ -64,39 +61,40 @@ proc args_test { name arglist } {
 #
 # Test that the --args are processed correctly.
 #
-set old_gdbflags $GDBFLAGS
 
-set GDBFLAGS "$old_gdbflags --args $binfile 1 3"
-args_test basic {{1} {3}}
+save_vars { GDBFLAGS } {
+    set old_gdbflags $GDBFLAGS
 
-#
-# Test that the --args are processed correctly even if one of them is empty.
-# The syntax needed is a little peculiar; DejaGNU treats the arguments as a
-# list and expands them itself, since no shell redirection is involved.
-#
-set GDBFLAGS "$old_gdbflags --args $binfile 1 {} 3"
-args_test "one empty" {{1} {} {3}}
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 3"
+    args_test basic {{1} {3}}
 
-#
-# try with 2 empty args
-#
-set GDBFLAGS "$old_gdbflags --args $binfile 1 {} {} 3"
-args_test "two empty" {{1} {} {} 3}
+    #
+    # Test that the --args are processed correctly even if one of them is empty.
+    # The syntax needed is a little peculiar; DejaGNU treats the arguments as a
+    # list and expands them itself, since no shell redirection is involved.
+    #
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 {} 3"
+    args_test "one empty" {{1} {} {3}}
 
-# Try with arguments containing literal single quotes.
+    #
+    # try with 2 empty args
+    #
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 {} {} 3"
+    args_test "two empty" {{1} {} {} 3}
 
-set GDBFLAGS "$old_gdbflags --args $binfile 1 '' 3"
-args_test "one empty (with single quotes)" {{1} {''} {3}}
+    # Try with arguments containing literal single quotes.
 
-set GDBFLAGS "$old_gdbflags --args $binfile 1 '' '' 3"
-args_test "two empty (with single quotes)" {{1} {''} {''} {3}}
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 '' 3"
+    args_test "one empty (with single quotes)" {{1} {''} {3}}
 
-# try with arguments containing literal newlines.
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 '' '' 3"
+    args_test "two empty (with single quotes)" {{1} {''} {''} {3}}
 
-set GDBFLAGS "-nx --args $binfile 1 {\n} 3"
-args_test "one newline" {{1} {\\n} {3}}
+    # try with arguments containing literal newlines.
 
-set GDBFLAGS "-nx --args $binfile 1 {\n} {\n} 3"
-args_test "two newlines" {{1} {\\n} {\\n} {3}}
+    set GDBFLAGS "-nx --args $binfile 1 {\n} 3"
+    args_test "one newline" {{1} {\\n} {3}}
 
-set GDBFLAGS $old_gdbflags
+    set GDBFLAGS "-nx --args $binfile 1 {\n} {\n} 3"
+    args_test "two newlines" {{1} {\\n} {\\n} {3}}
+}
-- 
2.31.1

[PATCH 2/4] gdb/testsuite: gdb.base/args.exp: use $old_gdbflags last two tests

[Simon Marchi]

All tests in this file append to GDBFLAGS instead of overwriting it,
except the last two.  I noticed because when testing with the
native-extended-remote board, it removes the "set sysroot" argument, and
it causes the test to be very long to run, due to big glibc debug info
being read through the remote target.

I think this oddity is due to a race condition between these two
commits:

  [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c22261528c50f7760dd6a2e29314662b377eebb4
  [2] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6b8ce727297b1e40738e50f83a75881b290fe6a6

The first one added the two tests.  The second one changes the test to
append to GDBFLAGS instead of overwriting it.  But the second one was
probably written before the first one was it, so missed the new tests.

Change those two tests to be like the others.

gdb/testsuite/ChangeLog:

	* gdb.base/args.exp: Use $old_gdbflags in all tests.

Change-Id: I531276125ecb70e80f52adbd320ebb85b0c8eba0
---
 gdb/testsuite/gdb.base/args.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index bff750dff24..e4cba65d326 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -92,9 +92,9 @@ save_vars { GDBFLAGS } {
 
     # try with arguments containing literal newlines.
 
-    set GDBFLAGS "-nx --args $binfile 1 {\n} 3"
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 {\n} 3"
     args_test "one newline" {{1} {\\n} {3}}
 
-    set GDBFLAGS "-nx --args $binfile 1 {\n} {\n} 3"
+    set GDBFLAGS "$old_gdbflags --args $binfile 1 {\n} {\n} 3"
     args_test "two newlines" {{1} {\\n} {\\n} {3}}
 }
-- 
2.31.1

[PATCH 3/4] gdb/testsuite: gdb.base/args.exp: remove trailing parenthesis in test names

[Simon Marchi]

Some test names end with a parenthesis, we don't want that:

    https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Fix that.

gdb/testsuite/ChangeLog:

	* gdb.base/args.exp: Remove trailing parenthesis in test names.

Change-Id: I0306ea202bae3a4ed5bf0bd65e0ab5ed5de52fe1
---
 gdb/testsuite/gdb.base/args.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index e4cba65d326..e76368f544e 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -85,10 +85,10 @@ save_vars { GDBFLAGS } {
     # Try with arguments containing literal single quotes.
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 '' 3"
-    args_test "one empty (with single quotes)" {{1} {''} {3}}
+    args_test "one empty with single quotes" {{1} {''} {3}}
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 '' '' 3"
-    args_test "two empty (with single quotes)" {{1} {''} {''} {3}}
+    args_test "two empty with single quotes" {{1} {''} {''} {3}}
 
     # try with arguments containing literal newlines.
 
-- 
2.31.1

[PATCH 4/4] gdb/testsuite: gdb.base/args.exp: add KFAIL for native-extended-gdbserver

[Simon Marchi]

This test tests passing arguments made of exactly two single-quotes
('') or a single newline character through the --args argument of GDB.
For some reason, GDB adds some extra single quotes when transmitting the
arguments to GDBserver.  This produces some FAILs when testing with the
native-extended-gdbserver board:

    FAIL: gdb.base/args.exp: argv[2] for one empty (with single quotes)
    FAIL: gdb.base/args.exp: argv[2] for two empty (with single quotes)
    FAIL: gdb.base/args.exp: argv[3] for two empty (with single quotes)
    FAIL: gdb.base/args.exp: argv[2] for one newline
    FAIL: gdb.base/args.exp: argv[2] for two newlines
    FAIL: gdb.base/args.exp: argv[3] for two newlines

This is documented as PR 27989.  Add some appropriate KFAILs.

gdb/testsuite/ChangeLog:

	* gdb.base/args.exp: Check target, KFAIL if remote.
	(args_test): Add parameter and use it.

Change-Id: I49225d1c7df7ebaba480ebdd596df80f8fbf62f0
---
 gdb/testsuite/gdb.base/args.exp | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index e76368f544e..29bb5984245 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -34,7 +34,10 @@ if {[build_executable $testfile.exp $testfile \
     return -1
 }
 
-proc args_test { name arglist } {
+# If SINGLE_QUOTES_NEWLINE_KFAIL true, arguments made of exactly '' or a
+# newline character will fail, so kfail those tests.
+
+proc args_test { name arglist {single_quotes_newline_kfail false}} {
     global srcdir
     global subdir
     global testfile
@@ -52,6 +55,10 @@ proc args_test { name arglist } {
 
     set i 1
     foreach arg $arglist {
+	if { $single_quotes_newline_kfail
+	     && ($arg == {''} || $arg == {\\n}) } {
+	    setup_kfail "gdb/27989" "*-*-*"
+	}
 	gdb_test "print argv\[$i\]" "\\\$$decimal = $hex \"$arg\"" \
 	    "argv\[$i\] for $name"
 	set i [expr $i + 1]
@@ -65,6 +72,10 @@ proc args_test { name arglist } {
 save_vars { GDBFLAGS } {
     set old_gdbflags $GDBFLAGS
 
+    # Start GDB just to see if we are using the native or extended-remote target.
+    clean_restart
+    set single_quotes_newline_kfail [gdb_is_target_remote]
+
     set GDBFLAGS "$old_gdbflags --args $binfile 1 3"
     args_test basic {{1} {3}}
 
@@ -85,16 +96,16 @@ save_vars { GDBFLAGS } {
     # Try with arguments containing literal single quotes.
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 '' 3"
-    args_test "one empty with single quotes" {{1} {''} {3}}
+    args_test "one empty with single quotes" {{1} {''} {3}} $single_quotes_newline_kfail
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 '' '' 3"
-    args_test "two empty with single quotes" {{1} {''} {''} {3}}
+    args_test "two empty with single quotes" {{1} {''} {''} {3}} $single_quotes_newline_kfail
 
     # try with arguments containing literal newlines.
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 {\n} 3"
-    args_test "one newline" {{1} {\\n} {3}}
+    args_test "one newline" {{1} {\\n} {3}} $single_quotes_newline_kfail
 
     set GDBFLAGS "$old_gdbflags --args $binfile 1 {\n} {\n} 3"
-    args_test "two newlines" {{1} {\\n} {\\n} {3}}
+    args_test "two newlines" {{1} {\\n} {\\n} {3}} $single_quotes_newline_kfail
 }
-- 
2.31.1

Re: [PATCH 4/4] gdb/testsuite: gdb.base/args.exp: add KFAIL for native-extended-gdbserver

[Pedro Alves]

On 2021-06-16 8:37 p.m., Simon Marchi via Gdb-patches wrote:
> +    # Start GDB just to see if we are using the native or extended-remote target.
> +    clean_restart
> +    set single_quotes_newline_kfail [gdb_is_target_remote]
> +

You don't need to do that.  You can check 

if {[target_info exists gdb_protocol]
    && ([target_info gdb_protocol] == "remote"
        || [target_info gdb_protocol] == "extended-remote")} {


The series looks good to me.

Pedro Alves

Re: [PATCH 4/4] gdb/testsuite: gdb.base/args.exp: add KFAIL for native-extended-gdbserver

[Simon Marchi]

On 2021-06-17 6:35 a.m., Pedro Alves wrote:
> On 2021-06-16 8:37 p.m., Simon Marchi via Gdb-patches wrote:
>> +    # Start GDB just to see if we are using the native or extended-remote target.
>> +    clean_restart
>> +    set single_quotes_newline_kfail [gdb_is_target_remote]
>> +
> 
> You don't need to do that.  You can check 
> 
> if {[target_info exists gdb_protocol]
>     && ([target_info gdb_protocol] == "remote"
>         || [target_info gdb_protocol] == "extended-remote")} {

Oh, right.  The "remote" case is not applicable here due to the nature
of the test, so I'll just check for extended-remote:

    set single_quotes_newline_kfail \
	[expr { [target_info exists gdb_protocol] \
	        && [target_info gdb_protocol] == "extended-remote" }]

I'll push with that fixed.

Simon