Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 1/6] gdb/testsuite: make with_rocm_gpu_lock transparent to return
Date: Fri, 16 Oct 2026 01:20:06 -0400	[thread overview]
Message-ID: <20261016052100.1386999-2-simon.marchi@efficios.com> (raw)
In-Reply-To: <20261016052100.1386999-1-simon.marchi@efficios.com>

A "return" in the body of with_rocm_gpu_lock does not return from the
calling proc.  Instead, it only returns from with_rocm_gpu_lock, and
execution continues after the with_rocm_gpu_lock block.  For instance,
with:

  proc do_test {} {
      with_rocm_gpu_lock {
	  if {![runto_main]} {
	      return
	  }
	  ...
      }

      # Execution continues here if runto_main fails.
  }

Also, the "return" skips the gdb_exit call at the end of
with_rocm_gpu_lock, despite the comment saying that it is there
precisely for the case where the body returns early.  Same thing if the
body throws an error, or uses "break" or "continue".  Finally, gdb_exit
is called after releasing the GPU lock, while the comment above
with_rocm_gpu_lock says it is called before.

Before commit fbb0edfe60ed ("[gdb/testsuite] Factor out proc
with_lock", 2024-05-04), with_rocm_gpu_lock caught the result of the
body, called gdb_exit, released the lock and then propagated the result
to its caller.  That commit moved the catching and propagating to the
new with_lock proc.  Since then, a "return" in the body makes with_lock
return from with_rocm_gpu_lock, skipping gdb_exit, and the caller of
with_rocm_gpu_lock continues as if nothing happened.  Commit
7730d75c2a89 ("[gdb/testsuite] Refactor exception handling",
2026-09-23) later changed with_lock to use transparent_uplevel, but
kept the same behavior.

Fix it by using try/finally and transparent_uplevel inside the with_lock
block, like other wrapper procs in the testsuite.  The block passed to
with_lock is evaluated in the frame of with_rocm_gpu_lock, so
transparent_uplevel evaluates the body in the frame of the caller of
with_rocm_gpu_lock, and propagates control flow commands to it.  The
finally clause calls gdb_exit while still holding the lock.

Add gdb.testsuite/with-rocm-gpu-lock.exp, based on
gdb.testsuite/transparent-uplevel.exp, which checks that control flow
commands in the body of with_rocm_gpu_lock behave the same as if they
were not wrapped, and that GDB is exited in all cases.  I don't know if
we really need that test, it's not like we have a test for all wrapper
procs (we have a lot of them).  But I included it to demonstrate the
problems with the current with_rocm_gpu_lock, if someone wants to toy
with it.  I wouldn't mind dropping it from the patch.

Change-Id: Ie42db74105f03b08e5722edc5b5d910b3bc250b6
---
 .../gdb.testsuite/with-rocm-gpu-lock.exp      | 90 +++++++++++++++++++
 gdb/testsuite/lib/rocm.exp                    | 20 +++--
 2 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.testsuite/with-rocm-gpu-lock.exp

diff --git a/gdb/testsuite/gdb.testsuite/with-rocm-gpu-lock.exp b/gdb/testsuite/gdb.testsuite/with-rocm-gpu-lock.exp
new file mode 100644
index 000000000000..6bbe52e203e7
--- /dev/null
+++ b/gdb/testsuite/gdb.testsuite/with-rocm-gpu-lock.exp
@@ -0,0 +1,90 @@
+# Copyright 2026 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that with_rocm_gpu_lock is transparent with respect to control flow
+# commands in its body, and that it exits GDB in all cases.
+
+load_lib rocm.exp
+
+set stmts \
+    [list \
+	 {} \
+	 "error foo" \
+	 return \
+	 "return foo" \
+	 break \
+	 continue \
+	 "return -code ok" \
+	 "return -code error" \
+	 "return -code return" \
+	 "return -code break" \
+	 "return -code continue" \
+	 "return -code ok -level 0" \
+	 "return -code ok -level 1" \
+	 "return -code ok -level 2" \
+	 "return -code error -level 0" \
+	 "return -code error -level 1" \
+	 "return -code error -level 2" \
+	 "return -code return -level 0" \
+	 "return -code return -level 1" \
+	 "return -code return -level 2" \
+	 "return -code break -level 0" \
+	 "return -code break -level 1" \
+	 "return -code break -level 2" \
+	 "return -code continue -level 0" \
+	 "return -code continue -level 1" \
+	 "return -code continue -level 2"]
+
+foreach stmt $stmts {
+    verbose -log "STMT: $stmt"
+
+    set res1 [catch {
+	{*}$stmt
+    } script_res1 opts1]
+
+    clean_restart
+
+    set res2 [catch {
+	with_rocm_gpu_lock {
+	    {*}$stmt
+	}
+    } script_res2 opts2]
+
+    gdb_assert {![info exists ::gdb_spawn_id]} "$stmt: gdb exited"
+
+    verbose -log "RES1: $res1"
+    verbose -log "RES2: $res2"
+    gdb_assert {$res1 == $res2} "$stmt: res"
+
+    verbose -log "SCRIPT_RES1: '$script_res1'"
+    verbose -log "SCRIPT_RES2: '$script_res2'"
+    gdb_assert {$script_res1 == $script_res2} "$stmt: script_res"
+
+    # Note: we skip -errorinfo and -errorstack here.  These differ for
+    # something like "error foo", and they should.  When it comes to errors,
+    # we're not trying to hide the indirection.
+    foreach key {-code -level -errorcode} {
+	set v1 {}
+	set v2 {}
+	if {[dict exists $opts1 $key]} {
+	    set v1 [dict get $opts1 $key]
+	}
+	if {[dict exists $opts2 $key]} {
+	    set v2 [dict get $opts2 $key]
+	}
+	verbose -log "KEY: $key, V1: $v1"
+	verbose -log "KEY: $key, V2: $v2"
+	gdb_assert {$v1 == $v2} "$stmt: opts: $key"
+    }
+}
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index f8a3b233faec..3a050769ab7c 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -162,15 +162,23 @@ gdb_caching_proc allow_hip_tests {} {
 # at a time.
 set gpu_lock_filename gpu-parallel.lock
 
-# Run body under the GPU lock.  Also calls gdb_exit before releasing
+# Run BODY under the GPU lock.  Also calls gdb_exit before releasing
 # the GPU lock.
+#
+# BODY is evaluated in the caller's frame, and control flow commands
+# in BODY behave as if BODY was not wrapped.  For instance, "return"
+# returns from the caller.
 
 proc with_rocm_gpu_lock { body } {
-    with_lock $::gpu_lock_filename {uplevel 1 $body}
-
-    # In case BODY returned early due to some testcase failing, and
-    # left GDB running, debugging the GPU.
-    gdb_exit
+    with_lock $::gpu_lock_filename {
+	try {
+	    transparent_uplevel $body
+	} finally {
+	    # In case BODY returned early due to some testcase failing, and
+	    # left GDB running, debugging the GPU.
+	    gdb_exit
+	}
+    }
 }
 
 # Return true if all the devices support debugging multiple processes
-- 
2.56.0


  reply	other threads:[~2026-10-16  5:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  5:20 [PATCH 0/6] Fix generating core files when GPU waves are present Simon Marchi
2026-10-16  5:20 ` Simon Marchi [this message]
2026-10-16  5:20 ` [PATCH 2/6] gdb/corefile: handle no signalled thread when writing NT_GDB_TDESC Simon Marchi
2026-10-16  5:20 ` [PATCH 3/6] gdb/amd-dbgapi: handle regno == -1 in fetch_registers and store_registers Simon Marchi
2026-10-16  5:20 ` [PATCH 4/6] gdb/linux-tdep: don't write core file notes for non-Linux threads Simon Marchi
2026-10-16  5:20 ` [PATCH 5/6] gdb/amd-dbgapi: defer TARGET_OBJECT_AUXV xfers to the beneath target Simon Marchi
2026-10-16  5:20 ` [PATCH 6/6] gdb/linux-tdep: ignore non-Linux threads in get_ptid_for_slash_proc Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261016052100.1386999-2-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox