diff mbox series

[08/11,gdb/testsuite] Refactor exception handling in gdb_test_multiple

Message ID 20260824135855.1195963-9-tdevries@suse.de
State New
Headers
Series Refactor exception handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Aug. 24, 2026, 1:58 p.m. UTC 
  Simplify gdb_test_multiple using return -options and try/finally.

In the process, we also try to fix PR34553.

I've added a test to verify this, but it'll be only useful after remote_expect
gets fixed [1].

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34552
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34553

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81691
---
 gdb/testsuite/gdb.testsuite/gdb-test.exp | 51 ++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                | 37 +++++++++--------
 2 files changed, 69 insertions(+), 19 deletions(-)

Comments

Tom de Vries Aug. 25, 2026, 7:29 a.m. UTC | #1 
On 8/24/26 3:58 PM, Tom de Vries wrote:
> implify gdb_test_multiple using return -options and try/finally.
> 
> In the process, we also try to fix PR34553.
> 
> I've added a test to verify this, but it'll be only useful after remote_expect
> gets fixed [1].
> 

I heard back from the dejagnu maintainers, who acknowledged the bug.

The current stable release is 1.6.3.  Since the currently envisioned fix 
breaks 8.4 support, it's scheduled for 1.6.5 rather than 1.6.4.

I proposed a fix that keeps backward compatibility, allowing it to be 
fixed in 1.6.4.

We could copy a version into lib/future.exp to have it fixed faster, but 
I don't see the need for this.  AFAIK we're currently not relying on this.

Thanks,
- Tom

> Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=34552
> Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=34553
> 
> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81691
Keith Seitz Sept. 1, 2026, 4:13 p.m. UTC | #2 
Hi,

On 8/24/26 6:58 AM, Tom de Vries wrote:
> Simplify gdb_test_multiple using return -options and try/finally.
> 
> In the process, we also try to fix PR34553.
> 
> I've added a test to verify this, but it'll be only useful after remote_expect
> gets fixed [1].
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34552
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34553
> 
> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81691
> ---
>   gdb/testsuite/gdb.testsuite/gdb-test.exp | 51 ++++++++++++++++++++++++
>   gdb/testsuite/lib/gdb.exp                | 37 +++++++++--------
>   2 files changed, 69 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.testsuite/gdb-test.exp b/gdb/testsuite/gdb.testsuite/gdb-test.exp
> index dab65f21bdb..231f0755f58 100644
> --- a/gdb/testsuite/gdb.testsuite/gdb-test.exp
> +++ b/gdb/testsuite/gdb.testsuite/gdb-test.exp
> @@ -49,6 +49,57 @@ with_test_prefix "cmd with trailing control code" {
>       }
>   }
>   
> +foreach_with_prefix variant {0 1 2} {
> +    proc level_2_inner {} {
> +	if {$::variant == 0} {
> +	    return -level 2 "return_level_2"
> +	} elseif {$::variant == 1} {
> +	    send_gdb "print 1\n"
> +	    set prompt_re [string_to_regexp "(gdb) "]
> +	    remote_expect host 10 {
> +		-re " = 1\r\n$prompt_re$" {
> +		    return -level 2 "return_level_2"
> +		}
> +	    }
> +	} else {
> +	    gdb_test_multiple "print 1" "" {
> +		-re -wrap " = 1" {
> +		    return -level 2 "return_level_2"
> +		}
> +	    }
> +	}
> +	return "inner"
> +    }
> +
> +    proc level_2_outer {} {
> +	level_2_inner
> +	return "outer"
> +    }
> +
> +    try {
> +	set res "initial"
> +	set res [level_2_outer]
> +    } finally {
> +	if {$variant == 0} {
> +	    # trivial case.
> +	    gdb_assert {$res == "return_level_2"}
> +	} elseif {$variant == 1} {
> +	    # Remove_expect case.

Typo? Should this be "remote_expect" case?

Keith

> +	    if {$res == "outer"} {
> +		# Dejagnu bug.
> +		# https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81691
> +		setup_xfail *-*-*
> +	    }
> +	    gdb_assert {$res == "return_level_2"}
> +	    set variant1_res $res
> +	} else {
> +	    # Gdb_test_multiple case. This should work if the remote_expect
> +	    # case works.
> +	    gdb_assert {$res == $variant1_res}
> +	}
> +    }
> +}
> +
>   # Change the prompt.
>   set prompt "(GDB) "
>   set prompt_re "\\(GDB\\) $"
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.testsuite/gdb-test.exp b/gdb/testsuite/gdb.testsuite/gdb-test.exp
index dab65f21bdb..231f0755f58 100644
--- a/gdb/testsuite/gdb.testsuite/gdb-test.exp
+++ b/gdb/testsuite/gdb.testsuite/gdb-test.exp
@@ -49,6 +49,57 @@  with_test_prefix "cmd with trailing control code" {
     }
 }
 
