[Lldb-commits] [lldb] 3cc9884 - [lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 21 06:01:13 PDT 2022
Author: Michael Buch
Date: 2022-10-21T14:00:50+01:00
New Revision: 3cc9884500ad53e878045bc1d119d8a6b326f274
URL: https://github.com/llvm/llvm-project/commit/3cc9884500ad53e878045bc1d119d8a6b326f274
DIFF: https://github.com/llvm/llvm-project/commit/3cc9884500ad53e878045bc1d119d8a6b326f274.diff
LOG: [lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser
This patch teaches the `CPlusPlusNameParser` to parse the
demangled/prettified [[gnu::abi_tag(...)]] attribute. The demangled format
isn't standardized and the additions to the parser were mainly driven
using Clang (and the limited information on this from the official
Clang docs).
This change is motivated by multiple failures around step-in
behaviour for libcxx APIs (many of which have ABI tags as of recently).
LLDB determines whether the `step-avoid-regexp` matches the current
frame by parsing the scope-qualified name out of the demangled
function symbol. On failure, the `CPlusPlusNameParser` will simply
return the fully demangled name (which can include the return type)
to the caller, which in `FrameMatchesAvoidCriteria` means we will
not correctly decide whether we should stop at a frame or not if
the function has an abi_tag.
Ideally we wouldn't want to rely on the non-standard format
of demangled attributes. Alternatives would be:
1. Use the mangle tree API to do the parsing for us
2. Reconstruct the scope-qualified name from DWARF instead of parsing
the demangled name
(1) isn't feasible without a significant refactor of `lldb_private::Mangled`,
if we want to do this efficiently.
(2) could be feasible in cases where debug-info for a frame is
available. But it does mean we certain operations (such as step-in regexp,
and frame function names) won't work with/won't show ABI tags.
**Testing**
* Un-XFAILed step-in API test
* Added parser unit-tests
Differential Revision: https://reviews.llvm.org/D136306
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
lldb/test/API/functionalities/step-avoids-regexp/main.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
index ac70226ca2a4d..427e5190846d0 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -226,9 +226,15 @@ bool CPlusPlusNameParser::ConsumeTemplateArgs() {
Advance();
break;
case tok::l_square:
- if (!ConsumeBrackets(tok::l_square, tok::r_square))
+ // Handle templates tagged with an ABI tag.
+ // An example demangled/prettified version is:
+ // func[abi:tag1][abi:tag2]<type[abi:tag3]>(int)
+ if (ConsumeAbiTag())
+ can_open_template = true;
+ else if (ConsumeBrackets(tok::l_square, tok::r_square))
+ can_open_template = false;
+ else
return false;
- can_open_template = false;
break;
case tok::l_paren:
if (!ConsumeArguments())
@@ -249,6 +255,32 @@ bool CPlusPlusNameParser::ConsumeTemplateArgs() {
return true;
}
+bool CPlusPlusNameParser::ConsumeAbiTag() {
+ Bookmark start_position = SetBookmark();
+ if (!ConsumeToken(tok::l_square))
+ return false;
+
+ if (HasMoreTokens() && Peek().is(tok::raw_identifier) &&
+ Peek().getRawIdentifier() == "abi")
+ Advance();
+ else
+ return false;
+
+ if (!ConsumeToken(tok::colon))
+ return false;
+
+ // Consume the actual tag string (and allow some special characters)
+ while (ConsumeToken(tok::raw_identifier, tok::comma, tok::period,
+ tok::numeric_constant))
+ ;
+
+ if (!ConsumeToken(tok::r_square))
+ return false;
+
+ start_position.Remove();
+ return true;
+}
+
bool CPlusPlusNameParser::ConsumeAnonymousNamespace() {
Bookmark start_position = SetBookmark();
if (!ConsumeToken(tok::l_paren)) {
@@ -519,6 +551,22 @@ CPlusPlusNameParser::ParseFullNameImpl() {
Advance();
state = State::AfterIdentifier;
break;
+ case tok::l_square: {
+ // Handles types or functions that were tagged
+ // with, e.g.,
+ // [[gnu::abi_tag("tag1","tag2")]] func()
+ // and demangled/prettified into:
+ // func[abi:tag1][abi:tag2]()
+
+ // ABI tags only appear after a method or type name
+ const bool valid_state =
+ state == State::AfterIdentifier || state == State::AfterOperator;
+ if (!valid_state || !ConsumeAbiTag()) {
+ continue_parsing = false;
+ }
+
+ break;
+ }
case tok::l_paren: {
if (state == State::Beginning || state == State::AfterTwoColons) {
// (anonymous namespace)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
index 426434c486088..9a85934e8d670 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
@@ -117,6 +117,7 @@ class CPlusPlusNameParser {
void Advance();
void TakeBack();
bool ConsumeToken(clang::tok::TokenKind kind);
+
template <typename... Ts> bool ConsumeToken(Ts... kinds);
Bookmark SetBookmark();
size_t GetCurrentPosition();
@@ -164,6 +165,16 @@ class CPlusPlusNameParser {
// Consumes full type name like 'Namespace::Class<int>::Method()::InnerClass'
bool ConsumeTypename();
+ /// Consumes ABI tags enclosed within '[abi:' ... ']'
+ ///
+ /// Since there is no restriction on what the ABI tag
+ /// string may contain, this API supports parsing a small
+ /// set of special characters.
+ ///
+ /// The following regex describes the set of supported characters:
+ /// [A-Za-z,.\s\d]+
+ bool ConsumeAbiTag();
+
llvm::Optional<ParsedNameRanges> ParseFullNameImpl();
llvm::StringRef GetTextForRange(const Range &range);
diff --git a/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
index ffca513886f05..50dcea16ac0fe 100644
--- a/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
+++ b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -38,14 +38,11 @@ def test_step_avoid_regex(self):
self.thread.StepInto()
self.hit_correct_function("main")
- @skipIfWindows
- @skipIf(compiler="clang", compiler_version=['<', '11.0'])
- @expectedFailureAll(bugnumber="rdar://100645742")
- def test_step_avoid_regex_abi_tagged_template(self):
- """Tests stepping into an ABI tagged function that matches the avoid regex"""
- self.build()
- (_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "with_tag_template", lldb.SBFileSpec('main.cpp'))
-
# Try to step into ignore::with_tag_template
self.thread.StepInto()
self.hit_correct_function("main")
+
+ # Step into with_tag_template_returns_ignore which is outside the 'ignore::'
+ # namespace but returns a type from 'ignore::'
+ self.thread.StepInto()
+ self.hit_correct_function("with_tag_template_returns_ignore")
diff --git a/lldb/test/API/functionalities/step-avoids-regexp/main.cpp b/lldb/test/API/functionalities/step-avoids-regexp/main.cpp
index 20671188bd58e..578f05fcc5189 100644
--- a/lldb/test/API/functionalities/step-avoids-regexp/main.cpp
+++ b/lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -1,4 +1,6 @@
namespace ignore {
+struct Dummy {};
+
template <typename T> auto auto_ret(T x) { return 0; }
[[gnu::abi_tag("test")]] int with_tag() { return 0; }
template <typename T> [[gnu::abi_tag("test")]] int with_tag_template() {
@@ -8,9 +10,15 @@ template <typename T> [[gnu::abi_tag("test")]] int with_tag_template() {
template <typename T> decltype(auto) decltype_auto_ret(T x) { return 0; }
} // namespace ignore
+template <typename T>
+[[gnu::abi_tag("test")]] ignore::Dummy with_tag_template_returns_ignore(T x) {
+ return {};
+}
+
int main() {
auto v1 = ignore::auto_ret<int>(5);
auto v2 = ignore::with_tag();
auto v3 = ignore::decltype_auto_ret<int>(5);
auto v4 = ignore::with_tag_template<int>();
+ auto v5 = with_tag_template_returns_ignore<int>(5);
}
diff --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
index 85cf8081e4270..e84378259072c 100644
--- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -114,13 +114,74 @@ TEST(CPlusPlusLanguage, MethodNameParsing) {
{"llvm::Optional<llvm::MCFixupKind>::operator*() const volatile &&",
"llvm::Optional<llvm::MCFixupKind>", "operator*", "()",
"const volatile &&", "llvm::Optional<llvm::MCFixupKind>::operator*"},
+ {"void foo<Dummy<char [10]>>()", "", "foo<Dummy<char [10]>>", "()", "",
+ "foo<Dummy<char [10]>>"},
// auto return type
{"auto std::test_return_auto<int>() const", "std",
"test_return_auto<int>", "()", "const", "std::test_return_auto<int>"},
{"decltype(auto) std::test_return_auto<int>(int) const", "std",
- "test_return_auto<int>", "(int)", "const",
- "std::test_return_auto<int>"}};
+ "test_return_auto<int>", "(int)", "const", "std::test_return_auto<int>"},
+
+ // abi_tag on class method
+ {"v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<int>> "
+ "v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<int>>"
+ "::method2<v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<"
+ "int>>>(int, v1::v2::Dummy<int>) const &&",
+ // Context
+ "v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<int>>",
+ // Basename
+ "method2<v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<"
+ "int>>>",
+ // Args, qualifiers
+ "(int, v1::v2::Dummy<int>)", "const &&",
+ // Full scope-qualified name without args
+ "v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<int>>"
+ "::method2<v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<"
+ "int>>>"},
+
+ // abi_tag on free function and template argument
+ {"v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::Dummy[abi:c1][abi:c2]<int>> "
+ "v1::v2::with_tag_in_ns[abi:f1][abi:f2]<v1::v2::Dummy[abi:c1][abi:c2]"
+ "<v1::v2::Dummy[abi:c1][abi:c2]<int>>>(int, v1::v2::Dummy<int>) const "
+ "&&",
+ // Context
+ "v1::v2",
+ // Basename
+ "with_tag_in_ns[abi:f1][abi:f2]<v1::v2::Dummy[abi:c1][abi:c2]<v1::v2::"
+ "Dummy[abi:c1][abi:c2]<int>>>",
+ // Args, qualifiers
+ "(int, v1::v2::Dummy<int>)", "const &&",
+ // Full scope-qualified name without args
+ "v1::v2::with_tag_in_ns[abi:f1][abi:f2]<v1::v2::Dummy[abi:c1][abi:c2]<"
+ "v1::v2::Dummy[abi:c1][abi:c2]<int>>>"},
+
+ // abi_tag with special characters
+ {"auto ns::with_tag_in_ns[abi:special tag,0.0][abi:special "
+ "tag,1.0]<Dummy<int>>"
+ "(float) const &&",
+ // Context
+ "ns",
+ // Basename
+ "with_tag_in_ns[abi:special tag,0.0][abi:special tag,1.0]<Dummy<int>>",
+ // Args, qualifiers
+ "(float)", "const &&",
+ // Full scope-qualified name without args
+ "ns::with_tag_in_ns[abi:special tag,0.0][abi:special "
+ "tag,1.0]<Dummy<int>>"},
+
+ // abi_tag on operator overloads
+ {"std::__1::error_code::operator bool[abi:v160000]() const",
+ "std::__1::error_code", "operator bool[abi:v160000]", "()", "const",
+ "std::__1::error_code::operator bool[abi:v160000]"},
+
+ {"auto ns::foo::operator[][abi:v160000](size_t) const", "ns::foo",
+ "operator[][abi:v160000]", "(size_t)", "const",
+ "ns::foo::operator[][abi:v160000]"},
+
+ {"auto Foo[abi:abc]<int>::operator<<<Foo[abi:abc]<int>>(int) &",
+ "Foo[abi:abc]<int>", "operator<<<Foo[abi:abc]<int>>", "(int)", "&",
+ "Foo[abi:abc]<int>::operator<<<Foo[abi:abc]<int>>"}};
for (const auto &test : test_cases) {
CPlusPlusLanguage::MethodName method(ConstString(test.input));
@@ -135,6 +196,30 @@ TEST(CPlusPlusLanguage, MethodNameParsing) {
}
}
+TEST(CPlusPlusLanguage, InvalidMethodNameParsing) {
+ // Tests that we correctly reject malformed function names
+
+ std::string test_cases[] = {
+ "int Foo::operator[]<[10>()",
+ "Foo::operator bool[10]()",
+ "auto A::operator<=>[abi:tag]<A::B>()",
+ "auto A::operator<<<(int)",
+ "auto A::operator>>>(int)",
+ "auto A::operator<<<Type[abi:tag]<>(int)",
+ "auto A::operator<<<Type[abi:tag]<Type<int>>(int)",
+ "auto A::foo[(int)",
+ "auto A::foo[](int)",
+ "auto A::foo[bar](int)",
+ "auto A::foo[abi](int)",
+ "auto A::foo[abi:(int)",
+ };
+
+ for (const auto &name : test_cases) {
+ CPlusPlusLanguage::MethodName method{ConstString(name)};
+ EXPECT_FALSE(method.IsValid()) << name;
+ }
+}
+
TEST(CPlusPlusLanguage, ContainsPath) {
CPlusPlusLanguage::MethodName
reference_1(ConstString("int foo::bar::func01(int a, double b)"));
More information about the lldb-commits
mailing list