Re: [PATCH][gdb/testsuite] Don't abort testrun for invalid command in test-case

[Pedro Alves <palves@redhat.com>]

On 6/11/20 3:35 PM, Tom de Vries wrote:
> Hi,
> 
> Say we add a call to foobar at the end of a test-case, and run the
> test-suite.  We'll run into a dejagnu error:
> ...
> ERROR: (DejaGnu) proc "foobar" does not exist.
> ...
> and the test-suite run is aborted.
> 
> It's reasonable that the test-case is aborted, but it's not reasonable that
> the testsuite run is aborted.
> 
> Problems in one test-case should not leak into other test-cases, and they
> generally don't.  The exception is the "invalid command name" problem due to
> an override of ::unknown in dejagnu's framework.exp.
> 
> Fix this by limiting the scope of dejagnu's ::unknown override.
> 
> Tested on x86_64-linux.
> 
> Any comments?

We should ask the DejaGnu folks to make the exiting on error
optional, so that in a future version we can drop the hack.

What about, say, an error due to calling a tcl proc with the wrong
arguments?  "unknown" isn't called for that.  Any idea how to
catch those?

> +# Save the unknown proc defined by dejagnu.
> +rename ::unknown ::dejagnu_unknown
> +
> +# Override the unknown proc with a gdb-local version.
> +proc unknown { args } {
> +    set script [info script]
> +
> +    set script [file dirname $script]
> +    set subdir3 [file tail $script]
> +
> +    set script [file dirname $script]
> +    set subdir2 [file tail $script]
> +
> +    set script [file dirname $script]
> +    set subdir1 [file tail $script]
> +
> +    if { $subdir1 == "gdb"
> +	 && $subdir2 == "testsuite"
> +	 && [regexp {^gdb[.]} $subdir3] } {
> +	# If we're executing a gdb test-case, skip dejagnu_unknown to prevent
> +	# it from exiting and aborting the entire test run.
> +	return [uplevel 1 ::tcl_unknown $args]
> +    }
> +

I'd think it would be cleaner to override unknown in gdb_init,
and restore in gdb_finish.  No need for filename matching that way.
Like below.  Any reason you didn't go for this instead?

From dc4d41908280b26224dc190ff9e03a07b7357b2b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 11 Jun 2020 16:11:32 +0100
Subject: [PATCH] unknown

---
 gdb/testsuite/lib/gdb.exp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9a0620a2bf1..312ff1fb366 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5193,6 +5193,15 @@ proc gdb_init { test_file_name } {
     global gdb_instances
     set gdb_instances 0
 
+    # Skip dejagnu_unknown while running a testcase to prevent it from
+    # exiting and aborting the entire test run.  Save the unknown proc
+    # defined by DejaGnu, and override the unknown proc with a
+    # gdb-local version.
+    rename ::unknown ::dejagnu_unknown
+    proc unknown { args } {
+	return [uplevel 1 ::tcl_unknown $args]
+    }
+
     return [default_gdb_init $test_file_name]
 }
 
@@ -5201,6 +5210,10 @@ proc gdb_finish { } {
     global gdb_prompt
     global cleanfiles
 
+    # Restore DejaGnu's 'unknown' version.
+    rename ::unknown ""
+    rename ::dejagnu_unknown ::unknown
+
     # Exit first, so that the files are no longer in use.
     gdb_exit
 

-- 
2.14.5