How were you setting the breakpoint in the case where it was failing from the command line? The breakpoint you are examining in the test in your patches is a "source regular expression" breakpoint. That would be "break set -p". If you were also doing that in the command line and got a different result that would be very odd, since neither interface (CLI or SB) does much work before handing the request off to the common routine. If (as I suspect) you were doing "b main.c:40" in the command line, then you should make a breakpoint with SBTarget.BreakpointCreateByLocation and see if that shows the same problem. That should do the same job as "break set --file --line".
Note, you can also run the command line breakpoint command in API tests. That's sometimes required, particularly if there's something about a combination of command flags that you want to test. Just use the lldbutil.run_break_set_by_file_and_line utility. That centralizes running and checking the basic results of the break set command, so if we do change the break command we only have to fix one place... In your case the checks that run_break_set_by_file_and_line do should suffice, but the command returns the created breakpoint number, so if you want to poke at it further, you can use target.FindBreakpointByID to get the SBBreakpoint you just made. There are also wrappers for the other lldb breakpoint types you can make from the command line, BTW. Jim > On Jan 14, 2021, at 1:38 PM, David Blaikie <dblai...@gmail.com> wrote: > > Thanks for the pointers! > > So I tried writing an API-type test for this line table/breakpoint > behavior (it doesn't seem like it's entirely/just the line table (for > one thing, the DWARF change is purely in the subprogram DIE, not in > the line table)) and I can't seem to figure it out. > > The attached patch modifies lldb to undo this patch (to show the > breakage) and modifies an existing API test to build with the clang > flag to enable ranges on subprograms - I've confirmed the DWARF built > by this test does have subprogram ranges. > > But my best attempt at setting the breakpoint and examining it in the > API test seems to produce the desired filename and line number (ie: > does not exhibit the bug), where interacting with lldb on the command > line does not render the file/line (ie: the bug). > > Jim, Pavel, anyone: Ideas on how I can/should test this behavior? > > $ ./bin/llvm-lit -v > tools/lldb/test/API/lang/c/stepping/TestStepAndBreakpoints.py > -- Testing: 1 tests, 1 workers -- > FAIL: lldb-api :: lang/c/stepping/TestStepAndBreakpoints.py (1 of 1) > ******************** TEST 'lldb-api :: > lang/c/stepping/TestStepAndBreakpoints.py' FAILED ******************** > Script: > -- > /usr/bin/python3.8 > /usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/dotest.py -u > CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env > OBJCOPY=/usr/bin/objcopy --env > LLVM_LIBS_DIR=/usr/local/google/home/blaikie/dev/llvm/build/default/./lib > --arch x86_64 --build-dir > /usr/local/google/home/blaikie/dev/llvm/build/default/lldb-test-build.noindex > --lldb-module-cache-dir > /usr/local/google/home/blaikie/dev/llvm/build/default/lldb-test-build.noindex/module-cache-lldb/lldb-api > --clang-module-cache-dir > /usr/local/google/home/blaikie/dev/llvm/build/default/lldb-test-build.noindex/module-cache-clang/lldb-api > --executable /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/lldb > --compiler /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/clang > --dsymutil > /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/dsymutil > --filecheck > /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/FileCheck > --yaml2obj > /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/yaml2obj > --lldb-libs-dir > /usr/local/google/home/blaikie/dev/llvm/build/default/./lib > /usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/lang/c/stepping > -p TestStepAndBreakpoints.py > -- > Exit Code: 1 > > Command Output (stdout): > -- > lldb version 12.0.0 (g...@github.com:llvm/llvm-project.git revision > d49974f9c98ebce5a679eced9f27add138b881fa) > clang revision d49974f9c98ebce5a679eced9f27add138b881fa > llvm revision d49974f9c98ebce5a679eced9f27add138b881fa > Libc++ tests will not be run because: Compiling with -stdlib=libc++ > fails with the error: <stdin>:1:10: fatal error: 'algorithm' file not > found > #include <algorithm> > ^~~~~~~~~~~ > 1 error generated. > > Skipping following debug info categories: ['dsym', 'gmodules'] > objc tests will be skipped because of unsupported platform > > -- > Command Output (stderr): > -- > FAIL: LLDB > (/usr/local/google/home/blaikie/dev/llvm/build/default/bin/clang-x86_64) > :: test_and_python_api (TestStepAndBreakpoints.TestCStepping) > ====================================================================== > FAIL: test_and_python_api (TestStepAndBreakpoints.TestCStepping) > Test stepping over vrs. hitting breakpoints & subsequent stepping > in various forms. > ---------------------------------------------------------------------- > Traceback (most recent call last): > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 345, in wrapper > return func(self, *args, **kwargs) > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 135, in wrapper > return func(*args, **kwargs) > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 135, in wrapper > return func(*args, **kwargs) > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 105, in wrapper > func(*args, **kwargs) > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 105, in wrapper > func(*args, **kwargs) > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/decorators.py", > line 105, in wrapper > func(*args, **kwargs) > [Previous line repeated 1 more time] > File > "/usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/lang/c/stepping/TestStepAndBreakpoints.py", > line 48, in test_and_python_api > > self.assertEqual(get_description(break_1_in_main.locations[0].GetAddress().line_entry), > "narf") > AssertionError: > '/usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/lang/c/stepping/main.c:40:14' > != 'narf' > - > /usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/lang/c/stepping/main.c:40:14+ > narf > Config=x86_64-/usr/local/google/home/blaikie/dev/llvm/build/default/bin/clang > ---------------------------------------------------------------------- > Ran 1 test in 0.320s > > RESULT: FAILED (0 passes, 1 failures, 0 errors, 0 skipped, 0 expected > failures, 0 unexpected successes) > > -- > > ******************** > ******************** > Failed Tests (1): > lldb-api :: lang/c/stepping/TestStepAndBreakpoints.py > > > Testing Time: 0.77s > Failed: 1 > > $ > LD_LIBRARY_PATH=/usr/local/google/home/blaikie/dev/llvm/build/default/lib:/usr/local/google/home/blaikie/install/lib:/usr/local/google/home/blaikie/install/lib64 > ./bin/lldb > ./lldb-test-build.noindex/lang/c/stepping/TestStepAndBreakpoints.test_and_python_api/a.out > (lldb) target create > "./lldb-test-build.noindex/lang/c/stepping/TestStepAndBreakpoints.test_and_python_api/a.out" > Current executable set to > '/usr/local/google/home/blaikie/dev/llvm/build/default/lldb-test-build.noindex/lang/c/stepping/TestStepAndBreakpoints.test_and_python_api/a.out' > (x86_64). > (lldb) b main.c:40 > Breakpoint 1: where = a.out`main + 22, address = 0x00000000004011c6 > (lldb) b main > Breakpoint 2: where = a.out`main + 22, address = 0x00000000004011c6 > (lldb) b a > Breakpoint 3: where = a.out`a + 11 at main.c:8:24, address = > 0x000000000040111b > (lldb) > > > > On Mon, Jan 11, 2021 at 11:13 AM Jim Ingham <jing...@apple.com> wrote: >> >> >> >>> On Jan 10, 2021, at 5:37 PM, David Blaikie <dblai...@gmail.com> wrote: >>> >>> 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). >>> >> >> You can get directly at the line table from the SB API's. There's an >> example of doing just that on the SBCompileUnit page: >> >> https://lldb.llvm.org/python_reference/lldb.SBCompileUnit-class.html >> >> There's a small sample test in API/sample_test which is a good place to >> start. That one assumes you are running the target, however, which you may >> or may not need to do. We've accumulated lots of convenience methods over >> time to make testing easier, but we haven't back ported them to old tests. >> If you want other models than the sample_test, look for ones made more >> recently. >> >> There's some description of the API tests here: >> >> https://lldb.llvm.org/resources/test.html >> >> and some test writing info in the sources in: >> >> llvm-project/lldb/docs/testsuite/best-practices.txt >> >> >>> 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? >> >> I don't know much about how the lit runner works, somebody else will have to >> answer that. >> >> Jim >> >>> >>> 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_test.diff> _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits