[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

Sun Jan 10 17:37:33 PST 2021

Thanks for all the context - so sounds like mostly based on (3) the
recommendation would be for this to be an API test (is there a way to test
the line table directly? good place for reference on the SB API options - I
looked at a few tests and they seemed quite different
( lldb/test/API/functionalities/breakpoint/move_nearest/TestMoveNearest.py
and lldb/test/API/commands/breakpoint/set/func-regex/TestBreakpointRegexError.py
) in the way they're written, so not sure what the norms are/how they work).

But more fundamentally, seems all the API tests are "unsupported" on my
system, and I can't seem to figure out what makes them unsupported
according to lit. Any ideas?

On Thu, Jan 7, 2021 at 4:55 PM Jim Ingham <jingham at apple.com> wrote:

>
>
> > On Jan 7, 2021, at 3:57 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > On Thu, Jan 7, 2021 at 3:37 PM Jim Ingham via lldb-commits
> > <lldb-commits at lists.llvm.org> wrote:
> >>
> >>
> >>
> >>> On Jan 7, 2021, at 2:29 PM, David Blaikie via Phabricator via
> lldb-commits <lldb-commits at lists.llvm.org> wrote:
> >>>
> >>> dblaikie added a comment.
> >>>
> >>> In D94063#2485271 <https://reviews.llvm.org/D94063#2485271>, @labath
> wrote:
> >>>
> >>>> In D94063#2483546 <https://reviews.llvm.org/D94063#2483546>,
> @dblaikie wrote:
> >>>>
> >>>>> If it's better to write it using C++ source and custom clang flags I
> can do that instead (it'll be an -mllvm flag - looks like there's one other
> test that does that:
> `lldb/test/API/lang/objc/forward-decl/TestForwardDecl.py:
> dict(CFLAGS_EXTRAS="-dwarf-version=5 -mllvm -accel-tables=Dwarf"))`) -
> means the test will be a bit more convoluted to tickle the subprogram
> ranges, but not much - just takes two functions+function-sections.
> >>>>
> >>>> I certainly wouldn't want to drop the existing test.
> >>>
> >>> Ah, what's the tradeoff between the different test types here?
> >>
> >> This is my take (this has been a contentious issue so I'm sure there
> are other takes...):
> >>
> >> The "Shell" tests use pattern matching against the lldb command line
> output.  They are useful for testing the details of the command
> interaction. You can also do that using pexpect in the API tests, but the
> Python 2.7 version of pexpect seemed really flakey so we switched to shell
> tests for this sort of thing.
> >>
> >> Because you are matching against text output that isn't API, they are
> less stable.  For instance if we changed anything in the "break set"
> output, your test would fail(*).  And because you are picking details out
> of that text, the tests are less precise.  You either have to match more of
> the command line than you are actually testing for, which isn't a good
> practice, or you run the risk of finding the text you were looking for in a
> directory path or other unrelated part of the output.  Also they are harder
> to debug if you can't reproduce the failure locally, since it isn't easy to
> add internal checks/output to the test to try hypotheses.  Whenever I have
> run into failures of this sort the first thing I do is convert the test to
> an API test...
> >>
> >> But the main benefit of the "Shell" tests is that you can write tests
> without having to know Python or learn the lldb Python API's.  And if you
> are coming from clang you already know how FileCheck tests work, so that's
> a bonus.  I think it's legit to require that folks actually working on lldb
> learn the SB API's.  But we were persuaded that it wasn't fair to impose
> that on people not working on lldb, and yet such folks do sometimes need to
> write tests for lldb...  So for simple tests, the Shell tests are an okay
> option.  But really, there's nothing you can do in a Shell test that you
> can't do in an API test.
> >>
> >> The "API" tests use the Python SB API's - though they also have the
> ability to run commands and do expect type checks on the output so for
> single commands they work much as the shell tests do (there's even a
> FileCheck style assert IIRC).  They are a little more verbose than shell
> tests (though we've reduced the boilerplate significantly over the years).
> And of course you have to know the SB API's.  But for instance, if you
> wanted to know that a breakpoint was set on line 5 of foo.c, you can set
> the breakpoint, then ask the resultant SBBreakpoint object what it's file &
> line numbers were directly.  So once you've gotten familiar with the setup,
> IMO you can write much higher quality tests with the API tests.
> >>
> >>
> >> Jim
> >>
> >> (*) I am personally not at all in favor of the Shell tests, but that's
> in part because back in the day I was asked to make a simple and useful
> change to the output of the gdb "break" command but then righting the gdb
> testsuite - which is all based on expecting the results of various gdb
> commands - was so tedious that we ended up dropping the change instead.  I
> don't want to get to that place with lldb, but the hope is that as long as
> we mostly write API tests, we can avoid encumbering the current command
> outputs too heavily...
> >
> >
> > Thanks for the context, I really appreciate it.
> >
> > The aspect I was especially curious about here was less in regards to
> > the mechanics of how the behavior is examined/tested (between shell
> > and SB API) but more in regards to source code versus assembly - where
> > the assembly can more explicitly target some DWARF feature, but isn't
> > especially portable - whereas the source code could be portable to
> > test on different architectures, but might require either more
> > contortions to reliably produce the desired DWARF, or possibly extra
> > compiler flags (that was especialyl of interest since Pavel mentioned
> > these tests could be portable across compilers, so how would I specify
> > any necessary custom flags to get clang to produce the desired DWARF
> > (& the tests would be fairly useless for other compilers without those
> > flags/features, or might require very different ways to produce
> > similarly "interesting" DWARF))
>
> My understanding of this is:
>
> 1) If you don't need to run the resultant process to implement the test,
> then using .s files with hand-crafted DWARF is okay.  We pretty much always
> build support for all the major architectures when we do our testing, so
> everybody should be able to build your .s file, and so the test can do its
> job everywhere.  But if your test requires running the target, then a .s
> file is limiting, because it can only be run on the architecture it was
> intended for and people working on other platforms might break the test w/o
> knowing till the patch gets wider exposure, which we try to avoid.
>
> You are only testing the file & line value of a breakpoint (really you are
> just using that to probe the line table, but whatever...)  That test can be
> done w/o running the process, so using a hand-crafted .s file should be
> fine.  If you are testing the DWARF for values, you can often use a static
> variable as the value.  In that case a .s file is okay as well, since you
> can inspect static variables w/o running the process.
>
> 2) There are circumstances where the only way you can test something is
> with a .s file.  For instance if you are trying to assert that lldb doesn't
> crash in the face of buggy DWARF you don't want there to be a compiler that
> generates that output, so a .s file is necessary even if you have to run
> the process.  If you were being really complete and you have a compiler
> that generates the same bug in different architectures you could increase
> coverage by generating .s files from different architectures and picking
> the one you can run.  But I don't remember anybody having actually done
> that.
>
> 3) However, we try to push as many tests as possible all the way through
> the compiler, since the lldb test suite is also one of the significant test
> harnesses for compiler debug output.  .s files are exposed to much less of
> the compiler, and so don't catch new compiler debug output bugs as well.
> So unless you have a good reason to use a .s file, you shouldn't.
>
> As for passing compiler/environment dependent extra flags to the compiler,
> I don't know how you would do that in a lit/FileCheck test.  I'm assuming
> there are platform substituting facilities in FileCheck or lit that you
> could use in the clang RUN line, but I'm not very familiar with lit or
> FileCheck so I don't know what they are.
>
> The Python testsuite knows what architecture it is targeting, and what
> compiler it is using.  Most of the tests use a fixed Makefile and class
> descending from the Builder class from the lldb testsuite (in
> lldb/packages/Python/lldbsuite/test/builders) to assemble make flags, and
> do the actual building.  I haven't had to do compiler dependent flags so
> I'm not sure how this would go exactly, but it shouldn't be particularly
> hard to have your test inject some dynamically determined make variables
> that you would use in EXTRA_CFLAGS in your Makefile.
>
> Jim
>
> >
> > - Dave
> >
> >>
> >>
> >> Jim
> >>
> >>>
> >>>> However, it could be useful to have c++ test too. This one could
> feature a more complicated executable, and be more open-ended/exploratory
> and test end-to-end functionality (including compiler integration), instead
> of a targeted "did we parse DW_AT_ranges correctly" regression test. Then
> this test could go into the `API` test category, as we have the ability to
> run those kinds of tests against different compilers.
> >>>
> >>> Does this include support for custom compiler flags (it'd currently
> take a non-official/internal-only llvm flag to create the DW_AT_ranges on a
> subprogram that I have in mind, for instance)?
> >>>
> >>>> However, all of that is strictly optional.
> >>>
> >>> I'll consider it for a separate commit.
> >>>
> >>>
> >>> Repository:
> >>> rG LLVM Github Monorepo
> >>>
> >>> CHANGES SINCE LAST ACTION
> >>> https://reviews.llvm.org/D94063/new/
> >>>
> >>> https://reviews.llvm.org/D94063
> >>>
> >>> _______________________________________________
> >>> lldb-commits mailing list
> >>> lldb-commits at lists.llvm.org
> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >>
> >> _______________________________________________
> >> lldb-commits mailing list
> >> lldb-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210110/d4203177/attachment-0001.html>