This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Add support for DW_AT_default_value in template params
ClosedPublic

Authored by Michael137 on Jan 16 2023, 1:30 AM.

Details

Summary

Summary

This patch makes LLDB understand the DW_AT_default_value on
template argument DIEs. As a result, type summaries will no
longer contain the defaulted template arguments, reducing
noise substantially. E.g.,

Before:

(lldb) v nested
(std::vector<std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator <char> > > > > >) nested = size=0 {}

After:

(lldb) v nested
(std::vector<std::vector<std::basic_string<char> > >) nested = size=0 {}

See discussion in https://reviews.llvm.org/D140423

Testing

  • Adjusted API tests
  • Added DWARFASTParserClang unit test

Diff Detail

Event Timeline

Michael137 created this revision.Jan 16 2023, 1:30 AM
Herald added a project: Restricted Project. · View Herald Transcript
Michael137 requested review of this revision.Jan 16 2023, 1:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2023, 1:30 AM
Michael137 edited the summary of this revision. (Show Details)Jan 16 2023, 1:45 AM
  • Add tests
Michael137 retitled this revision from [WIP][lldb] Add support for DW_AT_default_value in template params to [lldb] Add support for DW_AT_default_value in template params.Jan 26 2023, 7:39 AM
  • Use constructor instead of setter
aprantl added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
2038

nit: LLVM coding style doesn't use {} on single-statement conditionals.

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
63

Nice!

lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
33

Does this work on Linux?

421

I don't know if this API is a good match here, but just in case you didn't know, the canonical LLVM way of reading from files is to use MemoryBuffer https://llvm.org/doxygen/classllvm_1_1MemoryBuffer.html#aa98611beefe78f907beeee7305cc8174

  • Use GetInputFilePath and MemoryBuffer to read test YAML file
  • Remove redundant test-fixture initialization
lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
33

Didn't actually need these

Michael137 marked 2 inline comments as done.Jan 26 2023, 11:45 AM
Michael137 marked an inline comment as done.
  • clang-format
Michael137 edited the summary of this revision. (Show Details)Jan 26 2023, 11:49 AM
  • Remove dead code
aprantl accepted this revision.Jan 26 2023, 6:16 PM

LGTM! (I think you some superfluous #includes now)

lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
19

I think this is no longer needed?

This revision is now accepted and ready to land.Jan 26 2023, 6:16 PM
  • Remove redundant includes
This revision was landed with ongoing or failed builds.Jan 26 2023, 6:35 PM
This revision was automatically updated to reflect the committed changes.

I've reverted this due to test failures on Arm and AArch64. They were obscured by the build failure so once you'd fixed that the bot was silent about it.

Here's one of the builds: https://lab.llvm.org/buildbot/#/builders/96/builds/34718

SymbolFileDWARFTests: ../llvm-project/lldb/source/Host/linux/HostInfoLinux.cpp:47: static void lldb_private::HostInfoLinux::Terminate(): Assertion `g_fields && "Missing call to Initialize?"' failed.
Michael137 added a comment.EditedJan 27 2023, 3:58 AM

I've reverted this due to test failures on Arm and AArch64. They were obscured by the build failure so once you'd fixed that the bot was silent about it.

Here's one of the builds: https://lab.llvm.org/buildbot/#/builders/96/builds/34718

SymbolFileDWARFTests: ../llvm-project/lldb/source/Host/linux/HostInfoLinux.cpp:47: static void lldb_private::HostInfoLinux::Terminate(): Assertion `g_fields && "Missing call to Initialize?"' failed.

Thanks for letting me know. The Darwin bots were also failing (but in a different test https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/)

Looking

It appears that they passed on Windows, could be a Linux only issue.

I've reverted this due to test failures on Arm and AArch64. They were obscured by the build failure so once you'd fixed that the bot was silent about it.

Here's one of the builds: https://lab.llvm.org/buildbot/#/builders/96/builds/34718

SymbolFileDWARFTests: ../llvm-project/lldb/source/Host/linux/HostInfoLinux.cpp:47: static void lldb_private::HostInfoLinux::Terminate(): Assertion `g_fields && "Missing call to Initialize?"' failed.

Thanks for letting me know. The Darwin bots were also failing (but in a different test https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/)

Looking

The matrix bot failures are due to https://reviews.llvm.org/D141827. Fix is in-progress

Unit-test failures are odd since I do call HostInfo::Initialize in the test fixture setup. Investigating..

Michael137 reopened this revision.Jan 27 2023, 6:23 AM
This revision is now accepted and ready to land.Jan 27 2023, 6:23 AM
  • Fix import-std-module API tests

API tests should all work once following lands:

Just the Linux unit-test issue left now

  • Remove redundant HostInfo calls