public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint'
@ 2021-04-21 14:48 Tankut Baris Aktemur
0 siblings, 0 replies; only message in thread
From: Tankut Baris Aktemur @ 2021-04-21 14:48 UTC (permalink / raw)
To: gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=10a636ccb4635ccb85780f432b089521a1d85e18
commit 10a636ccb4635ccb85780f432b089521a1d85e18
Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date: Wed Apr 21 16:42:40 2021 +0200
gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint'
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="<MULTIPLE>",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="<MULTIPLE>",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-21 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* 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-21 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.mi/mi-break.exp: Extend with checks for invalid breakpoint
conditions.
Diff:
---
gdb/ChangeLog | 19 +++++++++++++++++++
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/ChangeLog | 5 +++++
gdb/testsuite/gdb.mi/mi-break.exp | 13 +++++++++++++
9 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a900a1d36b5..981fcb55b4c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2021-04-21 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
+
+ * 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.
+
2021-04-21 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* breakpoint.c (print_one_breakpoint_location): Display "N" for
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/ChangeLog b/gdb/testsuite/ChangeLog
index 353dde9168a..44fa9f35fef 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-21 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
+
+ * gdb.mi/mi-break.exp: Extend with checks for invalid breakpoint
+ conditions.
+
2021-04-21 Simon Marchi <simon.marchi@polymtl.ca>
Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 19438f21f63..b2db2d41d1f 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 \"Hello\"" \
+ ".*\\^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 {} {
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-04-21 14:48 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 14:48 [binutils-gdb] gdb/breakpoint: add a 'force_condition' parameter to 'create_breakpoint' Tankut Baris Aktemur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).