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

Pedro Alves
2020-06-11T15:31:14Z

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