From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id mkLNOUEmgGBJOAAAWB0awg (envelope-from ) for ; Wed, 21 Apr 2021 09:18:57 -0400 Received: by simark.ca (Postfix, from userid 112) id DE3D51F104; Wed, 21 Apr 2021 09:18:57 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.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 EF5D51E783 for ; Wed, 21 Apr 2021 09:18:56 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 98F4F396C83A; Wed, 21 Apr 2021 13:18:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 98F4F396C83A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619011136; bh=hAZnpWF1JPz0G/c5XeHGHzXIFm8Nx36G+rctkzYzqlM=; 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=APpoNxBTBpZtxsA9NVrWQtSEZUi+BuyAv5/4R7mCiCqNada9o2QQ298F2m2Dfek7a beg3/I5oqC23zKKFE82uu3BJ0xTQssSF+jVcSV8mAqXqc3q6F/OBgkIrIPjFmNqMD9 kUgZmp3beEMZHJRsW3he4I8pXpplf1kmYnlIaOqU= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 4626439B587E for ; Wed, 21 Apr 2021 13:18:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4626439B587E 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 13LDIYlm001440 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Apr 2021 09:18:38 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 13LDIYlm001440 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 B79031E783; Wed, 21 Apr 2021 09:18:33 -0400 (EDT) Subject: Re: [PATCH v3 3/4] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint' To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: <42d67b6b917e9ef0a25dfb41d7f8ba15710d42c2.1619006869.git.tankut.baris.aktemur@intel.com> Message-ID: <2c1ea6ae-85f6-1516-8553-66f6341d3813@polymtl.ca> Date: Wed, 21 Apr 2021 09:18:33 -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: <42d67b6b917e9ef0a25dfb41d7f8ba15710d42c2.1619006869.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, 21 Apr 2021 13:18:34 +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-21 8:17 a.m., Tankut Baris Aktemur 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=mi3 /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",locations=[{number="1.1",enabled="N",addr="0x000000000000114e",func="main",file="/tmp/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 /tmp/simple.c, line 2.\n" > =breakpoint-created,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="",cond="junk",times="0",original-location="main",locations=[{number="2.1",enabled="N",addr="0x000000000000114e",func="main",file="/tmp/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." > > This restores the behavior that is present in the existing releases. > > gdb/ChangeLog: > 2021-04-06 Tankut Baris Aktemur > > * breakpoint.h (create_breakpoint): Add a new parameter, > 'force_condition'. > * breakpoint.c (create_breakpoint): Use the 'force_condition' > argument when 'parse_extra' is false to check if the condition > is invalid at all of the breakpoint locations. > Update the users below. > (break_command_1) > (dprintf_command) > (trace_command) > (ftrace_command) > (strace_command) > (create_tracepoint_from_upload): Update. > * guile/scm-breakpoint.c (gdbscm_register_breakpoint_x): Update. > * mi/mi-cmd-break.c (mi_cmd_break_insert_1): Update. > * python/py-breakpoint.c (bppy_init): Update. > * python/py-finishbreakpoint.c (bpfinishpy_init): Update. > > gdb/testsuite/ChangeLog: > 2021-04-06 Tankut Baris Aktemur > > * gdb.mi/mi-break.exp: Extend with checks for invalid breakpoint > conditions. Up to this patch, it all LGTM and I think it could be merged right away. Thanks, Simon