This is an archive of the discontinued LLVM Phabricator instance.

lldb: Add support for DW_AT_ranges on DW_TAG_subprograms
ClosedPublic

Authored by dblaikie on Jan 5 2021, 12:11 AM.

Details

Summary

gcc already produces debug info with this form
-freorder-block-and-partition
clang produces this sort of thing with -fbasic-block-sections and with a
coming-soon tweak to use ranges in DWARFv5 where they can allow greater
reuse of debug_addr than the low/high_pc forms.

This fixes the case of breaking on a function name, but leaves broken
printing a variable - a follow-up commit will add that and improve the
test case to match.

Diff Detail

Event Timeline

dblaikie requested review of this revision.Jan 5 2021, 12:11 AM
dblaikie created this revision.
Herald added a project: Restricted Project. · View Herald Transcript
amharc added a subscriber: amharc.Jan 6 2021, 1:39 AM
labath added a comment.Jan 6 2021, 4:44 AM

The fix is good, but the test could be improved. Combining assembly input with running the inferior effectively limits the test to a single platform (assembly is not portable, and running requires asm to match the host). AFAICT, we don't actually need to run the binary to test this fix -- checking just the "breakpoint set" command should suffice (if you want to be more explicit in what is being checked, you can also run "breakpoint list -v" and test that). Then you'd just need to replace %clang_host with %clang -target x86_64-pc-linux (or something) and UNSUPPORTED: system-windows with REQUIRES: x86.

I'll write about variable printing in the other review.

dblaikie updated this revision to Diff 315035.Jan 6 2021, 6:13 PM

Update test to avoid running the program

The fix is good, but the test could be improved.

Yeah - haven't written lldb patches before so totally open to suggestions on the testing front for sure. Thanks!

Combining assembly input with running the inferior effectively limits the test to a single platform (assembly is not portable, and running requires asm to match the host).

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.

AFAICT, we don't actually need to run the binary to test this fix -- checking just the "breakpoint set" command should suffice (if you want to be more explicit in what is being checked, you can also run "breakpoint list -v" and test that). Then you'd just need to replace %clang_host with %clang -target x86_64-pc-linux (or something) and UNSUPPORTED: system-windows with REQUIRES: x86.

Yeah, I was hoping to setup the test for testing both these things in a uniform way, but if best practice is to test them with different mechanisms I'm OK with that.

I'll write about variable printing in the other review.

Thanks!

labath accepted this revision.Jan 7 2021, 12:52 PM

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. 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.

However, all of that is strictly optional.

This revision is now accepted and ready to land.Jan 7 2021, 12:52 PM
This revision was landed with ongoing or failed builds.Jan 7 2021, 2:28 PM
This revision was automatically updated to reflect the committed changes.

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?

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.

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?

Do you have python enabled in the build? (https://lldb.llvm.org/resources/build.html#preliminaries)

The cmake option LLDB_ENABLE_PYTHON defaults to Auto which has tripped me up in the past. If you set it to YES then you'll get a build error if it fails to find a suitable Python instead of silently disabling it.

Do you have python enabled in the build? (https://lldb.llvm.org/resources/build.html#preliminaries)

The cmake option LLDB_ENABLE_PYTHON defaults to Auto which has tripped me up in the past. If you set it to YES then you'll get a build error if it fails to find a suitable Python instead of silently disabling it.

Thanks for the hint! Yep, seems I didn't have the python3-dev package installed (took a while to stare at the error messages to guess/understand that that's what I was missing)

That got a few of these tests running, but still a significant chunk are "unsupported" due to (well, at least some are due to this - I'm guessing quite a few are due to this):

lldb version 12.0.0 (git@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']

But I do have libc++ and libc++abi listed as projects to build in my cmake config - but I guess because the just-built clang and just-built libc++ aren't installed, clang can't find libc++ so these tests fail/get marked as unsupported.

But how do these tests work for other people? Do they ever run in a plain build like this? It seems like they should/that would be important.

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@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)

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

(sorry for inactivity -- I mostly tuned out of this discussion as I
couldn't keep up with its pace)

jingham added a subscriber: labath.Jan 22 2021, 9:42 AM

If you are just loading an object file and the looking at the line table or something like that, would a UnitTest be more suitable?

Jim

lldb creates a single line table out of the debug_line input and the inlined_subroutine info from the debug_info. For instance, if you have a nested set of inlines that all have the same start address, they cannot be represented in the debug_line which requires monotonically increasing addresses. But when you are "stepping" into the set of nested inlines, lldb would like to represent the step as first stopping before the call in the outer function, then stepping one by one into the nested inlines. I don't know why exactly DWARF line tables need to be monotonically increasing in address, but we couldn't see any good reason why lldb should also have to do this little two-step every time we tried to figure out where you are, we decided to merge the info up front.

You could still write Unit Tests accessing the lldb line table construction rather than the DWARF debug_line ingestion. But IIRC the layer between the LineEntries in lldb_private and the classes in the SB API is pretty thin, so there's less motivation to test that way.

Jim