From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by sourceware.org (Postfix) with ESMTPS id A5EBD3858D33 for ; Tue, 10 Jan 2023 19:56:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A5EBD3858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f45.google.com with SMTP id j7so7927384wrn.9 for ; Tue, 10 Jan 2023 11:56:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zvFAZIIGuF/zcVvN8a5UdFNo2eJqez4gNynQyziH49I=; b=PR9Xvo+11b3GMA5FI354VXZK1ZhfvVOB/gk2jfIVAXQTwRwtJ23hbmbpPz/FugDbtW LsR7J6pQu/ayMB9qPk4vf1uDYrNo9H2EgB4iP1V+NcxV7gnpNlOL4G31wp1oR8jgJqQj J3osOLRjSS0YmAZzX6tV3iCvHjlH2gIwxE7JOfAlNmf8UV/yahEIlp1uHblvFBjhdM3C o7Fuq/vz5NPXGru2WZFcxRc02IiAhQ0n3i6RHuyREXa/mAH0HCrBRfI2xh7PcjQFvJJn uMQLGgcSqMC6LtscoE2OnxG4QsUSJWeqpgjr8EHzQPf1ZXkD6SC5a1PCcoyfBIMRT7+d 1euw== X-Gm-Message-State: AFqh2kphlY/WKu7I8mEbYBvootj0CUSyVOrDURcriXx22qiuI6cI28gA RBYFu9AAwOnodY/0SCS/gnvShHXBG1stSA== X-Google-Smtp-Source: AMrXdXsr1GAxYylWvcxFwzTr5ek5ekACmLYrRCT2WaTPNb+erRNGiRKS70/jViZo8AMOJNoj3nGy7g== X-Received: by 2002:a5d:4591:0:b0:2b4:790e:32f8 with SMTP id p17-20020a5d4591000000b002b4790e32f8mr9650548wrq.48.1673380609218; Tue, 10 Jan 2023 11:56:49 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id u5-20020adfdb85000000b002ba2646fd30sm14185332wri.36.2023.01.10.11.56.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 10 Jan 2023 11:56:48 -0800 (PST) Subject: Re: [PATCH] gdb: use gdb_test_multiple in gdb_breakpoint To: Simon Marchi , Tom de Vries , gdb-patches@sourceware.org References: <20230103192216.108444-1-simon.marchi@polymtl.ca> <315f1d7a-a948-5512-fd89-a40d7a25e937@polymtl.ca> From: Pedro Alves Message-ID: Date: Tue, 10 Jan 2023 19:56:47 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,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 List-Id: On 2023-01-10 3:50 p.m., Simon Marchi wrote: > > > On 1/10/23 10:33, Pedro Alves wrote: >> On 2023-01-05 4:28 p.m., Simon Marchi via Gdb-patches wrote: >> >>> --- >>> gdb/testsuite/lib/gdb.exp | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp >>> index e17eace4cb13..af538e5c8fbd 100644 >>> --- a/gdb/testsuite/lib/gdb.exp >>> +++ b/gdb/testsuite/lib/gdb.exp >>> @@ -657,6 +657,12 @@ proc gdb_breakpoint { linespec args } { >>> send_gdb "$pending_response\n" >>> exp_continue >>> } >>> + -re "$gdb_prompt $" { >>> + if { $print_fail } { >>> + fail $test_name >>> + } >>> + return 0 >>> + } >>> } >> >> The other removed "-re" cases also considered $print_fail, so if their replacement >> inside gdb_test_multiple is hit, they'll produce a FAIL. Was that intended? >> Should we instead add a "-nofail" option to gdb_test / gdb_test_multiple ? > > Good point, this is a change in behavior. Does this change cause you an > unexpected FAIL in practice? No, I was just trying to catch up on patches a bit, and read your patch and noticed that issue. I was going to reply to the patch directly, but then saw the follow up discussion and replied there (or rather, here). > I was actually planning on removing that > message / no-message option to gdb_breakpoint [1] to simplify it. I > don't see any use for the current behavior, I'd rather have it log a > test result all the time. I've always found the message/no-message API confusing. It exists on runto too, and maybe other procs. I think the intention of not issuing PASS by default, was that you can use gdb_breakpoint to implement other procedures inside lib/gdb.exp. If gdb_breakpoint starts issuing a PASS, then an implementation detail of such procedures starts being visible, by ending up with two PASSes for each call of the procedure that happens to use gdb_breakpoint, one for gdb_breakpoint, and one for the caller procedure proper. I think it's the same reason many procedures in lib/gdb.exp were kept using gdb_expect directly instead of being converted to gdb_test/gdb_test_multiple -- to avoid the internal PASS/FAIL. Pedro Alves > I can kind of see when that would maybe be > useful: if you wanted to set a breakpoint, and it could legitimately not > work (you could do something with gdb_breakpoint's return value). But > you could also use the break command directly, like many places do > already, so it's not really needed. Anyway, all of this to say that I > could fix what you pointed out by pruning / simplifying code rather than > adding more. > > Simon > > [1] https://review.lttng.org/c/binutils-gdb/+/7158/6 >