RE: [PATCH 2/3] gdb/breakpoint: set the condition exp after parsing the condition successfully

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

On Wednesday, July 22, 2020 3:28 PM, Simon Marchi wrote:
> Although, in the breakpoint case, when we have:
> 
> 	  for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
> 	    {
> 	      const char *arg = exp;
> 	      expression_up new_exp
> 		= parse_exp_1 (&arg, loc->address,
> 			       block_for_pc (loc->address), 0);
> 	      if (*arg != 0)
> 		error (_("Junk at end of expression"));
> 	      loc->cond = std::move (new_exp);
> 	    }
> 
> Doesn't that mean that if the expression succeeds to parse for one location and then
> fails to parse for another location, we'll have updated one location and not the other?

Ahh, yes.  The diff for the part above should have been:

          struct bp_location *loc;

+         /* 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
+            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 (loc = b->loc; loc; loc = loc->next)
+           {
+             arg = exp;
+             parse_exp_1 (&arg, loc->address,
+                          block_for_pc (loc->address), 0);
+             if (*arg != 0)
+               error (_("Junk at end of expression"));
+           }
+
+         /* If we reach here, the condition is valid at all locations.  */
          for (loc = b->loc; loc; loc = loc->next)
            {
              arg = exp;
              loc->cond =
                parse_exp_1 (&arg, loc->address,
                             block_for_pc (loc->address), 0);
-             if (*arg)
-               error (_("Junk at end of expression"));
            }

> How does that work (or should work) when we have a multi-location breakpoint and the
> condition only makes sense in one of the locations?

I'm in fact working on a follow-up patch on this topic, where the two-pass approach above
is used (hence I forgot to include it in this series).

Currently, GDB expects the condition to be valid at all locations.  The patch that I'll
soon post proposes to accept the condition if there exist locations where it's valid.
The locations where the condition is invalid are disabled.  But in the current state, the
condition has to make sense at all locations.

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928