gdb/breakpoint: disable a bp location if condition is invalid at that location

Currently, for a conditional breakpoint, GDB checks if the condition
can be evaluated in the context of the first symtab and line (SAL).
In case of an error, defining the conditional breakpoint is aborted.
This prevents having a conditional breakpoint whose condition may
actually be meaningful for some of the location contexts.  This patch
makes it possible to define conditional BPs by checking all location
contexts.  If the condition is meaningful for even one context, the
breakpoint is defined.  The locations for which the condition gives
errors are disabled.

The bp_location struct is introduced a new field, 'disabled_by_cond'.
This field denotes whether the location is disabled automatically
because the condition was non-evaluatable.  Disabled-by-cond locations
cannot be enabled by the user.  But locations that are not
disabled-by-cond can be enabled/disabled by the user manually as
before.

For a concrete example, consider 3 contexts of a function 'func'.

  class Base
  {
  public:
    int b = 20;

    void func () {}
  };

  class A : public Base
  {
  public:
    int a = 10;

    void func () {}
  };

  class C : public Base
  {
  public:
    int c = 30;

    void func () {}
  };

Note that

* the variable 'a' is defined only in the context of A::func.
* the variable 'c' is defined only in the context of C::func.
* the variable 'b' is defined in all the three contexts.

With the existing GDB, it's not possible to define a conditional
breakpoint at 'func' if the condition refers to 'a' or 'c':

  (gdb) break func if a == 10
  No symbol "a" in current context.
  (gdb) break func if c == 30
  No symbol "c" in current context.
  (gdb) info breakpoints
  No breakpoints or watchpoints.

With this patch, it becomes possible:

  (gdb) break func if a == 10
  warning: failed to validate condition at location 1, disabling:
    No symbol "a" in current context.
  warning: failed to validate condition at location 3, disabling:
    No symbol "a" in current context.
  Breakpoint 1 at 0x11b6: func. (3 locations)
  (gdb) break func if c == 30
  Note: breakpoint 1 also set at pc 0x11ce.
  Note: breakpoint 1 also set at pc 0x11c2.
  Note: breakpoint 1 also set at pc 0x11b6.
  warning: failed to validate condition at location 1, disabling:
    No symbol "c" in current context.
  warning: failed to validate condition at location 2, disabling:
    No symbol "c" in current context.
  Breakpoint 2 at 0x11b6: func. (3 locations)
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
          stop only if a == 10
  1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  2       breakpoint     keep y   <MULTIPLE>
          stop only if c == 30
  2.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  2.2                         N*  0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  2.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  (*): Breakpoint condition is invalid at this location.

Here, uppercase 'N' denotes that the location is disabled because of
the invalid condition, as mentioned with a footnote in the legend of
the table.  Locations that are disabled by the user are still denoted
with lowercase 'n'.  Executing the code hits the breakpoints 1.2 and
2.3 as expected.

Defining a condition on an unconditional breakpoint gives the same
behavior above:

  (gdb) break func
  Breakpoint 1 at 0x11b6: func. (3 locations)
  (gdb) cond 1 a == 10
  warning: failed to validate condition at location 1.1, disabling:
    No symbol "a" in current context.
  warning: failed to validate condition at location 1.3, disabling:
    No symbol "a" in current context.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
          stop only if a == 10
  1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  (*): Breakpoint condition is invalid at this location.

Locations that are disabled because of a condition cannot be enabled
by the user:

  ...
  (gdb) enable 1.1
  Breakpoint 1's condition is invalid at location 1, cannot enable.

Resetting the condition enables the locations back:

  ...
  (gdb) cond 1
  Breakpoint 1's condition is now valid at location 1, enabling.
  Breakpoint 1's condition is now valid at location 3, enabling.
  Breakpoint 1 now unconditional.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
  1.1                         y   0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         y   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39

If a location is disabled by the user, a condition can still be defined
but the location will remain disabled even if the condition is meaningful
for the disabled location:

  ...
  (gdb) disable 1.2
  (gdb) cond 1 a == 10
  warning: failed to validate condition at location 1.1, disabling:
    No symbol "a" in current context.
  warning: failed to validate condition at location 1.3, disabling:
    No symbol "a" in current context.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
          stop only if a == 10
  1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         n   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         N*  0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  (*): Breakpoint condition is invalid at this location.

