Re: [PATCH 2/6] Introduce `pre_expanded sals'

[Tom Tromey <tromey@redhat.com>]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Tom> This is just a way for decode_line_1 to tell the breakpoint code that
Tom> the returned sals has multiple locations but should still create just a
Tom> single breakpoint.  We needed this because a given SystemTap probe name
Tom> may have multiple locations.

Pedro> Hmm, doesn't sound right.  Conceptually, breakpoint locations are
Pedro> multiple expansions of the same source location.  Different
Pedro> source locations are different breakpoints.  E.g, bp_location
Pedro> doesn't have line number or source file fields.  From the user's
Pedro> perpective, there's only a single "point" in the source code for
Pedro> all the multiple locations for a single breakpoint.

Could you explain why this is important?  I agree this is how it is, but
I think it is actually somewhat confusing at times.


The problem I see with respect to the SystemTap change is that a given
probe location does not have a canonical name.

E.g., suppose you have a probe `program:name' that is invoked at 2
different spots in the source.  Suppose further that `break
probe:program:name' sets 2 breakpoints.  Now consider 2 scenarios:
First, the developer deletes a probe point.  Second, the developer adds
a probe point.

With the single-breakpoint approach, both of these do (what I consider
to be) the right thing -- the user ends up with what he asked for.  I
don't see how they can work with the multiple-breakpoint approach.


Furthermore, I think the `probe:' prefix should let us lift this
restriction anyhow.  It is a way of saying "this is not an ordinary
source location, but something else".


I think the deeper confusion in gdb is that an ambiguous name sets a
single breakpoint, with a single location, but the meaning of this name
is decided arbitrarily (say, by psymtab expansion order).

That is, `break file.c:73' sets a breakpoint in the first `file.c' we
happen to trip across.  Or, `break function' sets a breakpoint in the
first `function'.

I'd rather change gdb to set a breakpoint at all matching locations, and
let the user disambiguate if that is really what he wanted.

I hit this while debugging gdb itself from time to time -- try `break
parse_number' and guess where it gets set.

Tom