From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id rhxJHWstbmCYegAAWB0awg (envelope-from ) for ; Wed, 07 Apr 2021 18:08:43 -0400 Received: by simark.ca (Postfix, from userid 112) id 6B6E31E965; Wed, 7 Apr 2021 18:08:43 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id AC1F51E01F for ; Wed, 7 Apr 2021 18:08:42 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0D1B13939C1D; Wed, 7 Apr 2021 22:08:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0D1B13939C1D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1617833322; bh=ZsthwUAikUk1Wiyr0CsSfTlno+S6ej0v78v3aGnK5VQ=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=CINI0ahzFeR4a0gucE8yT5hJKG+6mCAwUE84zZY0n1PXsAGR1Mxmh40Dgl0w9pO3u m+w6YAyrx6u0AAztTM9zIkQNak8p+EcES8ej4TH/nO8tu7/17V6cVlSkSuq/zUovXt mulQUABai2UEJuEEG8ykHIwLAOo56lGMwZyEoM2c= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 3B253385801D for ; Wed, 7 Apr 2021 22:08:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3B253385801D Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 137M8VC2031896 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 7 Apr 2021 18:08:36 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 137M8VC2031896 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id E60611E01F; Wed, 7 Apr 2021 18:08:30 -0400 (EDT) Subject: Re: [PATCH 3/4] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint' To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: <40773bd923ed8cb1f9773ecd29a5af01e204d801.1617806598.git.tankut.baris.aktemur@intel.com> Message-ID: <8f9e3ef4-22c0-a872-68e4-20389e4fb746@polymtl.ca> Date: Wed, 7 Apr 2021 18:08:30 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <40773bd923ed8cb1f9773ecd29a5af01e204d801.1617806598.git.tankut.baris.aktemur@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 7 Apr 2021 22:08:31 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2021-04-07 10:55 a.m., Tankut Baris Aktemur via Gdb-patches wrote: > The 'create_breakpoint' function takes a 'parse_extra' argument that > determines whether the condition, thread, and force-condition > specifiers should be parsed from the extra string or be used from the > function arguments. However, for the case when 'parse_extra' is > false, there is no way to pass the force-condition specifier. This > patch adds it as a new argument. > > Also, in the case when parse_extra is false, the current behavior is > as if the condition is being forced. This is a bug. The default > behavior should reject the breakpoint. See below for a demo of this > incorrect behavior. (The MI command '-break-insert' uses the > 'create_breakpoint' function with parse_extra=0.) > > $ gdb -q --interpreter=mi2 /tmp/simple > =thread-group-added,id="i1" > =cmd-param-changed,param="history save",value="on" > =cmd-param-changed,param="auto-load safe-path",value="/" > ~"Reading symbols from /tmp/simple...\n" > (gdb) > -break-insert -c junk -f main > &"warning: failed to validate condition at location 1, disabling:\n " > &"No symbol \"junk\" in current context.\n" > ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="",cond="junk",times="0",original-location="main"},{number="1.1",enabled="N*",addr="0x000000000000114e",func="main",file="simple.c",fullname="/tmp/simple.c",line="2",thread-groups=["i1"]} > (gdb) > break main if junk > &"break main if junk\n" > &"No symbol \"junk\" in current context.\n" > ^error,msg="No symbol \"junk\" in current context." > (gdb) > break main -force-condition if junk > &"break main -force-condition if junk\n" > ~"Note: breakpoint 1 also set at pc 0x114e.\n" > &"warning: failed to validate condition at location 1, disabling:\n " > &"No symbol \"junk\" in current context.\n" > ~"Breakpoint 2 at 0x114e: file simple.c, line 2.\n" > =breakpoint-created,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="",cond="junk",times="0",original-location="main"},{number="2.1",enabled="N*",addr="0x000000000000114e",func="main",file="simple.c",fullname="/tmp/simple.c",line="2",thread-groups=["i1"]} > ^done > (gdb) > > After applying this patch, we get the behavior below: > > (gdb) > -break-insert -c junk -f main > ^error,msg="No symbol \"junk\" in current context." You can also mention that this restores the previous behavior (present in existing releases). > diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp > index 9d3d7ade6dc..ac13e4d1e09 100644 > --- a/gdb/testsuite/gdb.mi/mi-break.exp > +++ b/gdb/testsuite/gdb.mi/mi-break.exp > @@ -224,6 +224,19 @@ proc test_error {} { > mi_gdb_test "-break-insert -c i==4 \"callme if i < 4\"" \ > ".*\\^error,msg=\"Garbage 'if i < 4' at end of location\"" \ > "conditional breakpoint with garbage after location" > + > + # Try using an invalid condition. > + mi_gdb_test "-break-insert -c bad callme" \ > + ".*\\^error,msg=\"No symbol \\\\\"bad\\\\\" in current context.\"" \ > + "breakpoint with bad condition" > + > + mi_gdb_test "-dprintf-insert -c bad callme 123" \ > + ".*\\^error,msg=\"No symbol \\\\\"bad\\\\\" in current context.\"" \ > + "dprintf with bad condition" > + > + mi_gdb_test "-break-condition 5 bad" \ > + ".*\\^error,msg=\"No symbol \\\\\"bad\\\\\" in current context.\"" \ > + "invalid condition" Here, does the 5 refer to an existing breakpoint number created by another test function? It's a bit annoying when test functions rely on the state left by other test functions, that makes them harder to debug in isolation. In my ideal world, each test_* function in this file (except test_break, which is just the driver) would spawn a fresh GDB, such that if you need to debug one of them, you can comment out all the other ones. The patch is still OK though, and thanks for taking the time to write a test. Simon