From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by sourceware.org (Postfix) with ESMTPS id C3999386F012 for ; Thu, 28 May 2020 17:10:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C3999386F012 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 55381284A47; Thu, 28 May 2020 13:10:52 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id Vxx3--8C8vBt; Thu, 28 May 2020 13:10:51 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id D79DB2845F5; Thu, 28 May 2020 13:10:51 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com D79DB2845F5 X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id XkCZk0rsXqCR; Thu, 28 May 2020 13:10:51 -0400 (EDT) Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) by mail.efficios.com (Postfix) with ESMTPSA id AEFF8284DED; Thu, 28 May 2020 13:10:51 -0400 (EDT) Subject: Re: [PATCH] gdb/testsuite: introduce save_procs proc To: Tom Tromey , Simon Marchi via Gdb-patches References: <20200527215015.15106-1-simon.marchi@efficios.com> <87h7w0i0hy.fsf@tromey.com> From: Simon Marchi Message-ID: Date: Thu, 28 May 2020 13:10:50 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <87h7w0i0hy.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 May 2020 17:10:54 -0000 On 2020-05-28 10:43 a.m., Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches writes: > > Simon> The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally > Simon> defined in lib/gdb.exp. It restores the original version of the proc at > Simon> the end. But the problem is that if the test fails and an > Simon> exception is thrown, it won't be restored and the following tests will > Simon> be affected. > > Although I'd rather we not encourage proc overriding like this, on the > other hand I guess there's nothing really wrong with what you're doing. I agree. But at least the tests that do it will be more robust with this. (Also, I have zero interest in touching gdb.base/dbx.exp further :).) > Simon> # file into gdb for a dbx session. So why not just override gdb_file_cmd with the > Simon> # right sequence of events, allowing gdb_load to do its normal thing? This way > Simon> # remotes and simulators will work, too. > > This comment indicates that this has always been a hack :) > > Simon> + # Save args and bodies of specified procs. > Simon> + foreach proc_name $procs { > Simon> + set proc_args [info args $proc_name] > Simon> + set proc_body [info body $proc_name] > Simon> + > Simon> + set saved($proc_name) [list $proc_args $proc_body] > > Here you could store [list proc $proc_name $proc_args $proc_body] > > Simon> + # Restore procs. > Simon> + foreach {proc_name value} [array get saved] { > Simon> + set proc_args [lindex $value 0] > Simon> + set proc_body [lindex $value 1] > Simon> + > Simon> + eval proc $proc_name {$proc_args} {$proc_body} > > Then this weird-looking construct could be replaced with the simpler > > eval $value Good idea, `saved` can also be a flat list then. Update patch below. >From d859ffffb28da78d8f5c6653cb11456b4ef2ce4a Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 27 May 2020 17:50:15 -0400 Subject: [PATCH] gdb/testsuite: introduce save_procs proc The test gdb.base/dbx.exp redefines gdb_file_cmd, which is normally defined in lib/gdb.exp. It restores the original version of the proc at the end. But the problem is that if the test fails and an exception is thrown, it won't be restored and the following tests will be affected. This patch introduces a `save_procs` procedure inspired by `save_vars`. This lets the test restore the given procedures whatever happens. The gdb.base/dbx.exp is modified to use the new proc, and save_vars to save GDBFLAGS while at it. gdb/testsuite/ChangeLog: * lib/gdb.exp (save_procs): New. * gdb.base/dbx.exp: Use save_procs and save_vars. Change-Id: I5eff388dc74f6759f8ee6cf43d7e8a78502bce5a --- gdb/testsuite/gdb.base/dbx.exp | 45 +++++++++++++++------------------- gdb/testsuite/lib/gdb.exp | 42 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/gdb/testsuite/gdb.base/dbx.exp b/gdb/testsuite/gdb.base/dbx.exp index 4299c44368c6..4d84b54cda29 100644 --- a/gdb/testsuite/gdb.base/dbx.exp +++ b/gdb/testsuite/gdb.base/dbx.exp @@ -146,12 +146,8 @@ proc dbx_reinitialize_dir { subdir } { # file into gdb for a dbx session. So why not just override gdb_file_cmd with the # right sequence of events, allowing gdb_load to do its normal thing? This way # remotes and simulators will work, too. -# -# [drow 2002-03-30]: We can restore the old gdb_file_cmd afterwards, though. -set old_gdb_file_cmd_args [info args gdb_file_cmd] -set old_gdb_file_cmd_body [info body gdb_file_cmd] -proc gdb_file_cmd {arg} { +proc dbx_gdb_file_cmd {arg} { global loadpath global loadfile global GDB @@ -283,27 +279,26 @@ proc test_func { } { gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:${decimal}\r\n${decimal}\[ \t\]+total = sum\\(list, low, high\\);" } -# Start with a fresh gdb. - -gdb_exit -global GDBFLAGS -set saved_gdbflags $GDBFLAGS - -set GDBFLAGS "$GDBFLAGS --dbx" -gdb_start -dbx_reinitialize_dir $srcdir/$subdir -gdb_load ${binfile} +save_vars { GDBFLAGS } { + save_procs { gdb_file_cmd } { + rename gdb_file_cmd {} + rename dbx_gdb_file_cmd gdb_file_cmd -test_breakpoints -test_assign -test_whereis -gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*" -test_func + # Start with a fresh gdb. + gdb_exit -#exit and cleanup -gdb_exit + set GDBFLAGS "$GDBFLAGS --dbx" + gdb_start + dbx_reinitialize_dir $srcdir/$subdir + gdb_load ${binfile} -set GDBFLAGS $saved_gdbflags -eval proc gdb_file_cmd {$old_gdb_file_cmd_args} {$old_gdb_file_cmd_body} + test_breakpoints + test_assign + test_whereis + gdb_test "file average.c:1" "1\[ \t\]+/. This is a sample program.*" + test_func -return 0 + #exit and cleanup + gdb_exit + } +} diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 444cea01c36a..2a8e6d296e8d 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -2298,6 +2298,48 @@ proc save_vars { vars body } { } } +# Run BODY in the context of the caller. After BODY is run, the procedures +# listed in PROCS will be reset to the values they had before BODY was run. +# +# This is useful for providing a scope in which it is safe to temporarily +# replace procedures. +# +# save_vars { gdb_file_cmd } { +# # re-define gdb_file_cmd +# } + +proc save_procs { procs body } { + set saved {} + + # Save args and bodies of specified procs. + foreach proc_name $procs { + set proc_args [info args $proc_name] + set proc_body [info body $proc_name] + + # Save it in a form that can be eval'ed straight away. + lappend saved [list proc $proc_name $proc_args $proc_body] + } + + # Run body. + set code [catch {uplevel 1 $body} result] + if { $code == 1 } { + global errorInfo errorCode + set saved_error_info $errorInfo + set saved_error_code $errorCode + } + + # Restore procs. + foreach p $saved { + eval $p + } + + if {$code == 1} { + return -code $code -errorinfo $saved_error_info -errorcode $saved_error_code $result + } else { + return -code $code $result + } +} + # Run tests in BODY with the current working directory (CWD) set to # DIR. When BODY is finished, restore the original CWD. Return the # result of BODY. -- 2.26.2