From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ni0/NZQuyWhOdQYAWB0awg (envelope-from ) for ; Tue, 16 Sep 2025 05:32:04 -0400 Received: by simark.ca (Postfix, from userid 112) id BFFFD1E047; Tue, 16 Sep 2025 05:32:04 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id BAABC1E047 for ; Tue, 16 Sep 2025 05:32:03 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2C2F23858D39 for ; Tue, 16 Sep 2025 09:32:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2C2F23858D39 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by sourceware.org (Postfix) with ESMTPS id 5E8AA3858C2A for ; Tue, 16 Sep 2025 09:31:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5E8AA3858C2A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5E8AA3858C2A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.48 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1758015084; cv=none; b=wibX7qyKAEB8tV/KKeBIfSsrsfKlttTlNGzTRaDgeDn0lxAL9W3gaGXrYouKdZ2ile3jqpXZSkc7OmEddQqv30BkUU9ekTeOR049h5vBYZfMXsp4IXi5bFAuPFn9qE7lf68NO6/20TKGj7xBh5Yr10eOhyUNpjkGkM5oXIShUOA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1758015084; c=relaxed/simple; bh=pGpi32G98DPrA2LkmH7o5NAFOktBGxBJfzOX2bL32i4=; h=Message-ID:Date:MIME-Version:Subject:From:To; b=d75BxAFOpRpprplpcEZXOqOHTXcTCeCF1wpmCOpVZszhkgq8O8Fnki+P2nJhC3Ay9uykGoi7srbVmkDCsRXWkLfQmAoSw5TYfAyh2ZARTBkXwPE29uF92KpAVcPbWgHcA6U3uShwI0P1i1lKGep7BPYx6AxpFlD5rdDr1BygOAE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5E8AA3858C2A Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-45f2fa8a1adso13214875e9.1 for ; Tue, 16 Sep 2025 02:31:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1758015083; x=1758619883; h=content-transfer-encoding:in-reply-to:content-language:references :to:from:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CbOVjGg6+fNmjI7/EmTPVUpyqdsfv4SwgNsL/n8buV0=; b=ZI0fakdcmMfqodICeUbH1/DVsGNY7kvuVSVFc4QzXeole4iiDgWCv8CoQLN66bzcwO 4rBSG4yzBUL7cF85QS05lKDsFsYwMsq34Z2NKNN/ABBajjZgpqtN9rpYn31Sg/wZc5Rw IpKZSguBFuswbDjvJ0VtRl+EVCdrbXiHM2VpH1mrLt4WD80rlskK+I5H/bZ3ywr5bYVW XkcyIc8u/D6nqFK5KVUoZ84O9hCj+OYrE+xBiWo7s9l0eCbZPOyMyniYZ64dCuHFuloK UNbQnGF0jrdM+qDZeYu1uy0/Efbv8WJHYeO1cTnIaJFMpap4Jqlb/FAf2mjow5FgeDfu sjbQ== X-Gm-Message-State: AOJu0YxUppjZAkfF4C1JO22eYZ6HpSH81edYiCcpavLSnP4Y7q2IVJJG g9rDCY8vHJaS5WNF/wuG2Mjf1xvvRXNOu5J/qbMJapkscoew8mkgMLUjIBRhlov2 X-Gm-Gg: ASbGncslTTqIh9zK/Rjj8Pmp1tcS5pAEOg4fw+5aja7d9Gv31lrhZn085r97neQx2+X 3TtZlVehlvQreTuFTyvlEIM0pX4RR+DpnrJplqUnuhLRxteiecsZ2Bfp7wsTiS1bKNWpRwx+8YZ LoFXRO4gVrbd1TR3kqO6M3prMxzU5qvbZgorg4GVk9upNl5tjrMkdZUe9FLmf1YglH76N9tzNWz fOND8HofKhBPoix7y/+G/YIVlqw2mv5wd5LWidmj1KKxQqSITrVOX4Yoc7+jTGOXZs3KC3akhWw 0mV+BPk3n2p5+QUx6Ilyvfx6EHH4SQr2ewep35HpyhHkAs2+t1jx/JjIbj/kn1JhXSWuUR3/QND 9TiDZLYxB3e/ytgMkOnrlwOoQpSjsugw0XNFYO0sgJrZ6+u0DTwuX0MTgwvGzDjY= X-Google-Smtp-Source: AGHT+IHq1jwU1Ove6IGzXrmMIkjYxKStn/St4INmqshMVZpHMiO/G1qzkZ+ILVdmHl3IiKPVr1EjAw== X-Received: by 2002:a05:600c:a11a:b0:45b:910c:adf with SMTP id 5b1f17b1804b1-45f32d4e446mr12415945e9.12.1758015082537; Tue, 16 Sep 2025 02:31:22 -0700 (PDT) Received: from ?IPV6:2001:8a0:fad8:400:81eb:cc29:e92d:bfa3? ([2001:8a0:fad8:400:81eb:cc29:e92d:bfa3]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45e037c9d91sm209465945e9.20.2025.09.16.02.31.21 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Sep 2025 02:31:21 -0700 (PDT) Message-ID: Date: Tue, 16 Sep 2025 10:31:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PATCH v2] Fix nested gdb_caching_proc with args; Fix gdb.rocm/ tests From: Pedro Alves To: gdb-patches@sourceware.org References: <20250915212841.4161603-1-pedro@palves.net> <3e29d575-22c4-42b7-a8e9-2d57b7367a49@palves.net> Content-Language: en-US In-Reply-To: <3e29d575-22c4-42b7-a8e9-2d57b7367a49@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 2025-09-15 22:54, Pedro Alves wrote: > I should have mentioned something here: > > On 2025-09-15 22:28, Pedro Alves wrote: > >> --- a/gdb/testsuite/lib/cache.exp >> +++ b/gdb/testsuite/lib/cache.exp >> @@ -24,26 +24,9 @@ proc ignore_pass { msg } { >> >> # Call proc real_name and return the result, while ignoring calls to pass. >> proc gdb_do_cache_wrap {real_name args} { >> - if { [info procs save_pass] != "" } { >> - return [uplevel 2 $real_name] >> + with_override pass ignore_pass { >> + return [$real_name {*}$args] >> } > > The previous code was using "uplevel 2" here. But I'm struggling to see why that is the > right level. > > When gdb_do_cache_wrap is called via gdb_caching_proc => gdb_do_cache, that takes us to the scope of > the caller of the gdb_caching_proc, which off hand would seem right. > > However, when gdb_do_cache_wrap is called directly by gdb.testsuite/gdb-caching-proc-consistency.exp, it > takes us to ... the caller of test_proc? > > So I'm wondering, what could we possibly want to reference from a higher level in gdb_do_cache_wrap > that not having the uplevel in gdb_do_cache_wrap would get wrong? A caching proc IMO shouldn't be doing > something like taking the name of a variable as argument instead of a value, as then the procedure wouldn't > be guaranteed to be idempotent? So I dropped the uplevel, and found that that doesn't cause any > testsuite regression. But maybe I'm missing something. If I am, it'd be nice to add a testcase for it. I guess that is sort of a tangent. I should preserve the uplevel so that the patch does just one thing. A change to that detail can always be done separately. Here's a new version of the patch without that change. With that out of the way, this patch should be a lot more obvious. I'd like to merge it soon to unbreak the gdb.rocm/ tests. === 8< === >From 50e4abd32c3f162f6221bdb4946ef7c18f29d7ed Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 15 Sep 2025 22:12:28 +0100 Subject: [PATCH v2] Fix nested gdb_caching_proc with args; Fix gdb.rocm/ tests Commit d09eba07 ("Make get_compiler_info use gdb_caching_proc") regressed the gdb.rocm/ testsuite. E.g.: Running /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.exp ... ERROR: tcl error sourcing /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.rocm/simple.exp. ERROR: tcl error code TCL WRONGARGS ERROR: wrong # args: should be "gdb_real__get_compiler_info_1 language" while executing "gdb_real__get_compiler_info_1" ("uplevel" body line 1) invoked from within "uplevel 2 $real_name" (procedure "gdb_do_cache_wrap" line 3) invoked from within "gdb_do_cache_wrap $real_name {*}$args" (procedure "gdb_do_cache" line 98) invoked from within This is actually a latent problem in gdb_do_cache_wrap, introduced in: commit 71f1ab80f1aabd70bce526635f84c7b849e8a0f4 CommitDate: Mon Mar 6 16:49:19 2023 +0100 [gdb/testsuite] Allow args in gdb_caching_proc This change: # Call proc real_name and return the result, while ignoring calls to pass. -proc gdb_do_cache_wrap {real_name} { +proc gdb_do_cache_wrap {real_name args} { if { [info procs save_pass] != "" } { return [uplevel 2 $real_name] <<<<<<<<<<<<<<<<<<<<<<< HERE } @@ -31,7 +31,7 @@ proc gdb_do_cache_wrap {real_name} { rename pass save_pass rename ignore_pass pass - set code [catch {uplevel 2 $real_name} result] + set code [catch {uplevel 2 [list $real_name {*}$args]} result] Missed updating the line marked with HERE above, to pass down $args. So the case of a caching proc calling another caching proc with args isn't handled correctly. We could fix this by fixing the HERE line like so: - return [uplevel 2 $real_name] + return [uplevel 2 [list $real_name {*}$args]] However, we have with_override nowadays that we can use here which eliminates the duplicated logic, which was what was missed originally. A new test that exposes the problem is added to gdb.testsuite/gdb-caching-proc.exp. This also adds a new test to gdb.testsuite/with-override.exp that I think was missing, making sure that the inner foo override restores the outer foo override. Tested-By: Simon Marchi Change-Id: I8b2a7366bf910902fe5f547bde58c3b475bf5133 --- .../gdb.testsuite/gdb-caching-proc.exp | 21 +++++++++++++------ gdb/testsuite/gdb.testsuite/with-override.exp | 4 ++++ gdb/testsuite/lib/cache.exp | 21 ++----------------- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/gdb/testsuite/gdb.testsuite/gdb-caching-proc.exp b/gdb/testsuite/gdb.testsuite/gdb-caching-proc.exp index f9610afd076..6b46b1c555a 100644 --- a/gdb/testsuite/gdb.testsuite/gdb-caching-proc.exp +++ b/gdb/testsuite/gdb.testsuite/gdb-caching-proc.exp @@ -22,23 +22,32 @@ gdb_caching_proc gdb_testsuite_gdb_caching_proc_exp_arg { arg } { return $arg } +gdb_caching_proc gdb_testsuite_gdb_caching_proc_exp_arg_nested { arg } { + incr ::count + return [gdb_testsuite_gdb_caching_proc_exp_arg $arg] +} + +# List of "expected $::count after running expression" and +# "expression". set assertions { - { [gdb_testsuite_gdb_caching_proc_exp_noarg] == 1 } - { [gdb_testsuite_gdb_caching_proc_exp_arg 1] == 1 } - { [gdb_testsuite_gdb_caching_proc_exp_arg "foo foo"] == "foo foo" } + 1 { [gdb_testsuite_gdb_caching_proc_exp_noarg] == 1 } + 1 { [gdb_testsuite_gdb_caching_proc_exp_arg 1] == 1 } + 1 { [gdb_testsuite_gdb_caching_proc_exp_arg "foo foo"] == "foo foo" } + 1 { [gdb_testsuite_gdb_caching_proc_exp_arg_nested "foo foo"] == "foo foo" } + 2 { [gdb_testsuite_gdb_caching_proc_exp_arg_nested "bar bar"] == "bar bar" } } set assertion_nr 0 -foreach assertion $assertions { +foreach {expected_count assertion} $assertions { with_test_prefix $assertion_nr { set ::count 0 gdb_assert $assertion - gdb_assert { $::count == 1 } + gdb_assert { $::count == $expected_count } with_test_prefix cached { gdb_assert $assertion - gdb_assert { $::count == 1 } + gdb_assert { $::count == $expected_count } } } incr assertion_nr diff --git a/gdb/testsuite/gdb.testsuite/with-override.exp b/gdb/testsuite/gdb.testsuite/with-override.exp index 1a4ee6adbbb..2a316f904e4 100644 --- a/gdb/testsuite/gdb.testsuite/with-override.exp +++ b/gdb/testsuite/gdb.testsuite/with-override.exp @@ -42,6 +42,10 @@ with_test_prefix no-args { gdb_assert { [foo] == 2 } } } + + with_test_prefix "foo1 again" { + gdb_assert { [foo] == 1 } + } } with_test_prefix after { diff --git a/gdb/testsuite/lib/cache.exp b/gdb/testsuite/lib/cache.exp index 6ca3f183f9f..4ebb8259957 100644 --- a/gdb/testsuite/lib/cache.exp +++ b/gdb/testsuite/lib/cache.exp @@ -24,26 +24,9 @@ proc ignore_pass { msg } { # Call proc real_name and return the result, while ignoring calls to pass. proc gdb_do_cache_wrap {real_name args} { - if { [info procs save_pass] != "" } { - return [uplevel 2 $real_name] + with_override pass ignore_pass { + return [uplevel 2 [list $real_name {*}$args]] } - - rename pass save_pass - rename ignore_pass pass - - set code [catch {uplevel 2 [list $real_name {*}$args]} result] - - rename pass ignore_pass - rename save_pass pass - - if {$code == 1} { - global errorInfo errorCode - return -code error -errorinfo $errorInfo -errorcode $errorCode $result - } elseif {$code > 1} { - return -code $code $result - } - - return $result } # Global written to by gdb_exit_called proc. Is set to true to base-commit: d09eba07ca013e6b95eeafd67d79c32ebf6f28eb -- 2.51.0