From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id OBpbBxEYgGDnNQAAWB0awg (envelope-from ) for ; Wed, 21 Apr 2021 08:18:25 -0400 Received: by simark.ca (Postfix, from userid 112) id 16A9A1F104; Wed, 21 Apr 2021 08:18:25 -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 D380C1E789 for ; Wed, 21 Apr 2021 08:18:21 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 89A9239B7023; Wed, 21 Apr 2021 12:18:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 89A9239B7023 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619007501; bh=vhZ4rvHA4YbwqZ5Cm7M/1DL/4/N5uxMyPBB8co4adEc=; h=To:Subject:Date:In-Reply-To:References:In-Reply-To:References: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help: List-Subscribe:From:Reply-To:From; b=EuwxLDvBU7meH8yzoitcWI1VxL0T4tGc7bIt2DJKmpR4IT/to94iWZUQ/IGLqusYt 38C7nZUTlL6uOaD3/u9Zb7dfXsL1T5EFEPIrBeicCFe9SvJEdLvLGfjTLXhbV2kTkr tDxUmWVwNMEk5zEsVBqIQOboeu/zKnu5wOogg8+8= Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by sourceware.org (Postfix) with ESMTPS id 44CB03982057 for ; Wed, 21 Apr 2021 12:18:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 44CB03982057 IronPort-SDR: 2f/ch9Jr9XvmsG/rnsB3bBfiDu1vv6AhowNz04iBlytBTRJmH4iTFbSaZE23wZ/wCyBBvdn4sv zk2TodTIf/hg== X-IronPort-AV: E=McAfee;i="6200,9189,9960"; a="195242120" X-IronPort-AV: E=Sophos;i="5.82,238,1613462400"; d="scan'208";a="195242120" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Apr 2021 05:18:04 -0700 IronPort-SDR: ndOR7cfeivRo98kLl8O8LfA6qbexRTS2gSEo25ITTlnJPJue0GujNLOwKHmo51R78HsrrcZUCU FN6meH/znmdg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.82,238,1613462400"; d="scan'208";a="614074359" Received: from irvmail001.ir.intel.com ([10.43.11.63]) by fmsmga006.fm.intel.com with ESMTP; 21 Apr 2021 05:18:02 -0700 Received: from ulvlx001.iul.intel.com (ulvlx001.iul.intel.com [172.28.207.17]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id 13LCI2BF031915; Wed, 21 Apr 2021 13:18:02 +0100 Received: from ulvlx001.iul.intel.com (localhost [127.0.0.1]) by ulvlx001.iul.intel.com with ESMTP id 13LCI29E026459; Wed, 21 Apr 2021 14:18:02 +0200 Received: (from taktemur@localhost) by ulvlx001.iul.intel.com with LOCAL id 13LCI1h8026453; Wed, 21 Apr 2021 14:18:02 +0200 To: gdb-patches@sourceware.org Subject: [PATCH v3 3/4] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint' Date: Wed, 21 Apr 2021 14:17:41 +0200 Message-Id: <42d67b6b917e9ef0a25dfb41d7f8ba15710d42c2.1619006869.git.tankut.baris.aktemur@intel.com> X-Mailer: git-send-email 1.7.0.7 In-Reply-To: References: In-Reply-To: References: 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: Tankut Baris Aktemur via Gdb-patches Reply-To: Tankut Baris Aktemur Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" 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. --- gdb/breakpoint.c | 40 ++++++++++++++++++++++++++----- gdb/breakpoint.h | 6 +++++ gdb/guile/scm-breakpoint.c | 2 +- gdb/mi/mi-cmd-break.c | 1 + gdb/python/py-breakpoint.c | 2 +- gdb/python/py-finishbreakpoint.c | 2 +- gdb/testsuite/gdb.mi/mi-break.exp | 13 ++++++++++ 7 files changed, 57 insertions(+), 9 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0136019b4ae..c2d0ffba974 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -9460,7 +9460,7 @@ create_breakpoint (struct gdbarch *gdbarch, struct event_location *location, const char *cond_string, int thread, const char *extra_string, - int parse_extra, + bool force_condition, int parse_extra, int tempflag, enum bptype type_wanted, int ignore_count, enum auto_boolean pending_break_support, @@ -9558,6 +9558,33 @@ create_breakpoint (struct gdbarch *gdbarch, && extra_string != NULL && *extra_string != '\0') error (_("Garbage '%s' at end of location"), extra_string); + /* Check the validity of the condition. We should error out + if the condition is invalid at all of the locations and + if it is not forced. In the PARSE_EXTRA case above, this + check is done when parsing the EXTRA_STRING. */ + if (cond_string != nullptr && !force_condition) + { + int num_failures = 0; + const linespec_sals &lsal = canonical.lsals[0]; + for (const auto &sal : lsal.sals) + { + const char *cond = cond_string; + try + { + parse_exp_1 (&cond, sal.pc, block_for_pc (sal.pc), 0); + /* One success is sufficient to keep going. */ + break; + } + catch (const gdb_exception_error &) + { + num_failures++; + /* If this is the last sal, error out. */ + if (num_failures == lsal.sals.size ()) + throw; + } + } + } + /* Create a private copy of condition string. */ if (cond_string) cond_string_copy.reset (xstrdup (cond_string)); @@ -9636,7 +9663,7 @@ break_command_1 (const char *arg, int flag, int from_tty) create_breakpoint (get_current_arch (), location.get (), - NULL, 0, arg, 1 /* parse arg */, + NULL, 0, arg, false, 1 /* parse arg */, tempflag, type_wanted, 0 /* Ignore count */, pending_break_support, @@ -9823,7 +9850,7 @@ dprintf_command (const char *arg, int from_tty) create_breakpoint (get_current_arch (), location.get (), - NULL, 0, arg, 1 /* parse arg */, + NULL, 0, arg, false, 1 /* parse arg */, 0, bp_dprintf, 0 /* Ignore count */, pending_break_support, @@ -14721,7 +14748,7 @@ trace_command (const char *arg, int from_tty) create_breakpoint (get_current_arch (), location.get (), - NULL, 0, arg, 1 /* parse arg */, + NULL, 0, arg, false, 1 /* parse arg */, 0 /* tempflag */, bp_tracepoint /* type_wanted */, 0 /* Ignore count */, @@ -14739,7 +14766,7 @@ ftrace_command (const char *arg, int from_tty) current_language); create_breakpoint (get_current_arch (), location.get (), - NULL, 0, arg, 1 /* parse arg */, + NULL, 0, arg, false, 1 /* parse arg */, 0 /* tempflag */, bp_fast_tracepoint /* type_wanted */, 0 /* Ignore count */, @@ -14773,7 +14800,7 @@ strace_command (const char *arg, int from_tty) create_breakpoint (get_current_arch (), location.get (), - NULL, 0, arg, 1 /* parse arg */, + NULL, 0, arg, false, 1 /* parse arg */, 0 /* tempflag */, bp_static_tracepoint /* type_wanted */, 0 /* Ignore count */, @@ -14843,6 +14870,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp) if (!create_breakpoint (get_current_arch (), location.get (), utp->cond_string.get (), -1, addr_str, + false /* force_condition */, 0 /* parse cond/thread */, 0 /* tempflag */, utp->type /* type_wanted */, diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index c30656971bb..ded498f5562 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1409,6 +1409,11 @@ enum breakpoint_create_flags the condition, thread, and extra string from EXTRA_STRING, ignoring the similarly named parameters. + If FORCE_CONDITION is true, the condition is accepted even when it is + invalid at all of the locations. However, if PARSE_EXTRA is non-zero, + the FORCE_CONDITION parameter is ignored and the corresponding argument + is parsed from EXTRA_STRING. + If INTERNAL is non-zero, the breakpoint number will be allocated from the internal breakpoint count. @@ -1418,6 +1423,7 @@ extern int create_breakpoint (struct gdbarch *gdbarch, struct event_location *location, const char *cond_string, int thread, const char *extra_string, + bool force_condition, int parse_extra, int tempflag, enum bptype wanted_type, int ignore_count, diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c index 99098e7e155..af63893461b 100644 --- a/gdb/guile/scm-breakpoint.c +++ b/gdb/guile/scm-breakpoint.c @@ -440,7 +440,7 @@ gdbscm_register_breakpoint_x (SCM self) const breakpoint_ops *ops = breakpoint_ops_for_event_location (eloc.get (), false); create_breakpoint (get_current_arch (), - eloc.get (), NULL, -1, NULL, + eloc.get (), NULL, -1, NULL, false, 0, 0, bp_breakpoint, 0, diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c index 1c3e9209a74..5a4a62ce8c3 100644 --- a/gdb/mi/mi-cmd-break.c +++ b/gdb/mi/mi-cmd-break.c @@ -353,6 +353,7 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc) create_breakpoint (get_current_arch (), location.get (), condition, thread, extra_string.c_str (), + false, 0 /* condition and thread are valid. */, temp_p, type_wanted, ignore_count, diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index 3fbb1c633ff..9650bd023b5 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -835,7 +835,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs) breakpoint_ops_for_event_location (location.get (), false); create_breakpoint (python_gdbarch, - location.get (), NULL, -1, NULL, + location.get (), NULL, -1, NULL, false, 0, temporary_bp, type, 0, diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c index d01b84273fc..38b4cc67901 100644 --- a/gdb/python/py-finishbreakpoint.c +++ b/gdb/python/py-finishbreakpoint.c @@ -294,7 +294,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) event_location_up location = new_address_location (get_frame_pc (prev_frame), NULL, 0); create_breakpoint (python_gdbarch, - location.get (), NULL, thread, NULL, + location.get (), NULL, thread, NULL, false, 0, 1 /*temp_flag*/, bp_breakpoint, diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index 19438f21f63..e46a6cc0ec6 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -224,6 +224,19 @@ proc_with_prefix 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" } proc_with_prefix test_disabled_creation {} { -- 2.17.1