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

[Tom Tromey <tromey at redhat dot com>]

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

[...]
Pedro> This is why I proposed in the response to Jan, that if we want
Pedro> to group different "_source_ locations" under the same breakpoint,
Pedro> we implement it as a 3rd hierarchy, so you'd get:

Ok, I think I mostly understand this proposal.

I would like to work through this and come to some agreement, since I am
planning to implement the result -- not just for SystemTap probes but
also because I recently starting looking at an ambiguous name feature
request we've had open in RH bugzilla for far too long.

My current understanding is that an ambiguous linespec should cause
`break' to make a new "3rd-tier" breakpoint with sub-breakpoints for
each separate source location.  These sub-breakpoints would have
canonicalized linespecs for better stable naming.  Furthermore they
would be created and destroyed automatically in response to inferior
changes.

FWIW, I tend to think the 3-tier solution is too complicated.  I think
having two ways that a breakpoint can have multiple locations will
confuse users.

Also, I came up with some cases I don't understand; I feel certain there
are more lurking.

Suppose the user does `break function' where `function' is *not*
ambiguous.  This makes an ordinary breakpoint.  Now suppose the inferior
calls dlopen and the new library has a definition of `function'.  What
happens?

1. Breakpoint changes from ordinary to 3-tier.
2. We add logic to canonicalize the existing breakpoint's linespec
   and do not set a new breakpoint.
3. We didn't make an ordinary breakpoint after all, all breakpoints are
   3-tier.

Choice 1 seems potentially weird, though maybe it would work ok?

Choice 2 seems bad in some circumstances.  `function' might be inlined
in some places, in which case you would actively want new breakpoint
locations to be set.

Choice 3 introduces complexity for all uses even though most breakpoints
aren't in fact ambiguous.


Here is an oddity: consider `break probe:something'.  Now suppose this
probe was used in multiple locations -- one having debuginfo and two not
having debuginfo.

I think this implies that a 3-tier breakpoint must have one special
sub-breakpoint that collects locations from debuginfo-less matches.  Or
perhaps each could be given its own sub-breakpoint -- but there is no
sensible way to provide canonical linespecs here, so matching won't
really work.


Suppose the inferior uses dlmopen and loads the same library twice.  Now
there is really no way to canonicalize a linespec -- it can't even be
objfile-relative (postulate for a moment that there is a syntax for
this).



I think most of these cases are readily resolveable with the "break at
all matching locations" approach:

* Unambiguous -> ambiguous transition is no problem.
  Breakpoint condition may no longer parse -- not super, but workable.

  (As an aside: seeing those "can't parse" messages has bugged me in the
  past, it seems like it would be good to have a way to tell GDB "if you
  can't parse it, please stop right away so I can fix the darn thing".)

* The no-debuginfo SystemTap probe case: no problem in itself, and we
  could simply mandate that such a breakpoint can only have certain
  expressions attached (also mentioned below).

* dlmopen.  Also no problem, the user asked for it :)


One advantage of 3-tier is that it does let you put different conditions
on different sub-breakpoints.  (But the general idea of the simpler
approach is that you should disambiguate if you want tight control.
Which gives me the idea that we could make a new break command that does
just does automatically.)


I think either case will need some expansion of the matching heuristic
for breakpoints, and maybe some linespec additions to make it possible
to disambiguate more cases.


I am not very concerned about the MI problem.  We can emit a new field
holding a vector of file:line locations, or something like that.  Front
ends will adapt.

[...]
Pedro> You still have manifestations of the same problem.  Location numbers are
Pedro> supposed to be stable.  For example, users can enable/disable locations
Pedro> individually.

This problem is fundamental, just because there is no stable name for
source locations in the face of changes to the source.  We are always
going to have heuristics in GDB, and some losing cases.

Pedro> See, "bp_location" concerns with address locations while the user
Pedro> the user is concerned about "source locations".  I think that
Pedro> if "bp_location" had been named "bp_compiled_address", we wouldn't
Pedro> be having this discussion.  :-)

For the SystemTap probe case in particular, I am not so sure.

I mean, I can see how it would be useful to an MI consumer if GDB
emitted file:line locations for all the locations hit by a "probe:"
breakpoint.  This is not fundamental to one approach or the other, it
can be made to work.

I think it would be a reasonable choice on our part to simply mandate
that a SystemTap probe breakpoint have a single condition (for all
locations) which could only refer to globals and probe arguments.

Pedro> It's not necessarily a bad thing (and not necessarily a good thing
Pedro> either).   If you have a big project, with a bunch of static
Pedro> "foo" functions, it's not unreasonable to have "b foo"
Pedro> break at the "closest" function with that name in your context.

Sure, but that isn't what actually happens.  And, this would require
separate thinking.  Like, suppose the "closest" metric yields an
ambiguous result, what should happen?

Pedro> What would "list parse_number" do if "break parse_number" breaks
Pedro> in all the instances of that static function?

Error or give a menu.

Tom