[PATCH] gdb/testsuite: introduce save_procs proc

[PATCH] gdb/testsuite: introduce save_procs proc

[Simon Marchi]

The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally
defined in lib/gdb.exp.  It restores the original version of the proc at
the end.  But the problem is that if the test fails and an
exception is thrown, it won't be restored and the following tests will
be affected.

This patch introduces a `save_procs` procedure inspired by `save_vars`.
This lets the test restore the given procedures whatever happens.

The gdb.base/dbx.exp is modified to use the new proc, and save_vars to
save GDBFLAGS while at it.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (save_procs): New.
	* gdb.base/dbx.exp: Use save_procs and save_vars.

Change-Id: I5eff388dc74f6759f8ee6cf43d7e8a78502bce5a
---
 gdb/testsuite/gdb.base/dbx.exp | 45 +++++++++++++++-------------------
 gdb/testsuite/lib/gdb.exp      | 44 +++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.base/dbx.exp b/gdb/testsuite/gdb.base/dbx.exp
index 4299c44368c..4d84b54cda2 100644
--- a/gdb/testsuite/gdb.base/dbx.exp
+++ b/gdb/testsuite/gdb.base/dbx.exp
@@ -146,12 +146,8 @@ proc dbx_reinitialize_dir { subdir } {
 # file into gdb for a dbx session. So why not just override gdb_file_cmd with the
 # right sequence of events, allowing gdb_load to do its normal thing? This way
 # remotes and simulators will work, too.
-#
-# [drow 2002-03-30]: We can restore the old gdb_file_cmd afterwards, though.
-set old_gdb_file_cmd_args [info args gdb_file_cmd]
-set old_gdb_file_cmd_body [info body gdb_file_cmd]
 
-proc gdb_file_cmd {arg} {
+proc dbx_gdb_file_cmd {arg} {
     global loadpath
     global loadfile
     global GDB
@@ -283,27 +279,26 @@ proc test_func { } {
     gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:${decimal}\r\n${decimal}\[ \t\]+total = sum\\(list, low, high\\);"
 }
 
-# Start with a fresh gdb.
-
-gdb_exit
-global GDBFLAGS
-set saved_gdbflags $GDBFLAGS
-
-set GDBFLAGS "$GDBFLAGS --dbx"
-gdb_start
-dbx_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+save_vars { GDBFLAGS } {
+    save_procs { gdb_file_cmd } {
+	rename gdb_file_cmd {}
+	rename dbx_gdb_file_cmd gdb_file_cmd
 
-test_breakpoints
-test_assign
-test_whereis
-gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*"
-test_func
+	# Start with a fresh gdb.
+	gdb_exit
 
-#exit and cleanup
-gdb_exit
+	set GDBFLAGS "$GDBFLAGS --dbx"
+	gdb_start
+	dbx_reinitialize_dir $srcdir/$subdir
+	gdb_load ${binfile}
 
-set GDBFLAGS $saved_gdbflags
-eval proc gdb_file_cmd {$old_gdb_file_cmd_args} {$old_gdb_file_cmd_body}
+	test_breakpoints
+	test_assign
+	test_whereis
+	gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*"
+	test_func
 
-return 0
+	#exit and cleanup
+	gdb_exit
+    }
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 444cea01c36..6a87fee1855 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2298,6 +2298,50 @@ proc save_vars { vars body } {
     }
 }
 
+# Run BODY in the context of the caller.  After BODY is run, the procedures
+# listed in PROCS will be reset to the values they had before BODY was run.
+#
+# This is useful for providing a scope in which it is safe to temporarily
+# replace procedures.
+#
+#   save_vars { gdb_file_cmd } {
+#       # re-define gdb_file_cmd
+#   }
+
+proc save_procs { procs body } {
+    array set saved { }
+
+    # Save args and bodies of specified procs.
+    foreach proc_name $procs {
+	set proc_args [info args $proc_name]
+	set proc_body [info body $proc_name]
+
+	set saved($proc_name) [list $proc_args $proc_body]
+    }
+
+    # Run body.
+    set code [catch {uplevel 1 $body} result]
+    if { $code == 1 } {
+	global errorInfo errorCode
+	set saved_error_info $errorInfo
+	set saved_error_code $errorCode
+    }
+
+    # Restore procs.
+    foreach {proc_name value} [array get saved] {
+	set proc_args [lindex $value 0]
+	set proc_body [lindex $value 1]
+
+	eval proc $proc_name {$proc_args} {$proc_body}
+    }
+
+    if {$code == 1} {
+	return -code $code -errorinfo $saved_error_info -errorcode $saved_error_code $result
+    } else {
+	return -code $code $result
+    }
+}
+
 # Run tests in BODY with the current working directory (CWD) set to
 # DIR.  When BODY is finished, restore the original CWD.  Return the
 # result of BODY.
-- 
2.26.2

Re: [PATCH] gdb/testsuite: introduce save_procs proc

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally
Simon> defined in lib/gdb.exp.  It restores the original version of the proc at
Simon> the end.  But the problem is that if the test fails and an
Simon> exception is thrown, it won't be restored and the following tests will
Simon> be affected.

Although I'd rather we not encourage proc overriding like this, on the
other hand I guess there's nothing really wrong with what you're doing.

Simon>  # file into gdb for a dbx session. So why not just override gdb_file_cmd with the
Simon>  # right sequence of events, allowing gdb_load to do its normal thing? This way
Simon>  # remotes and simulators will work, too.

This comment indicates that this has always been a hack :)

Simon> +    # Save args and bodies of specified procs.
Simon> +    foreach proc_name $procs {
Simon> +	set proc_args [info args $proc_name]
Simon> +	set proc_body [info body $proc_name]
Simon> +
Simon> +	set saved($proc_name) [list $proc_args $proc_body]

Here you could store [list proc $proc_name $proc_args $proc_body]

Simon> +    # Restore procs.
Simon> +    foreach {proc_name value} [array get saved] {
Simon> +	set proc_args [lindex $value 0]
Simon> +	set proc_body [lindex $value 1]
Simon> +
Simon> +	eval proc $proc_name {$proc_args} {$proc_body}

Then this weird-looking construct could be replaced with the simpler

    eval $value

Tom

Re: [PATCH] gdb/testsuite: introduce save_procs proc

[Simon Marchi]

On 2020-05-28 10:43 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally
> Simon> defined in lib/gdb.exp.  It restores the original version of the proc at
> Simon> the end.  But the problem is that if the test fails and an
> Simon> exception is thrown, it won't be restored and the following tests will
> Simon> be affected.
> 
> Although I'd rather we not encourage proc overriding like this, on the
> other hand I guess there's nothing really wrong with what you're doing.

I agree.  But at least the tests that do it will be more robust with this.

(Also, I have zero interest in touching gdb.base/dbx.exp further :).)

> Simon>  # file into gdb for a dbx session. So why not just override gdb_file_cmd with the
> Simon>  # right sequence of events, allowing gdb_load to do its normal thing? This way
> Simon>  # remotes and simulators will work, too.
> 
> This comment indicates that this has always been a hack :)
> 
> Simon> +    # Save args and bodies of specified procs.
> Simon> +    foreach proc_name $procs {
> Simon> +	set proc_args [info args $proc_name]
> Simon> +	set proc_body [info body $proc_name]
> Simon> +
> Simon> +	set saved($proc_name) [list $proc_args $proc_body]
> 
> Here you could store [list proc $proc_name $proc_args $proc_body]
> 
> Simon> +    # Restore procs.
> Simon> +    foreach {proc_name value} [array get saved] {
> Simon> +	set proc_args [lindex $value 0]
> Simon> +	set proc_body [lindex $value 1]
> Simon> +
> Simon> +	eval proc $proc_name {$proc_args} {$proc_body}
> 
> Then this weird-looking construct could be replaced with the simpler
> 
>     eval $value

Good idea, `saved` can also be a flat list then.  Update patch below.


From d859ffffb28da78d8f5c6653cb11456b4ef2ce4a Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 27 May 2020 17:50:15 -0400
Subject: [PATCH] gdb/testsuite: introduce save_procs proc

The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally
defined in lib/gdb.exp.  It restores the original version of the proc at
the end.  But the problem is that if the test fails and an
exception is thrown, it won't be restored and the following tests will
be affected.

This patch introduces a `save_procs` procedure inspired by `save_vars`.
This lets the test restore the given procedures whatever happens.

The gdb.base/dbx.exp is modified to use the new proc, and save_vars to
save GDBFLAGS while at it.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (save_procs): New.
	* gdb.base/dbx.exp: Use save_procs and save_vars.

Change-Id: I5eff388dc74f6759f8ee6cf43d7e8a78502bce5a
---
 gdb/testsuite/gdb.base/dbx.exp | 45 +++++++++++++++-------------------
 gdb/testsuite/lib/gdb.exp      | 42 +++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.base/dbx.exp b/gdb/testsuite/gdb.base/dbx.exp
index 4299c44368c6..4d84b54cda29 100644
--- a/gdb/testsuite/gdb.base/dbx.exp
+++ b/gdb/testsuite/gdb.base/dbx.exp
@@ -146,12 +146,8 @@ proc dbx_reinitialize_dir { subdir } {
 # file into gdb for a dbx session. So why not just override gdb_file_cmd with the
 # right sequence of events, allowing gdb_load to do its normal thing? This way
 # remotes and simulators will work, too.
-#
-# [drow 2002-03-30]: We can restore the old gdb_file_cmd afterwards, though.
-set old_gdb_file_cmd_args [info args gdb_file_cmd]
-set old_gdb_file_cmd_body [info body gdb_file_cmd]

-proc gdb_file_cmd {arg} {
+proc dbx_gdb_file_cmd {arg} {
     global loadpath
     global loadfile
     global GDB
@@ -283,27 +279,26 @@ proc test_func { } {
     gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:${decimal}\r\n${decimal}\[ \t\]+total = sum\\(list, low, high\\);"
 }

-# Start with a fresh gdb.
-
-gdb_exit
-global GDBFLAGS
-set saved_gdbflags $GDBFLAGS
-
-set GDBFLAGS "$GDBFLAGS --dbx"
-gdb_start
-dbx_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
+save_vars { GDBFLAGS } {
+    save_procs { gdb_file_cmd } {
+	rename gdb_file_cmd {}
+	rename dbx_gdb_file_cmd gdb_file_cmd

-test_breakpoints
-test_assign
-test_whereis
-gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*"
-test_func
+	# Start with a fresh gdb.
+	gdb_exit

-#exit and cleanup
-gdb_exit
+	set GDBFLAGS "$GDBFLAGS --dbx"
+	gdb_start
+	dbx_reinitialize_dir $srcdir/$subdir
+	gdb_load ${binfile}

-set GDBFLAGS $saved_gdbflags
-eval proc gdb_file_cmd {$old_gdb_file_cmd_args} {$old_gdb_file_cmd_body}
+	test_breakpoints
+	test_assign
+	test_whereis
+	gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*"
+	test_func

-return 0
+	#exit and cleanup
+	gdb_exit
+    }
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 444cea01c36a..2a8e6d296e8d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2298,6 +2298,48 @@ proc save_vars { vars body } {
     }
 }

+# Run BODY in the context of the caller.  After BODY is run, the procedures
+# listed in PROCS will be reset to the values they had before BODY was run.
+#
+# This is useful for providing a scope in which it is safe to temporarily
+# replace procedures.
+#
+#   save_vars { gdb_file_cmd } {
+#       # re-define gdb_file_cmd
+#   }
+
+proc save_procs { procs body } {
+    set saved {}
+
+    # Save args and bodies of specified procs.
+    foreach proc_name $procs {
+	set proc_args [info args $proc_name]
+	set proc_body [info body $proc_name]
+
+	# Save it in a form that can be eval'ed straight away.
+	lappend saved [list proc $proc_name $proc_args $proc_body]
+    }
+
+    # Run body.
+    set code [catch {uplevel 1 $body} result]
+    if { $code == 1 } {
+	global errorInfo errorCode
+	set saved_error_info $errorInfo
+	set saved_error_code $errorCode
+    }
+
+    # Restore procs.
+    foreach p $saved {
+	eval $p
+    }
+
+    if {$code == 1} {
+	return -code $code -errorinfo $saved_error_info -errorcode $saved_error_code $result
+    } else {
+	return -code $code $result
+    }
+}
+
 # Run tests in BODY with the current working directory (CWD) set to
 # DIR.  When BODY is finished, restore the original CWD.  Return the
 # result of BODY.
-- 
2.26.2