The condition of a breakpoint can be changed.  Locations'
enable/disable states are updated accordingly.

  ...
  (gdb) cond 1 c == 30
  warning: failed to validate condition at location 1.1, disabling:
    No symbol "c" in current context.
  Breakpoint 1's condition is now valid at location 3, enabling.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
          stop only if c == 30
  1.1                         N*  0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         N*  0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  (*): Breakpoint condition is invalid at this location.

  (gdb) cond 1 b == 20
  Breakpoint 1's condition is now valid at location 1, enabling.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
          stop only if b == 20
  1.1                         y   0x00000000000011b6 in Base::func() at condbreak-multi-context.cc:23
  1.2                         n   0x00000000000011c2 in A::func() at condbreak-multi-context.cc:31
  1.3                         y   0x00000000000011ce in C::func() at condbreak-multi-context.cc:39
  # Note that location 1.2 was disabled by the user previously.

If the condition expression is bad for all the locations, it will be
rejected.

  (gdb) cond 1 garbage
  No symbol "garbage" in current context.

For conditions that are invalid or valid for all the locations of a
breakpoint, the existing behavior is preserved.

Regression-tested on X86_64 Linux.

gdb/ChangeLog:
2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* breakpoint.h (class bp_location) <disabled_by_cond>: New field.
	* breakpoint.c (set_breakpoint_location_condition): New function.
	(set_breakpoint_condition): Disable a breakpoint location if parsing
	the condition string gives an error.
	(should_be_inserted): Update to consider the 'disabled_by_cond' field.
	(build_target_condition_list): Ditto.
	(build_target_command_list): Ditto.
	(build_bpstat_chain): Ditto.
	(print_one_breakpoint_location): Ditto.
	(print_one_breakpoint): Ditto.
	(breakpoint_1): Ditto.
	(bp_location::bp_location): Ditto.
	(locations_are_equal): Ditto.
	(update_breakpoint_locations): Ditto.
	(enable_disable_bp_num_loc): Ditto.
	(init_breakpoint_sal): Use set_breakpoint_location_condition.
	(find_condition_and_thread_for_sals): New static function.
	(create_breakpoint): Call find_condition_and_thread_for_sals.
	(location_to_sals): Call find_condition_and_thread_for_sals instead
	of find_condition_and_thread.

gdb/testsuite/ChangeLog:
2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.base/condbreak-multi-context.cc: New file.
	* gdb.base/condbreak-multi-context.exp: New file.

gdb/doc/ChangeLog:
2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.texinfo (Set Breaks): Document disabling of breakpoint
	locations for which the breakpoint condition is invalid.

8 files changed, 548 insertions, 50 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4a83aac..6b0a7c1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,26 @@
+2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* breakpoint.h (class bp_location) <disabled_by_cond>: New field.
+	* breakpoint.c (set_breakpoint_location_condition): New function.
+	(set_breakpoint_condition): Disable a breakpoint location if parsing
+	the condition string gives an error.
+	(should_be_inserted): Update to consider the 'disabled_by_cond' field.
+	(build_target_condition_list): Ditto.
+	(build_target_command_list): Ditto.
+	(build_bpstat_chain): Ditto.
+	(print_one_breakpoint_location): Ditto.
+	(print_one_breakpoint): Ditto.
+	(breakpoint_1): Ditto.
+	(bp_location::bp_location): Ditto.
+	(locations_are_equal): Ditto.
+	(update_breakpoint_locations): Ditto.
+	(enable_disable_bp_num_loc): Ditto.
+	(init_breakpoint_sal): Use set_breakpoint_location_condition.
+	(find_condition_and_thread_for_sals): New static function.
+	(create_breakpoint): Call find_condition_and_thread_for_sals.
+	(location_to_sals): Call find_condition_and_thread_for_sals instead
+	of find_condition_and_thread.
+
 2020-10-26  Tom de Vries  <tdevries@suse.de>
 
 	* dwarf2/read.c (process_full_comp_unit): Call
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 631bee5..0d3fd0c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -830,6 +830,56 @@ get_first_locp_gte_addr (CORE_ADDR address)
   return locp_found;
 }
 
