[lldb-dev] Setting breakpoint on file and function name doesn't work as expected.

[Konrad Kleine via lldb-dev]

Hi Jim,

Am Fr., 8. Nov. 2019 um 19:57 Uhr schrieb Jim Ingham <jingham at apple.com>:

>
>
> > On Nov 8, 2019, at 1:53 AM, Konrad Kleine <kkleine at redhat.com> wrote:
> >
> > Jim,
> >
> > thank you for the explanation. I'm trying to see the situation more from
> an end user's perspective. When --file or -f have two different meanings
> depending on how they are combined, that's bad IMHO.
>
> I don't think that it is bad that the file parameter in a "file and line"
> breakpoint and the file parameter in a function name breakpoint have
> different meanings.  That might very well make sense when you think about
> the kind of search the breakpoint is likely to do.  But this does raise a
> problem with the documentation.
>

I think it is dangerous to make too many assumptions and given that people
are lazy I think even with a good piece of documentation people would still
get it wrong. To me this is similar to a documentation that says: When you
cross the street in this direction, go when the light is green. When you
cross the street in the other direction, go when the light is red. With any
documentation, no matter how accurate it is you will have people not read
it entirely. For example, I used -f -n in a way and it worked. Then I
enabled LTO and my debugging habits began to break.

 One way to do it is to try to list all the meanings for each option ("when
> used in conjunction with...")  I don't think there are actually enough
> variants that this will bloat the documentation over much, but that's
> something to watch out for.
>

For the time being and the code working the way it is I'm totally not
against any documentation updates.

>
> Another thing I've thought about doing is adding the ability to have help
> include one of the non-optional, non-overlapping options to the command, so
> you could say:
>
> (lldb) help break set -n
>
> and that would tell you that this is a "by function name" breakpoint, and
> in that case -n means...  That might help reduce the information overload,
> and give a better sense of what these complex commands do.
>
> As I said, it would have been better from a documentation standpoint to
> make all these different breakpoint commands sub-commands of "break
> set"("break set function", "break set file-and-line', etc...) but I think
> people would find that too verbose.
>
> >
> > From what I read in your response I get the feeling that you assume a
> user knows about the difference between CU and his or her source file and
> the implications it can have when for example LTO is enabled and we make
> heavy use of inlining. I see this as a problem because source-level
> debugging for a function name and a file to an end user means exactly that,
> nomatter where the function is inlined. Do you agree?
>
> I am not sure what you are asking me to agree to.
>

> (lldb) break set -n foo -f bar.*
>
> means "set the breakpoint on functions named foo DEFINED in the file
> "bar.*".
>

My example with foo() was very contrived but not unusual. Take any template
function for example:

// foo.h
int foo(){ return 42; }

template <typename T>
T twice(T arg) { return arg+arg; }

// main.cpp
#include "foo.h"
int main(){return twice(foo());}

When I want to break on the function twice() defined in foo.h I would go
for "-f foo.h -n twice<int>" but I have to go for "-f main.cpp -n
twice<int>". And in terms of DWARF, there's enough to let me do the first
variant:

DW_AT_name ("twice<int>")
DW_AT_decl_file ("/home/kkleine/./foo.h")
DW_AT_decl_line (4)

Why don't we respect those DW_AT_decl_file and DW_AT_decl_line? Those are
always there, for inlining (with LTO), for templates and for regular
functions.


> It could mean other things in the context of inlining, for instance you
> might want to tell lldb to break on the function "foo" whenever it is
> inlined INTO the CU bar.*.  That's also a perfectly valid thing to do, and
> you might think "-n -f" was the combination to do that, but it is not what
> it does.


I understand that, the question, as I asked it above, is why we just search
by CU?


> Again, the feature was intended to disambiguate between different
> functions with the same name by definition site which the current
> definition does.  So in this sense the user will have to know what the -f
> means (and we do need some good solution for documenting this more
> clearly.)
>

> Back to your original query...  If the function is defined in a .h file,
> or gets inlined by LTO, this filtering is trickier, and I didn't implement
> that behavior when I implemented this breakpoint type.  So in that case,
> and in the case where LTO inlines a function, the feature isn't implemented
> correctly.


Okay, that's good to know.


> The -n searche always looks for out of line and inline instances when
> doing the search.  So we already get the searcher to all the instances.
>  You would just have to widen the search beyond "Does the CU match" to try
> to figure out where the inlined instance was defined.
>

I will take a look at the code but I already noticed that call to
CompUnitPasses which by default returns true and is only overwritten in
some circumstances.

Thank you for taking the time to go through this.

Konrad
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20191111/f2673a5d/attachment.html>