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 --git a/gdb/ChangeLog b/gdb/ChangeLog
index a900a1d36b51e152e50d748ceafeca785027865b..981fcb55b4c226c37e51140afcf6eeb3e81a6724 100644 (file)
--- 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 0136019b4ae5d3de3d3de0eb3e0a29dba3df78e5..c2d0ffba974d4783c11f03a5ca3a8eb1626c0f18 100644 (file)
--- 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 c30656971bb71d09e30ba5e6034447c63cab1243..ded498f55622626e4bbf75d1c507cd8cd31826b4 100644 (file)
--- 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 99098e7e155165fd8a3a3ab2cf62cb71b13d3357..af63893461ba2fd10b4f7c204ad9596b783c756c 100644 (file)
--- 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 1c3e9209a74d7d17f008041b874fa32498ed4a2d..5a4a62ce8c3497e9acc64eab4bbcc4502d3e3c38 100644 (file)
--- 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 3fbb1c633ff49ff91d013cb22980df9d78c332bc..9650bd023b56e0c9cf0a244429e4128c8bb544c0 100644 (file)
--- 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 d01b84273fc44b9bee5c6bff18389defc7f0dd15..38b4cc67901f8aab77b775dab42b2265809dd25a 100644 (file)
--- 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 353dde9168a0b05a352335a2b5dbc65ec3fa9fef..44fa9f35fefe802a59019b4bd4fe9d857185299d 100644 (file)
--- 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 19438f21f6345f8347cb0be9f245b81e57f703fd..b2db2d41d1fcf4e1df451134067704e787c9e2ad 100644 (file)
--- 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 {} {