+/* Parse COND_STRING in the context of LOC and set as the condition
+   expression of LOC.  BP_NUM is the number of LOC's owner, LOC_NUM is
+   the number of LOC within its owner.  In case of parsing error, mark
+   LOC as DISABLED_BY_COND.  In case of success, unset DISABLED_BY_COND.  */
+
+static void
+set_breakpoint_location_condition (const char *cond_string, bp_location *loc,
+				   int bp_num, int loc_num)
+{
+  bool has_junk = false;
+  try
+    {
+      expression_up new_exp = parse_exp_1 (&cond_string, loc->address,
+					   block_for_pc (loc->address), 0);
+      if (*cond_string != 0)
+	has_junk = true;
+      else
+	{
+	  loc->cond = std::move (new_exp);
+	  if (loc->disabled_by_cond && loc->enabled)
+	    printf_filtered (_("Breakpoint %d's condition is now valid at "
+			       "location %d, enabling.\n"),
+			     bp_num, loc_num);
+
+	  loc->disabled_by_cond = false;
+	}
+    }
+  catch (const gdb_exception_error &e)
+    {
+      if (loc->enabled)
+	{
+	  /* Warn if a user-enabled location is now becoming disabled-by-cond.
+	     BP_NUM is 0 if the breakpoint is being defined for the first
+	     time using the "break ... if ..." command, and non-zero if
+	     already defined.  */
+	  if (bp_num != 0)
+	    warning (_("failed to validate condition at location %d.%d, "
+		       "disabling:\n  %s"), bp_num, loc_num, e.what ());
+	  else
+	    warning (_("failed to validate condition at location %d, "
+		       "disabling:\n  %s"), loc_num, e.what ());
+	}
+
+      loc->disabled_by_cond = true;
+    }
+
+  if (has_junk)
+    error (_("Garbage '%s' follows condition"), cond_string);
+}
+
 void
 set_breakpoint_condition (struct breakpoint *b, const char *exp,
 			  int from_tty)
@@ -843,9 +893,16 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
 	static_cast<watchpoint *> (b)->cond_exp.reset ();
       else
 	{
+	  int loc_num = 1;
 	  for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
 	    {
 	      loc->cond.reset ();
+	      if (loc->disabled_by_cond && loc->enabled)
+		printf_filtered (_("Breakpoint %d's condition is now valid at "
+				   "location %d, enabling.\n"),
+				 b->number, loc_num);
+	      loc->disabled_by_cond = false;
+	      loc_num++;
 
 	      /* No need to free the condition agent expression
 		 bytecode (if we have one).  We will handle this
@@ -873,29 +930,37 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
 	{
 	  /* Parse and set condition expressions.  We make two passes.
 	     In the first, we parse the condition string to see if it
-	     is valid in all locations.  If so, the condition would be
-	     accepted.  So we go ahead and set the locations'
-	     conditions.  In case a failing case is found, we throw
+	     is valid in at least one location.  If so, the condition
+	     would be accepted.  So we go ahead and set the locations'
+	     conditions.  In case no valid case is found, we throw
 	     the error and the condition string will be rejected.
 	     This two-pass approach is taken to avoid setting the
 	     state of locations in case of a reject.  */
 	  for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
 	    {
-	      const char *arg = exp;
-	      parse_exp_1 (&arg, loc->address,
-			   block_for_pc (loc->address), 0);
-	      if (*arg != 0)
-		error (_("Junk at end of expression"));
+	      try
+		{
+		  const char *arg = exp;
+		  parse_exp_1 (&arg, loc->address,
+			       block_for_pc (loc->address), 0);
+		  if (*arg != 0)
+		    error (_("Junk at end of expression"));
+		  break;
+		}
+	      catch (const gdb_exception_error &e)
+		{
+		  /* Condition string is invalid.  If this happens to
+		     be the last loc, abandon.  */
+		  if (loc->next == nullptr)
+		    throw;
+		}
 	    }
 
-	  /* If we reach here, the condition is valid at all locations.  */
-	  for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
-	    {
-	      const char *arg = exp;
-	      loc->cond =
-		parse_exp_1 (&arg, loc->address,
-			     block_for_pc (loc->address), 0);
-	    }
+	  /* If we reach here, the condition is valid at some locations.  */
+          int loc_num = 1;
+          for (bp_location *loc = b->loc; loc != nullptr;
+               loc = loc->next, loc_num++)
+            set_breakpoint_location_condition (exp, loc, b->number, loc_num);
 	}
 
       /* We know that the new condition parsed successfully.  The
@@ -2010,7 +2075,8 @@ should_be_inserted (struct bp_location *bl)
   if (bl->owner->disposition == disp_del_at_next_stop)
     return 0;
 
-  if (!bl->enabled || bl->shlib_disabled || bl->duplicate)
+  if (!bl->enabled || bl->disabled_by_cond
+      || bl->shlib_disabled || bl->duplicate)
     return 0;
 
   if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup)
@@ -2205,7 +2271,8 @@ build_target_condition_list (struct bp_location *bl)
 	  && is_breakpoint (loc->owner)
 	  && loc->pspace->num == bl->pspace->num
 	  && loc->owner->enable_state == bp_enabled
-	  && loc->enabled)
+	  && loc->enabled
+	  && !loc->disabled_by_cond)
 	{
 	  /* Add the condition to the vector.  This will be used later
 	     to send the conditions to the target.  */
@@ -2395,7 +2462,8 @@ build_target_command_list (struct bp_location *bl)
 	  && is_breakpoint (loc->owner)
 	  && loc->pspace->num == bl->pspace->num
 	  && loc->owner->enable_state == bp_enabled
-	  && loc->enabled)
+	  && loc->enabled
+	  && !loc->disabled_by_cond)
 	{
 	  /* Add the command to the vector.  This will be used later
 	     to send the commands to the target.  */
@@ -5278,7 +5346,7 @@ build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr,
 	  if (b->type == bp_hardware_watchpoint && bl != b->loc)
 	    break;
 
-	  if (!bl->enabled || bl->shlib_disabled)
+	  if (!bl->enabled || bl->disabled_by_cond || bl->shlib_disabled)
 	    continue;
 
 	  if (!bpstat_check_location (bl, aspace, bp_addr, ws))
@@ -5987,7 +6055,8 @@ print_one_breakpoint_location (struct breakpoint *b,
      breakpoints with single disabled location.  */
   if (loc == NULL 
       && (b->loc != NULL 
-	  && (b->loc->next != NULL || !b->loc->enabled)))
+	  && (b->loc->next != NULL
+	      || !b->loc->enabled || b->loc->disabled_by_cond)))
     header_of_multiple = 1;
   if (loc == NULL)
     loc = b->loc;
