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 <jing...@apple.com> wrote: > > > > On Jan 7, 2021, at 3:57 PM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Thu, Jan 7, 2021 at 3:37 PM Jim Ingham via lldb-commits > > <lldb-commits@lists.llvm.org> wrote: > >> > >> > >> > >>> On Jan 7, 2021, at 2:29 PM, David Blaikie via Phabricator via > lldb-commits <lldb-commits@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@lists.llvm.org > >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > >> > >> _______________________________________________ > >> lldb-commits mailing list > >> lldb-commits@lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
