forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 339
[lldb] Infrastructure for highlighting function names in backtraces #10556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Michael137
wants to merge
27
commits into
swift/release/6.2
Choose a base branch
from
lldb/backtrace-highlighting-to-6.2
base: swift/release/6.2
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+2,289
−380
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…functions and use LLVM style (llvm#135031) I've always found this hard to read. Some upcoming changes make similar computations, so I thought it's a good time to factor out this logic into re-usable helpers and clean it up using LLVM's preferred early-return style. (cherry picked from commit f030f6f)
…me into helpers and use LLVM style (llvm#135331) Same cleanup as in llvm#135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication. (cherry picked from commit b656915)
…by reference (llvm#135536) Both the `CPlusPlusLanguage` plugins and the Swift language plugin already assume the `sc != nullptr`. And all `FormatEntity` callsites of `GetFunctionDisplayName` already check for nullptr before passing `sc`. This patch makes this pre-condition explicit by changing the parameter to `const SymbolContext &`. This will help with some upcoming changes in this area. (cherry picked from commit 52e45a7)
…vailable (llvm#135343) When a frame is inlined, LLDB will display its name in backtraces as follows: ``` * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.3 * frame #0: 0x0000000100000398 a.out`func() [inlined] baz(x=10) at inline.cpp:1:42 frame #1: 0x0000000100000398 a.out`func() [inlined] bar() at inline.cpp:2:37 frame #2: 0x0000000100000398 a.out`func() at inline.cpp:4:15 frame #3: 0x00000001000003c0 a.out`main at inline.cpp:7:5 frame #4: 0x000000026eb29ab8 dyld`start + 6812 ``` The longer the names get the more confusing this gets because the first function name that appears is the parent frame. My assumption (which may need some more surveying) is that for the majority of cases we only care about the actual frame name (not the parent). So this patch removes all the special logic that prints the parent frame. Another quirk of the current format is that the inlined frame name does not abide by the `${function.name-XXX}` format variables. We always just print the raw demangled name. With this patch, we would format the inlined frame name according to the `frame-format` setting (see the test-cases). If we really want to have the `parentFrame [inlined] inlinedFrame` format, we could expose it through a new `frame-format` variable (e..g., `${function.inlined-at-name}` and let the user decide where to place things. (cherry picked from commit 1e153b7)
…function (cherry picked from commit af7a7ba)
Adjust after llvm#135343 (cherry picked from commit a3f8359)
One can use `FormatStringRef` instead anyway (cherry picked from commit cbbf562)
@swift-ci test |
…tBuffer (llvm#133249) This patch includes the necessary changes for the LLDB feature proposed in https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309. The TL;DR is that we want to track where certain parts of a demangled name begin/end so we can highlight them in backtraces. We introduce a new `printLeft`/`printRight` API on `OutputBuffer` that a client (in our case LLDB) can implement to track state while printing the demangle tree. This requires redirecting all calls to to `printLeft`/`printRight` to the `OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so the definition of `Node` is available to them). Also introduces `notifyInsertion`/`notifyDeletion` APIs that a client can override to respond to cases where the `OutputBuffer` changes arbitrary parts of the name. (cherry picked from commit 889dad7)
…er parameters I noticed that there are test-cases that are commented out. But the manglings for them seem to be impossible to generate from valid C++. I added two test-cases generated from following C++ program: ``` struct X { int func() const && { return 5; } const int &&func2() { return 5; } const int &&func3(const int &x) volatile { return 5; } }; void f(int (X::*)() const &&, int const && (X::*)(), int const && (X::*)(const int &) volatile) {} int main() { f(&X::func, &X::func2, &X::func3); return 0; } ``` (cherry picked from commit 46f18b7)
…tion (llvm#131836) This patch implements a new `TrackingOutputBuffer` which tracks where the scope/basename/arguments begin and end in the demangled string. The idea that a function name can be decomposed into <scope, base, arguments>. The assumption is that given the ranges of those three elements and the demangled name, LLDB will be able to to reconstruct the full demangled name. The tracking of those ranges is pretty simple. We don’t ever deal with nesting, so whenever we recurse into a template argument list or another function type, we just stop tracking any positions. Once we recursed out of those, and are back to printing the top-level function name, we continue tracking the positions. We introduce a new structure `FunctionNameInfo` that holds all this information and is stored in the new `TrackingOutputBuffer` class. Tests are in `ItaniumDemangleTest.cpp`. llvm#131836 (cherry picked from commit 9c830ce)
52aa1e5
to
57c91de
Compare
@swift-ci test |
…#131836) Add version of GetDemangledName that will force re-demangling. This is required because LLDB will SetDemangledName without going through the demangler. So we need a way to force demangling to set the m_demangled_info member when we need it. llvm#131836 (cherry picked from commit f220ea2)
Uses the `TrackingOutputBuffer` to populate the new member `Mangled::m_demangled_info`. `m_demangled_info` is lazily popluated by `GetDemangledInfo`. To ensure `m_demangled` and `m_demangled_info` are in-sync we clear `m_demangled_info` anytime `m_demangled` is set/cleared. llvm#131836 (cherry picked from commit a267225)
llvm#131836) Adds new frame-format variables and implements them in the CPlusPlusLanguage plugin. We use the `DemangledNameInfo` type to retrieve the necessary part of the demangled name. llvm#131836 (cherry picked from commit 8b91b44)
…etting (llvm#131836) Adds the new `plugin.cplusplus.display.function-name-format` setting and makes the `${function.name-with-args}` query it for formatting the function name. One caveat is that the setting can't itself be set to `${function.name-with-args}` because that would cause infinite recursion and blow the stack. I added an XFAILed test-case for it and will address it in a follow-up patch. llvm#131836 (cherry picked from commit d555b9f)
This wasn't defined correctly and was unused. And it was causing a CI build failure: ``` /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/lldb/include/lldb/Core/DemangledNameInfo.h: In function ‘bool lldb_private::operator==(const lldb_private::DemangledNameInfo&, const lldb_private::DemangledNameInfo&)’: /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/lldb/include/lldb/Core/DemangledNameInfo.h:60:25: error: ‘const struct lldb_private::DemangledNameInfo’ has no member named ‘QualifiersRange’ 60 | lhs.QualifiersRange) == | ^~~~~~~~~~~~~~~ /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/lldb/include/lldb/Core/DemangledNameInfo.h:62:25: error: ‘const struct lldb_private::DemangledNameInfo’ has no member named ‘QualifiersRange’ 62 | lhs.QualifiersRange); | ^~~~~~~~~~~~~~~ 111.469 [1284/7/3896] Building CXX object tools/lldb/source/Expression/CMakeFiles/lldbExpression.dir/ObjectFileJIT.cpp.o 111.470 [1284/6/3897] Building CXX object tools/lldb/source/Expression/CMakeFiles/lldbExpression.dir/Materializer.cpp.o 111.492 [1284/5/3898] Building CXX object tools/lldb/source/Expression/CMakeFiles/lldbExpression.dir/REPL.cpp.o 111.496 [1284/4/3899] Building CXX object tools/lldb/source/Expression/CMakeFiles/lldbExpression.dir/UserExpression.cpp.o 111.695 [1284/3/3900] Building CXX object tools/lldb/source/Commands/CMakeFiles/lldbCommands.dir/CommandObjectTarget.cpp.o 112.944 [1284/2/3901] Linking CXX shared library lib/libclang.so.21.0.0git 113.098 [1284/1/3902] Linking CXX shared library lib/libclang-cpp.so.21.0git ``` (cherry picked from commit 5a64510)
No equality operator was specified. Define one locally. (cherry picked from commit da14f6d)
And remove now redunant friend declaration. For some reason this was failing to build on one of the MSVC bots after llvm#131836: ``` FAILED: tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ClangExpressionParser.cpp.obj ccache C:\PROGRA~1\MICROS~1\2022\COMMUN~1\VC\Tools\MSVC\1441~1.341\bin\Hostx64\x64\cl.exe /nologo /TP -DCLANG_BUILD_STATIC -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\source\Plugins\ExpressionParser\Clang -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\source\Plugins\ExpressionParser\Clang -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\include -IC:\Python312\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\..\clang\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\..\clang\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\source -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\source -D__OPTIMIZE__ /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2 -MD -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Fotools\lldb\source\Plugins\ExpressionParser\Clang\CMakeFiles\lldbPluginExpressionParserClang.dir\ClangExpressionParser.cpp.obj /Fdtools\lldb\source\Plugins\ExpressionParser\Clang\CMakeFiles\lldbPluginExpressionParserClang.dir\lldbPluginExpressionParserClang.pdb /FS -c C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\source\Plugins\ExpressionParser\Clang\ClangExpressionParser.cpp C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(667): error C2248: 'llvm::itanium_demangle::ObjCProtoName::Protocol': cannot access private member declared in class 'llvm::itanium_demangle::ObjCProtoName' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(615): note: see declaration of 'llvm::itanium_demangle::ObjCProtoName::Protocol' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\include\llvm/Demangle/ItaniumDemangle.h(613): note: see declaration of 'llvm::itanium_demangle::ObjCProtoName' ``` It's not quite clear to me why this wasn't compiling but either way this is cleaner. (cherry picked from commit bd96fa7)
(cherry picked from commit b571aa4)
It's a C++20 feature and is causing some buildbots to fail: ``` FAILED: tools/lldb/unittests/Core/CMakeFiles/LLDBCoreTests.dir/MangledTest.cpp.obj ccache C:\PROGRA~1\MICROS~1\2022\COMMUN~1\VC\Tools\MSVC\1443~1.348\bin\Hostx64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\unittests\Core -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\include -IC:\Python312\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\llvm\..\clang\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\build\tools\lldb\..\clang\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\source -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\third-party\unittest\googletest\include -IC:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\third-party\unittest\googlemock\include -D__OPTIMIZE__ /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2 -MD -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Fotools\lldb\unittests\Core\CMakeFiles\LLDBCoreTests.dir\MangledTest.cpp.obj /Fdtools\lldb\unittests\Core\CMakeFiles\LLDBCoreTests.dir\ /FS -c C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(416): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(415): error C7556: cannot mix designated-initializers with non-designated-initializers C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(418): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(423): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(422): error C7556: cannot mix designated-initializers with non-designated-initializers C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(424): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(429): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(428): error C7556: cannot mix designated-initializers with non-designated-initializers C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(431): error C7555: use of designated initializers requires at least '/std:c++20' C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\unittests\Core\MangledTest.cpp(436): error C7555: use of designated initializers requires at least '/std:c++20' ``` (cherry picked from commit 5137587)
This were failing on Windows CI with errors like: ``` 22: (lldb) bt 23: * thread #1, stop reason = breakpoint 1.1 24: frame #0: 0x00007ff7c5e41000 TestFrameFormatFunctionFormattedArgumentsObjC.test.tmp.objc.out`func at main.m:2 25: frame #1: 0x00007ff7c5e4101c TestFrameFormatFunctionFormattedArgumentsObjC.test.tmp.objc.out`bar + 12 at main.m:3 26: frame #2: 0x00007ff7c5e4103c TestFrameFormatFunctionFormattedArgumentsObjC.test.tmp.objc.out`main + 16 at main.m:5 27: custom-frame '()' !~~~~~~~~~~~ error: no match expected 28: custom-frame '(__formal=<unavailable>)' ``` (cherry picked from commit cc0bdb3)
All of these were failing on Windows CI. Some are failing because breakpoints on template functions can't be set by name. Others are failing because of slight textual differences. Most are failing because we can't track components of a mangled name from PDB, so XFAIL those. (cherry picked from commit e6f7e34)
These don't make sense for PDB on Windows (cherry picked from commit 7581aa1)
This was XPASSing on the linux-win remote CI jobs. Since we're now compiling with DWARF (not PDB), lets see if these pass on Windows again. (cherry picked from commit 8a5bc9e)
These are failing for various reasons on CI, most likely due to us requiring the Microsoft mangler. So XFAIL these. (cherry picked from commit 28293ea)
(cherry picked from commit db417e8)
57c91de
to
e4ad71d
Compare
@swift-ci test |
Fails on Windows CI: ``` | 10: (lldb) break set -l 3 | check:30'0 ~~~~~~~~~~~~~~~~~~~~~~ | 11: error: No selected frame to use to find the default file. | check:30'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 12: error: No file supplied and no default file available. | check:30'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 13: (lldb) exit ``` This passes fine when compiling on Windows for Linux targets.
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.