Message ID | 20221122155546.599061-1-simon.marchi@efficios.com |
---|---|
State | Committed |
Commit | d56614a9925791b15d7052eb4c798ab24a724611 |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E422D385800C for <patchwork@sourceware.org>; Tue, 22 Nov 2022 15:56:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E422D385800C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1669132607; bh=QkPlQ3MNYobHhaRKUIxTM2SlBjn9NFXfRpngGZa4HDM=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=WhlKvK0AoSMbfMrr7bYc3wT8sP31df/m5AFeo283pvEHOKJ8E1tUbNOs5czSPjA/r 7f9ZDe21Y6N+Ggrk6y47CLlS8VBHm23M4dmASbN+v/iE7WntvaduD0MONhiYW8SX8U KmuqfEl/qyod7QdBslDk07sV/o0AuPk6LwNMli64= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 44CD03858D35 for <gdb-patches@sourceware.org>; Tue, 22 Nov 2022 15:56:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 44CD03858D35 X-ASG-Debug-ID: 1669132548-0c856e02a126e660001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id fq5A3LyehVQ3YbHV (version=TLSv1 cipher=AES128-SHA bits=128 verify=NO); Tue, 22 Nov 2022 10:55:48 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from smarchi-efficios.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 0FB4A441B21; Tue, 22 Nov 2022 10:55:48 -0500 (EST) X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH 1/2] =?utf-8?q?gdb/testsuite=3A_make_gdb=5Ftest=5Fmultiple_r?= =?utf-8?q?eturn_immediately_if=C2=A0send=5Fgdb_fails?= Date: Tue, 22 Nov 2022 10:55:45 -0500 X-ASG-Orig-Subj: [PATCH 1/2] =?utf-8?q?gdb/testsuite=3A_make_gdb=5Ftest=5Fmu?= =?utf-8?q?ltiple_return_immediately_if=C2=A0send=5Fgdb_fails?= Message-Id: <20221122155546.599061-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1669132548 X-Barracuda-Encrypted: AES128-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 2971 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=5.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.102320 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-3498.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Simon Marchi <simon.marchi@efficios.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[1/2] gdb/testsuite: make gdb_test_multiple return immediately if send_gdb fails
|
|
Commit Message
Simon Marchi
Nov. 22, 2022, 3:55 p.m. UTC
From: Simon Marchi <simon.marchi@polymtl.ca>
In the failure seen by Philippe here:
https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/
... the testsuite only outputs PASSes, and an ERROR, resulting from an
uncaught exception. This is a bit sneaky, because ERRORs are not
reported in the test summary. In certain circumstances, it can be easy
to miss.
Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes. But
this is only if it manages to send the command, and it's that command
that crashes GDB. Here, the ERROR is due to the fact that GDB had
already crashed by the time we entered gdb_test_multiple and tried to
send a command. GDB was crashed by the previous "file" command, sent by
gdb_unload. Because gdb_unload uses bare expect, it didn't record a
test failure when crashing GDB (this will be addressed separately).
In this patch, I propose to make gdb_test_multiple call unresolved
directly and return -1 send_gdb fails. This way, if GDB is already
crashed by the time we enter gdb_test_multiple, it will leave a trace in
the test results in the form of an UNRESOLVED. It will also spare us of
the not-so-useful-in-my-opinion TCL backtrace.
Before, it looks like:
ERROR: Couldn't send python print(objfile.filename) to GDB.
ERROR: : spawn id exp9 not open
while executing
"expect {
-i exp9 -timeout 10
-re ".*A problem internal to GDB has been detected" {
fail "$message (GDB internal error)"
gdb_internal_error..."
("uplevel" body line 1)
invoked from within
"uplevel $body" NONE : spawn id exp9 not open
And after:
Couldn't send python print(objfile.filename) to GDB.
UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded
Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba
---
gdb/testsuite/lib/gdb.exp | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Comments
Thanks for this, it will for sure help me in the future to not miss anymore some regressions I introduce. One small question: Is a GDB crash the only reason for which we could have a message such as: Couldn't send python print(objfile.filename) to GDB. ? Possibly better then to indicate that GDB crashed (on an internal error) ? Or if there are some other reasons, it might be good to give more details about the reason ? Note that if this is not straightforward to do, not a big deal as just making the problem visible with UNRESOLVED should draw the attention and the detailed log will show the crash I guess. Thanks Philippe On Tue, 2022-11-22 at 10:55 -0500, Simon Marchi via Gdb-patches wrote: > From: Simon Marchi <simon.marchi@polymtl.ca> > > In the failure seen by Philippe here: > > https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/ > > ... the testsuite only outputs PASSes, and an ERROR, resulting from an > uncaught exception. This is a bit sneaky, because ERRORs are not > reported in the test summary. In certain circumstances, it can be easy > to miss. > > Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes. But > this is only if it manages to send the command, and it's that command > that crashes GDB. Here, the ERROR is due to the fact that GDB had > already crashed by the time we entered gdb_test_multiple and tried to > send a command. GDB was crashed by the previous "file" command, sent by > gdb_unload. Because gdb_unload uses bare expect, it didn't record a > test failure when crashing GDB (this will be addressed separately). > > In this patch, I propose to make gdb_test_multiple call unresolved > directly and return -1 send_gdb fails. This way, if GDB is already > crashed by the time we enter gdb_test_multiple, it will leave a trace in > the test results in the form of an UNRESOLVED. It will also spare us of > the not-so-useful-in-my-opinion TCL backtrace. > > Before, it looks like: > > ERROR: Couldn't send python print(objfile.filename) to GDB. > ERROR: : spawn id exp9 not open > while executing > "expect { > -i exp9 -timeout 10 > -re ".*A problem internal to GDB has been detected" { > fail "$message (GDB internal error)" > gdb_internal_error..." > ("uplevel" body line 1) > invoked from within > "uplevel $body" NONE : spawn id exp9 not open > > And after: > > Couldn't send python print(objfile.filename) to GDB. > UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded > > Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba > --- > gdb/testsuite/lib/gdb.exp | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 7d05fbe557bb..fcd54c88f251 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } { > if { $foo < [expr $len - 1] } { > set str [string range "$string" 0 $foo] > if { [send_gdb "$str"] != "" } { > - perror "Couldn't send $command to GDB." > + verbose -log "Couldn't send $command to GDB." > + unresolved $message > + return -1 > } > # since we're checking if each line of the multi-line > # command are 'accepted' by GDB here, > @@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } { > } > if { "$string" != "" } { > if { [send_gdb "$string"] != "" } { > - perror "Couldn't send $command to GDB." > + verbose -log "Couldn't send $command to GDB." > + unresolved $message > + return -1 > } > } > }
On 11/23/22 05:59, Philippe Waroquiers wrote: > Thanks for this, it will for sure help me in the future to not miss anymore > some regressions I introduce. > > One small question: > Is a GDB crash the only reason for which we could have a message such as: > Couldn't send python print(objfile.filename) to GDB. > ? > Possibly better then to indicate that GDB crashed (on an internal error) ? > Or if there are some other reasons, it might be good to give more details about the > reason ? If send_gdb fails, it's generally because the underlying send (the expect proc) has failed, because GDB's spawn id is no longer valid. And that is normally due to a crash. But the reality is that we do not know for sure, it could also be a testsuite problem, where we closed GDB and failed to reinitialize $gdb_spawn_id. So saying GDB crashed could also give you a wrong lead. I prefer to stick with what we know. > Note that if this is not straightforward to do, not a big deal > as just making the problem visible with UNRESOLVED should draw the attention > and the detailed log will show the crash I guess. I think that's the important part, it's the previous command that caused the crash, that one should show up in the logs. Looking at lib/gdb.exp there are many more uses of gdb_expect that could be converted to gdb_test_multiple. Simon
On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote: > From: Simon Marchi <simon.marchi@polymtl.ca> > > In the failure seen by Philippe here: > > https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/ > > ... the testsuite only outputs PASSes, and an ERROR, resulting from an > uncaught exception. This is a bit sneaky, because ERRORs are not > reported in the test summary. In certain circumstances, it can be easy > to miss. > > Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes. But > this is only if it manages to send the command, and it's that command > that crashes GDB. Here, the ERROR is due to the fact that GDB had > already crashed by the time we entered gdb_test_multiple and tried to > send a command. GDB was crashed by the previous "file" command, sent by > gdb_unload. Because gdb_unload uses bare expect, it didn't record a > test failure when crashing GDB (this will be addressed separately). > > In this patch, I propose to make gdb_test_multiple call unresolved > directly and return -1 send_gdb fails. This way, if GDB is already > crashed by the time we enter gdb_test_multiple, it will leave a trace in > the test results in the form of an UNRESOLVED. It will also spare us of spare us of the -> spare us the ? > the not-so-useful-in-my-opinion TCL backtrace. > Agreed. > Before, it looks like: > > ERROR: Couldn't send python print(objfile.filename) to GDB. > ERROR: : spawn id exp9 not open > while executing > "expect { > -i exp9 -timeout 10 > -re ".*A problem internal to GDB has been detected" { > fail "$message (GDB internal error)" > gdb_internal_error..." > ("uplevel" body line 1) > invoked from within > "uplevel $body" NONE : spawn id exp9 not open > > And after: > > Couldn't send python print(objfile.filename) to GDB. > UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded > LGTM, thanks for doing this. Thanks, - Tom > Change-Id: I72af8dc0d687826fc3f76911c27a9e5f91b677ba > --- > gdb/testsuite/lib/gdb.exp | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 7d05fbe557bb..fcd54c88f251 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } { > if { $foo < [expr $len - 1] } { > set str [string range "$string" 0 $foo] > if { [send_gdb "$str"] != "" } { > - perror "Couldn't send $command to GDB." > + verbose -log "Couldn't send $command to GDB." > + unresolved $message > + return -1 > } > # since we're checking if each line of the multi-line > # command are 'accepted' by GDB here, > @@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } { > } > if { "$string" != "" } { > if { [send_gdb "$string"] != "" } { > - perror "Couldn't send $command to GDB." > + verbose -log "Couldn't send $command to GDB." > + unresolved $message > + return -1 > } > } > }
On 11/29/22 11:21, Tom de Vries wrote: > On 11/22/22 16:55, Simon Marchi via Gdb-patches wrote: >> From: Simon Marchi <simon.marchi@polymtl.ca> >> >> In the failure seen by Philippe here: >> >> https://inbox.sourceware.org/gdb-patches/20221120173024.3647464-1-philippe.waroquiers@skynet.be/ >> >> ... the testsuite only outputs PASSes, and an ERROR, resulting from an >> uncaught exception. This is a bit sneaky, because ERRORs are not >> reported in the test summary. In certain circumstances, it can be easy >> to miss. >> >> Normally, gdb_test_multiple outputs an UNRESOLVED when GDB crashes. But > this is only if it manages to send the command, and it's that command >> that crashes GDB. Here, the ERROR is due to the fact that GDB had >> already crashed by the time we entered gdb_test_multiple and tried to >> send a command. GDB was crashed by the previous "file" command, sent by >> gdb_unload. Because gdb_unload uses bare expect, it didn't record a >> test failure when crashing GDB (this will be addressed separately). >> >> In this patch, I propose to make gdb_test_multiple call unresolved >> directly and return -1 send_gdb fails. This way, if GDB is already >> crashed by the time we enter gdb_test_multiple, it will leave a trace in >> the test results in the form of an UNRESOLVED. It will also spare us of > > spare us of the -> spare us the ? Fixed. >> the not-so-useful-in-my-opinion TCL backtrace. >> > > Agreed. > >> Before, it looks like: >> >> ERROR: Couldn't send python print(objfile.filename) to GDB. >> ERROR: : spawn id exp9 not open >> while executing >> "expect { >> -i exp9 -timeout 10 >> -re ".*A problem internal to GDB has been detected" { >> fail "$message (GDB internal error)" >> gdb_internal_error..." >> ("uplevel" body line 1) >> invoked from within >> "uplevel $body" NONE : spawn id exp9 not open >> >> And after: >> >> Couldn't send python print(objfile.filename) to GDB. >> UNRESOLVED: gdb.python/py-objfile.exp: objfile.filename after objfile is unloaded >> > > LGTM, thanks for doing this. Thanks, will push. Simon
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 7d05fbe557bb..fcd54c88f251 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -1161,7 +1161,9 @@ proc gdb_test_multiple { command message args } { if { $foo < [expr $len - 1] } { set str [string range "$string" 0 $foo] if { [send_gdb "$str"] != "" } { - perror "Couldn't send $command to GDB." + verbose -log "Couldn't send $command to GDB." + unresolved $message + return -1 } # since we're checking if each line of the multi-line # command are 'accepted' by GDB here, @@ -1180,7 +1182,9 @@ proc gdb_test_multiple { command message args } { } if { "$string" != "" } { if { [send_gdb "$string"] != "" } { - perror "Couldn't send $command to GDB." + verbose -log "Couldn't send $command to GDB." + unresolved $message + return -1 } } }