+foreach_with_prefix variant {0 1 2} {
+    proc level_2_inner {} {
+	if {$::variant == 0} {
+	    return -level 2 "return_level_2"
+	} elseif {$::variant == 1} {
+	    send_gdb "print 1\n"
+	    set prompt_re [string_to_regexp "(gdb) "]
+	    remote_expect host 10 {
+		-re " = 1\r\n$prompt_re$" {
+		    return -level 2 "return_level_2"
+		}
+	    }
+	} else {
+	    gdb_test_multiple "print 1" "" {
+		-re -wrap " = 1" {
+		    return -level 2 "return_level_2"
+		}
+	    }
+	}
+	return "inner"
+    }
+
+    proc level_2_outer {} {
+	level_2_inner
+	return "outer"
+    }
+
+    try {
+	set res "initial"
+	set res [level_2_outer]
+    } finally {
+	if {$variant == 0} {
+	    # trivial case.
+	    gdb_assert {$res == "return_level_2"}
+	} elseif {$variant == 1} {
+	    # Remove_expect case.
+	    if {$res == "outer"} {
+		# Dejagnu bug.
+		# https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81691
+		setup_xfail *-*-*
+	    }
+	    gdb_assert {$res == "return_level_2"}
+	    set variant1_res $res
+	} else {
+	    # Gdb_test_multiple case. This should work if the remote_expect
+	    # case works.
+	    gdb_assert {$res == $variant1_res}
+	}
+    }
+}
+
 # Change the prompt.
 set prompt "(GDB) "
 set prompt_re "\\(GDB\\) $"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 479d1229142..99468ac3ef3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1369,7 +1369,6 @@  proc gdb_test_multiple { command message args } {
 	send_user "Message is \"$message\"\n"
     }
 
-    set result -1
     set string "${command}\n"
     if { $command != "" } {
 	set multi_line_re "\[\r\n\] *>"
@@ -1574,25 +1573,25 @@  proc gdb_test_multiple { command message args } {
     }
     set gdb_test_name "$message"
 
-    set result 0
-    set code [catch {gdb_expect $code} string]
-
-    # Clean up the gdb_test_name variable.  If we had a
-    # previous value then restore it, otherwise, delete the variable
-    # from the parent scope.
-    if { [info exists gdb_test_name_old] } {
-	set gdb_test_name "$gdb_test_name_old"
-    } else {
-	unset gdb_test_name
-    }
-
-    if {$code == 1} {
-	global errorInfo errorCode
-	return -code error -errorinfo $errorInfo -errorcode $errorCode $string
-    } elseif {$code > 1} {
-	return -code $code $string
+    try {
+	if {[info exists result]} {
+	    error "result set but not used"
+	}
+	set result 0
+	if {[catch {gdb_expect $code} string opts] == 0} {
+	    return $result
+	}
+	return -options [dict incr opts -level] $string
+    } finally {
+	# Clean up the gdb_test_name variable.  If we had a
+	# previous value then restore it, otherwise, delete the variable
+	# from the parent scope.
+	if { [info exists gdb_test_name_old] } {
+	    set gdb_test_name "$gdb_test_name_old"
+	} else {
+	    unset gdb_test_name
+	}
     }
-    return $result
 }
 
 # Usage: gdb_test_multiline NAME INPUT RESULT {INPUT RESULT} ...