From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id B12503893659 for ; Thu, 11 Jun 2020 15:31:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B12503893659 Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-198-Trh69pz6PEi0m-qJS-Rpkg-1; Thu, 11 Jun 2020 11:31:07 -0400 X-MC-Unique: Trh69pz6PEi0m-qJS-Rpkg-1 Received: by mail-wr1-f72.google.com with SMTP id s7so2692368wrm.16 for ; Thu, 11 Jun 2020 08:31:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=ge4qakenWq35XfJ5j1qJRyVtcqSO8beVuU3zPUwKUjY=; b=Pzx0U/Q/gHT8lZPIav+lOH5NVL7ZMtdnujU1bHX7UvNdrZEmwgesvvPs0MXt9HRERr YxA6oxtCxhbSWqrsksHUjSy3Pd7tkVv0E1A8F3N76Ci94nWbioOnCgmelX3rn3v4YQHi e4dYmFTEkBRqGAN6E5INNGCb1T+vl/HIjBBmzzACwZl3FD6dya+Sqoizdh2P8GqznDXo 50xWJyj3Rq5BH0M4zwMeMcwUqMFVSk8kdU13cYO+s8+0oSuuV04PKdTVPYfU8EVcp+fn 8TIEtDqa1GORvkx/fIlLXk9xDFcBSEJHsS3GbMqnvwsHRRV+P1lN94dTTJ9q0pqye+yx YANw== X-Gm-Message-State: AOAM533W+hX0DxnFG5JlAywLZb+9LG8qqlo4TeATfiAvXprS/hvwiXeP 8rz22+OOPG+BEZG8nVCptzbO8bV2fueaI7VHlmSH8Y1JeserELKoH6CX5q1a7Ozb5VHrH8RGYLi OICw8FSKiGVUojCpNPBO04A== X-Received: by 2002:a1c:1d44:: with SMTP id d65mr8566673wmd.179.1591889465297; Thu, 11 Jun 2020 08:31:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx1Mavv2F0pXROrE+d186szhQLlF513vbKsbR9ZBRsuMsNZsAcRc+JOxnhPVDf++lvIfxDLPA== X-Received: by 2002:a1c:1d44:: with SMTP id d65mr8566655wmd.179.1591889465074; Thu, 11 Jun 2020 08:31:05 -0700 (PDT) Received: from ?IPv6:2001:8a0:f922:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f922:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id y66sm4891435wmy.24.2020.06.11.08.31.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Jun 2020 08:31:04 -0700 (PDT) Subject: Re: [PATCH][gdb/testsuite] Don't abort testrun for invalid command in test-case To: Tom de Vries , gdb-patches@sourceware.org References: <20200611143522.GA19667@delia> From: Pedro Alves Message-ID: <7d7ce523-b58c-a77d-15be-8091feb6389a@redhat.com> Date: Thu, 11 Jun 2020 16:31:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200611143522.GA19667@delia> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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, 11 Jun 2020 15:31:16 -0000 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 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