[PATCH 2/3] gdb/breakpoint: set the condition exp after parsing the condition successfully
Aktemur, Tankut Baris
tankut.baris.aktemur@intel.com
Thu Jul 23 07:11:05 GMT 2020
On Wednesday, July 22, 2020 6:06 PM, Simon Marchi wrote:
> On 2020-07-22 11:29 a.m., Aktemur, Tankut Baris wrote:
> > 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.
>
> Ok, so do you want to wait and post everything together, or do still want to consider
> merging this one on its own, since it's still a step forward?
I'd like to merge this on its own.
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
More information about the Gdb-patches
mailing list