> 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