@@ -6018,7 +6087,8 @@ print_one_breakpoint_location (struct breakpoint *b,
   /* 4 */
   annotate_field (3);
   if (part_of_multiple)
-    uiout->field_string ("enabled", loc->enabled ? "y" : "n");
+    uiout->field_string ("enabled", (loc->disabled_by_cond ? "N*"
+				     : (loc->enabled ? "y" : "n")));
   else
     uiout->field_fmt ("enabled", "%c", bpenables[(int) b->enable_state]);
 
@@ -6318,7 +6388,9 @@ print_one_breakpoint (struct breakpoint *b,
 	  && (!is_catchpoint (b) || is_exception_catchpoint (b)
 	      || is_ada_exception_catchpoint (b))
 	  && (allflag
-	      || (b->loc && (b->loc->next || !b->loc->enabled))))
+	      || (b->loc && (b->loc->next
+			     || !b->loc->enabled
+			     || b->loc->disabled_by_cond))))
 	{
 	  gdb::optional<ui_out_emit_list> locations_list;
 
@@ -6412,6 +6484,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
   int print_address_bits = 0;
   int print_type_col_width = 14;
   struct ui_out *uiout = current_uiout;
+  bool has_disabled_by_cond_location = false;
 
   get_user_print_options (&opts);
 
@@ -6512,7 +6585,12 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
 	/* We only print out user settable breakpoints unless the
 	   show_internal is set.  */
 	if (show_internal || user_breakpoint_p (b))
-	  print_one_breakpoint (b, &last_loc, show_internal);
+	  {
+	    print_one_breakpoint (b, &last_loc, show_internal);
+	    for (bp_location *loc = b->loc; loc != NULL; loc = loc->next)
+	      if (loc->disabled_by_cond)
+		has_disabled_by_cond_location = true;
+	  }
       }
   }
 
@@ -6533,6 +6611,10 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
     {
       if (last_loc && !server_command)
 	set_next_address (last_loc->gdbarch, last_loc->address);
+
+      if (has_disabled_by_cond_location)
+	uiout->message (_("(*): Breakpoint condition is invalid at this "
+			  "location.\n"));
     }
 
   /* FIXME?  Should this be moved up so that it is only called when
@@ -6960,6 +7042,7 @@ bp_location::bp_location (breakpoint *owner, bp_loc_type type)
   this->cond_bytecode = NULL;
   this->shlib_disabled = 0;
   this->enabled = 1;
+  this->disabled_by_cond = false;
 
   this->loc_type = type;
 
@@ -8832,15 +8915,9 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	    loc->inserted = 1;
 	}
 
-      if (b->cond_string)
-	{
-	  const char *arg = b->cond_string;
-
-	  loc->cond = parse_exp_1 (&arg, loc->address,
-				   block_for_pc (loc->address), 0);
-	  if (*arg)
-              error (_("Garbage '%s' follows condition"), arg);
-	}
+      /* Do not set breakpoint locations conditions yet.  As locations
+	 are inserted, they get sorted based on their addresses.  Let
+	 the list stabilize to have reliable location numbers.  */
 
       /* Dynamic printf requires and uses additional arguments on the
 	 command line, otherwise it's an error.  */
@@ -8855,6 +8932,19 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 	error (_("Garbage '%s' at end of command"), b->extra_string);
     }
 
