[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms
David Blaikie via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 14 13:38:53 PST 2021
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 (git at 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 <jingham at apple.com> wrote:
>
>
>
> > On Jan 10, 2021, at 5:37 PM, David Blaikie <dblaikie at 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 <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 --------------
A non-text attachment was scrubbed...
Name: lldb_test.diff
Type: application/octet-stream
Size: 3394 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210114/1389daeb/attachment-0001.obj>
More information about the lldb-commits
mailing list