+
+  /* The order of the locations is now stable.  Set the location
+     condition using the location's number.  */
+  int loc_num = 1;
+  for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
+    {
+      if (b->cond_string != nullptr)
+	set_breakpoint_location_condition (b->cond_string, loc, b->number,
+					   loc_num);
+
+      ++loc_num;
+    }
+
   b->display_canonical = display_canonical;
   if (location != NULL)
     b->location = std::move (location);
@@ -9143,6 +9233,50 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
     }
 }
 
+/* Call 'find_condition_and_thread' for each sal in SALS until a parse
+   succeeds.  The parsed values are written to COND_STRING, THREAD,
+   TASK, and REST.  See the comment of 'find_condition_and_thread'
+   for the description of these parameters and INPUT.  */
+
+static void
+find_condition_and_thread_for_sals (const std::vector<symtab_and_line> &sals,
+				    const char *input, char **cond_string,
+				    int *thread, int *task, char **rest)
+{
+  int num_failures = 0;
+  for (auto &sal : sals)
+    {
+      char *cond = nullptr;
+      int thread_id = 0;
+      int task_id = 0;
+      char *remaining = nullptr;
+
+      /* Here we want to parse 'arg' to separate condition from thread
+	 number.  But because parsing happens in a context and the
+	 contexts of sals might be different, try each until there is
+	 success.  Finding one successful parse is sufficient for our
+	 goal.  When setting the breakpoint we'll re-parse the
+	 condition in the context of each sal.  */
+      try
+	{
+	  find_condition_and_thread (input, sal.pc, &cond, &thread_id,
+				     &task_id, &remaining);
+	  *cond_string = cond;
+	  *thread = thread_id;
+	  *task = task_id;
+	  *rest = remaining;
+	  break;
+	}
+      catch (const gdb_exception_error &e)
+	{
+	  num_failures++;
+	  /* If no sal remains, do not continue.  */
+	  if (num_failures == sals.size ())
+	    throw;
+	}
+    }
+}
+
 /* Decode a static tracepoint marker spec.  */
 
 static std::vector<symtab_and_line>
@@ -9306,13 +9440,8 @@ create_breakpoint (struct gdbarch *gdbarch,
 
 	  const linespec_sals &lsal = canonical.lsals[0];
 
-	  /* Here we only parse 'arg' to separate condition
-	     from thread number, so parsing in context of first
-	     sal is OK.  When setting the breakpoint we'll
-	     re-parse it in context of each sal.  */
-
-	  find_condition_and_thread (extra_string, lsal.sals[0].pc,
-				     &cond, &thread, &task, &rest);
+	  find_condition_and_thread_for_sals (lsal.sals, extra_string,
+					      &cond, &thread, &task, &rest);
 	  cond_string_copy.reset (cond);
 	  extra_string_copy.reset (rest);
         }
@@ -13414,6 +13543,9 @@ locations_are_equal (struct bp_location *a, struct bp_location *b)
       if (a->enabled != b->enabled)
 	return 0;
 
+      if (a->disabled_by_cond != b->disabled_by_cond)
+	return 0;
+
       a = a->next;
       b = b->next;
     }
@@ -13521,10 +13653,7 @@ update_breakpoint_locations (struct breakpoint *b,
 	    }
 	  catch (const gdb_exception_error &e)
 	    {
-	      warning (_("failed to reevaluate condition "
-			 "for breakpoint %d: %s"), 
-		       b->number, e.what ());
-	      new_loc->enabled = 0;
+	      new_loc->disabled_by_cond = true;
 	    }
 	}
 
@@ -13549,7 +13678,7 @@ update_breakpoint_locations (struct breakpoint *b,
 
     for (; e; e = e->next)
       {
-	if (!e->enabled && e->function_name)
+	if ((!e->enabled || e->disabled_by_cond) && e->function_name)
 	  {
 	    struct bp_location *l = b->loc;
 	    if (have_ambiguous_names)
@@ -13565,7 +13694,8 @@ update_breakpoint_locations (struct breakpoint *b,
 		       enough.  */
 		    if (breakpoint_locations_match (e, l, true))
 		      {
-			l->enabled = 0;
+			l->enabled = e->enabled;
+			l->disabled_by_cond = e->disabled_by_cond;
 			break;
 		      }
 		  }
@@ -13576,7 +13706,8 @@ update_breakpoint_locations (struct breakpoint *b,
 		  if (l->function_name
 		      && strcmp (e->function_name, l->function_name) == 0)
 		    {
-		      l->enabled = 0;
+		      l->enabled = e->enabled;
+		      l->disabled_by_cond = e->disabled_by_cond;
 		      break;
 		    }
 	      }
@@ -13650,9 +13781,9 @@ location_to_sals (struct breakpoint *b, struct event_location *location,
 	  char *cond_string, *extra_string;
 	  int thread, task;
 
-	  find_condition_and_thread (b->extra_string, sals[0].pc,
-				     &cond_string, &thread, &task,
-				     &extra_string);
+	  find_condition_and_thread_for_sals (sals, b->extra_string,
+					      &cond_string, &thread,
+					      &task, &extra_string);
 	  gdb_assert (b->cond_string == NULL);
 	  if (cond_string)
 	    b->cond_string = cond_string;
@@ -14137,6 +14268,10 @@ enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
   struct bp_location *loc = find_location_by_number (bp_num, loc_num);
   if (loc != NULL)
     {
+      if (loc->disabled_by_cond && enable)
+	error (_("Breakpoint %d's condition is invalid at location %d, "
+		 "cannot enable."), bp_num, loc_num);
+
       if (loc->enabled != enable)
 	{
 	  loc->enabled = enable;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index b9a605e..7d02ced 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -387,6 +387,12 @@ public:
   /* Is this particular location enabled.  */
   bool enabled = false;
   
+  /* Is this particular location disabled because the condition
+     expression is invalid at this location.  For a location to be
+     reported as enabled, the ENABLED field above has to be true *and*
+     the DISABLED_BY_COND field has to be false.  */
+  bool disabled_by_cond = false;
+
   /* True if this breakpoint is now inserted.  */
   bool inserted = false;
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 2b2b4e5..84c173b 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.texinfo (Set Breaks): Document disabling of breakpoint
+	locations for which the breakpoint condition is invalid.
+
 2020-10-06  Michael Forney  <mforney@mforney.org>
 
 	* Makefile.in (HAVE_NATIVE_GCORE_HOST): Add for gcore.1
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2636b6f..9da79ed 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4278,6 +4278,46 @@ value is nonzero---that is, if @var{cond} evaluates as true.
 above (or no argument) specifying where to break.  @xref{Conditions,
 ,Break Conditions}, for more information on breakpoint conditions.
 
+The breakpoint may be mapped to multiple locations.  If the breakpoint
+condition @var{cond} is invalid at some but not all of the locations,
+the locations for which the condition is invalid are disabled.  For
+example, @value{GDBN} reports below that two of the three locations
+are disabled.
+
+@smallexample
+(@value{GDBP}) break func if a == 10
+warning: failed to validate condition at location 0x11ce, disabling:
+  No symbol "a" in current context.
+warning: failed to validate condition at location 0x11b6, disabling:
+  No symbol "a" in current context.
+Breakpoint 1 at 0x11b6: func. (3 locations)
+@end smallexample
+
+Locations that are disabled because of the condition are denoted by an
+uppercase @code{N} in the output of the @code{info breakpoints}
+command:
+
+@smallexample
+(@value{GDBP}) info breakpoints
+Num     Type           Disp Enb Address            What
+1       breakpoint     keep y   <MULTIPLE>
+        stop only if a == 10
+1.1                         N*  0x00000000000011b6 in ...
+1.2                         y   0x00000000000011c2 in ...
+1.3                         N*  0x00000000000011ce in ...
+(*): Breakpoint condition is invalid at this location.
+@end smallexample
+
+If the breakpoint condition @var{cond} is invalid in the context of
+@emph{all} the locations of the breakpoint, @value{GDBN} refuses to
+define the breakpoint.  For example, if variable @code{foo} is an
+undefined variable:
+
+@smallexample
+(@value{GDBP}) break func if foo
+No symbol "foo" in current context.
+@end smallexample
+
 @kindex tbreak
 @item tbreak @var{args}
 Set a breakpoint enabled only for one stop.  The @var{args} are the
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 29db747..4e5f5dc 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-10-27  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.base/condbreak-multi-context.cc: New file.
+	* gdb.base/condbreak-multi-context.exp: New file.
+
 2020-10-26  Tom Tromey  <tom@tromey.com>
 
 	* lib/mi-support.exp (default_mi_gdb_start): Call
diff --git a/gdb/testsuite/gdb.base/condbreak-multi-context.cc b/gdb/testsuite/gdb.base/condbreak-multi-context.cc
new file mode 100644
index 0000000..5e994e2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-multi-context.cc
@@ -0,0 +1,54 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+class Base
+{
+public:
+  int b = 20;
+
+  void func () {}
+};
+
+class A : public Base
+{
+public:
+  int a = 10;
+
+  void func () {}
+};
+
+class C : public Base
+{
+public:
+  int c = 30;
+
+  void func () {}
+};
+
+int
+main ()
+{
+  Base bobj;
+  A aobj;
+  C cobj;
+
+  aobj.func ();
+  bobj.func ();
+  cobj.func ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/condbreak-multi-context.exp b/gdb/testsuite/gdb.base/condbreak-multi-context.exp
new file mode 100644
index 0000000..4e56d36
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-multi-context.exp
@@ -0,0 +1,230 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test defining a conditional breakpoint that applies to multiple
+# locations with different contexts (e.g. different set of local vars).
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" ${binfile} ${srcfile}]} {
+    return
+}
+
+set warning "warning: failed to validate condition"
+set fill "\[^\r\n\]*"
+
+# Check that breakpoints are as expected.
+
+proc test_info_break {suffix} {
+    global bpnum1 bpnum2 fill
+
+    set bp_hit_info "${fill}(\r\n${fill}breakpoint already hit 1 time)?"
+
+    gdb_test "info break ${bpnum1} ${bpnum2}" \
+	[multi_line \
+	     "Num${fill}" \
+	     "${bpnum1}${fill}breakpoint${fill}keep y${fill}MULTIPLE${fill}" \
+	     "${fill}stop only if a == 10${bp_hit_info}" \
+	     "${bpnum1}.1${fill}N\\*${fill}Base::func${fill}" \
+	     "${bpnum1}.2${fill}y${fill}A::func${fill}" \
+	     "${bpnum1}.3${fill}N\\*${fill}C::func${fill}" \
+	     "${bpnum2}${fill}breakpoint${fill}keep y${fill}MULTIPLE${fill}" \
+	     "${fill}stop only if c == 30${bp_hit_info}" \
+	     "${bpnum2}.1${fill}N\\*${fill}Base::func${fill}" \
+	     "${bpnum2}.2${fill}N\\*${fill}A::func${fill}" \
+	     "${bpnum2}.3${fill}y${fill}C::func${fill}" \
+	     "\\(\\*\\): Breakpoint condition is invalid at this location."] \
+	"info break $suffix"
+}
+
+# Scenario 1: Define breakpoints conditionally, using the "break N if
+# cond" syntax.  Run the program, check that we hit those locations
+# only.
+
+with_test_prefix "scenario 1" {
+    # Define the conditional breakpoints.
+    gdb_test "break func if a == 10" \
+	[multi_line \
+	     "${warning} at location 1, disabling:" \
+	     "  No symbol \"a\" in current context." \
+	     "${warning} at location 3, disabling:" \
+	     "  No symbol \"a\" in current context." \
+	     "Breakpoint $decimal at $fill .3 locations."] \
+	"define bp with condition a == 10"
+    set bpnum1 [get_integer_valueof "\$bpnum" 0 "get bpnum1"]
+
+    gdb_test "break func if c == 30" \
+	[multi_line \
+	     ".*${warning} at location 1, disabling:" \
+	     "  No symbol \"c\" in current context." \
+	     ".*${warning} at location 2, disabling:" \
+	     "  No symbol \"c\" in current context." \
+	     ".*Breakpoint $decimal at $fill .3 locations."] \
+	"define bp with condition c == 30"
+    set bpnum2 [get_integer_valueof "\$bpnum" 0 "get bpnum2"]
+
+    test_info_break 1
+
+    # Do not use runto_main, it deletes all breakpoints.
+    gdb_run_cmd
+
+    # Check our conditional breakpoints.
+    gdb_test "" ".*Breakpoint \[0-9\]+, A::func .*" \
+	"run until A::func"
+    gdb_test "print a" " = 10"
+
+    gdb_test "continue" "Continuing.*Breakpoint \[0-9\]+, C::func .*" \
+	"run until C::func"
+    gdb_test "print c" " = 30"
+
+    # No more hits!
+    gdb_continue_to_end
+
+    test_info_break 2
+}
+
+# Start GDB with two breakpoints and define the conditions separately.
+
+proc setup_bps {} {
+    global srcfile binfile srcfile2
+    global bpnum1 bpnum2 bp_location warning
+
+    clean_restart ${binfile}
+
+    # Define the breakpoints.
+    gdb_breakpoint "func"
+    set bpnum1 [get_integer_valueof "\$bpnum" 0 "get bpnum1"]
+
+    gdb_breakpoint "func"
+    set bpnum2 [get_integer_valueof "\$bpnum" 0 "get bpnum2"]
+
+    # Defining a condition on 'a' disables 2 locations.
+    gdb_test "cond $bpnum1 a == 10" \
+	[multi_line \
+	     "$warning at location ${bpnum1}.1, disabling:" \
+	     "  No symbol \"a\" in current context." \
+	     "$warning at location ${bpnum1}.3, disabling:" \
+	     "  No symbol \"a\" in current context."]
+
+    # Defining a condition on 'c' disables 2 locations.
+    gdb_test "cond $bpnum2 c == 30" \
+	[multi_line \
+	     "$warning at location ${bpnum2}.1, disabling:" \
+	     "  No symbol \"c\" in current context." \
+	     "$warning at location ${bpnum2}.2, disabling:" \
+	     "  No symbol \"c\" in current context."]
+}
+
+# Scenario 2: Define breakpoints unconditionally, and then define
+# conditions using the "cond N <cond>" syntax.  Expect that the
+# locations where <cond> is not evaluatable are disabled.  Run the
+# program, check that we hit the enabled locations only.
+
+with_test_prefix "scenario 2" {
+    setup_bps
+
+    test_info_break 1
+
+    # Do not use runto_main, it deletes all breakpoints.
+    gdb_run_cmd
+
+    # Check that we hit enabled locations only.
+    gdb_test "" ".*Breakpoint \[0-9\]+, A::func .*" \
+	"run until A::func"
+    gdb_test "print a" " = 10"
+
+    gdb_test "continue" "Continuing.*Breakpoint \[0-9\]+, C::func .*" \
+	"run until C::func"
+    gdb_test "print c" " = 30"
+
+    # No more hits!
+    gdb_continue_to_end
+
+    test_info_break 2
+}
+
+# Test the breakpoint location enabled states.
+
+proc check_bp_locations {bpnum states msg} {
+    global fill
+
+    set expected  ".*${bpnum}.1${fill} [lindex $states 0] ${fill}\r\n"
+    append expected "${bpnum}.2${fill} [lindex $states 1] ${fill}\r\n"
+    append expected "${bpnum}.3${fill} [lindex $states 2] ${fill}"
+    if {[lsearch $states N*] >= 0} {
+	append expected "\r\n\\(\\*\\): Breakpoint condition is invalid at this location."
+    }
+
+    gdb_test "info break $bpnum" $expected "check bp $bpnum $msg"
+}
+
+# Scenario 3: Apply misc. checks on the already-defined breakpoints.
+
+with_test_prefix "scenario 3" {
+    setup_bps
+
+    gdb_test "cond $bpnum1 c == 30" \
+	[multi_line \
+	     "${warning} at location ${bpnum1}.1, disabling:" \
+	     "  No symbol \"c\" in current context." \
+	     "${warning} at location ${bpnum1}.2, disabling:" \
+	     "  No symbol \"c\" in current context." \
+	     "Breakpoint ${bpnum1}'s condition is now valid at location 3, enabling."] \
+	"change the condition of bp 1"
+    check_bp_locations $bpnum1 {N* N* y} "after changing the condition"
+
+    gdb_test "cond $bpnum1" \
+	[multi_line \
+	     "Breakpoint ${bpnum1}'s condition is now valid at location 1, enabling." \
+	     "Breakpoint ${bpnum1}'s condition is now valid at location 2, enabling." \
+	     "Breakpoint ${bpnum1} now unconditional."] \
+	"reset the condition of bp 1"
+    check_bp_locations $bpnum1 {y y y} "after resetting the condition"
+
+    gdb_test_no_output "disable ${bpnum2}.2"
+    check_bp_locations $bpnum2 {N* N* y} "after disabling loc 2"
+
+    gdb_test "cond $bpnum2" ".*" "reset the condition of bp 2"
+    check_bp_locations $bpnum2 {y n y} "loc 2 should remain disabled"
+
+    gdb_test_no_output "disable ${bpnum2}.3"
+    check_bp_locations $bpnum2 {y n n} "after disabling loc 3"
+
+    gdb_test "cond $bpnum2 c == 30" \
+	[multi_line \
+	     "${warning} at location ${bpnum2}.1, disabling:" \
+	     "  No symbol \"c\" in current context."] \
+	"re-define a condition"
+    check_bp_locations $bpnum2 {N* N* n} "loc 3 should remain disabled"
+
+    gdb_test "enable ${bpnum2}.1" \
+	"Breakpoint ${bpnum2}'s condition is invalid at location 1, cannot enable." \
+	"reject enabling a location that is disabled-by-cond"
+    check_bp_locations $bpnum2 {N* N* n} "after enable attempt"
+
+    gdb_test "cond $bpnum2 garbage" \
+	"No symbol \"garbage\" in current context." \
+	"reject condition if bad for all locations"
+
+    gdb_test_no_output "delete $bpnum1"
+
+    # Do not use runto_main, it deletes all breakpoints.
+    gdb_breakpoint "main"
+    gdb_run_cmd
+    gdb_test "" ".*reakpoint .*, main .*${srcfile}.*" "start"
+
+    # The second BP's locations are all disabled.  No more hits!
+    gdb_continue_to_end
+}