[lldb][CPlusPlus] Add abi_tag support to the CPlusPlusNameParser

[Michael137]

Summary

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

[Michael137]

Would really be nice not to have to do this. Especially because it looks like only the LLVM demangler prints out the ABI tags (though other vendors may start doing the same at some point)

This is partly educational to see what we'd have to do to support demangled names with abi-tags

[Michael137]

Wonder if we should skip the tags rather than consume them. Depends on what DW_AT_name says. Also I suspect there are cases where we would want to include tags and others where we dont

Will need to handle special characters in the tags too (currently it just handles simple identifiers)

[Michael137]

• Support certain special tokens

[Michael137]

• Add test case

[aprantl]

As much as I dislike expanding our hand-rolled parser, this does seem small and important enough to warrant doing it!

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp

260

nit: the function is inconsistent about whether to use curly braces on single statements. LLVM style says don't.

[Michael137]

• Docs fixes
• Add more test cases
• Handle operator overloads

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h

private:
     size_t m_position_value;
     bool m_restore = true;
   };
 
   bool HasMoreTokens();
   void Advance();
   void TakeBack();
   bool ConsumeToken(clang::tok::TokenKind kind);
+
   template <typename... Ts> bool ConsumeToken(Ts... kinds);
   Bookmark SetBookmark();
   size_t GetCurrentPosition();
   clang::Token &Peek();
   bool ConsumeBrackets(clang::tok::TokenKind left, clang::tok::TokenKind right);
 
   llvm::Optional<ParsedFunction> ParseFunctionImpl(bool expect_return_type);
 
private:
   void SkipPtrsAndRefs();
 
   // Consumes things like 'const * const &'
   bool ConsumePtrsAndRefs();
 
   // 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);
 
   // Populate m_tokens by calling clang lexer on m_text.
   void ExtractTokens();
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_CPLUSPLUSNAMEPARSER_H

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp

case tok::kw_operator: // C++ operator overloading.
         return false;
       can_open_template = true;
       break;
     case tok::raw_identifier:
       can_open_template = true;
       Advance();
       break;
     case tok::l_square:
-      if (!ConsumeBrackets(tok::l_square, tok::r_square))
-        return false;
+      // 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;
       break;
     case tok::l_paren:
       if (!ConsumeArguments())
         return false;
       can_open_template = false;
       break;
     default:
       can_open_template = false;
       Advance();
       break;
     }
   }
 
   if (template_counter != 0) {
     return false;
   }
   start_position.Remove();
   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)) {
     return false;
   }
   constexpr llvm::StringLiteral g_anonymous("anonymous");
   if (HasMoreTokens() && Peek().is(tok::raw_identifier) &&
       Peek().getRawIdentifier() == g_anonymous) {
while (continue_parsing && HasMoreTokens()) {
     case tok::raw_identifier: // Just a name.
       if (state != State::Beginning && state != State::AfterTwoColons) {
         continue_parsing = false;
         break;
       }
       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)
         if (ConsumeAnonymousNamespace()) {
           state = State::AfterIdentifier;
           break;
         }
       }

lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py

def test_step_avoid_regex(self):
         # Try to step into ignore::with_tag
         self.thread.StepInto()
         self.hit_correct_function("main")
 
         # Try to step into ignore::decltype_auto_ret
         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")

lldb/test/API/functionalities/step-avoids-regexp/main.cpp

 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() {
   return 0;
 }
 
 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);
 }

lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

TestCase test_cases[] = {
        "long)",
        "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
       {"f<A<operator<(X,Y)::Subclass>, sizeof(B)<sizeof(C)>()", "",
        "f<A<operator<(X,Y)::Subclass>, sizeof(B)<sizeof(C)>", "()", "",
        "f<A<operator<(X,Y)::Subclass>, sizeof(B)<sizeof(C)>"},
       {"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));
     EXPECT_TRUE(method.IsValid()) << test.input;
     if (method.IsValid()) {
       EXPECT_EQ(test.context, method.GetContext().str());
       EXPECT_EQ(test.basename, method.GetBasename().str());
       EXPECT_EQ(test.arguments, method.GetArguments().str());
       EXPECT_EQ(test.qualifiers, method.GetQualifiers().str());
       EXPECT_EQ(test.scope_qualified_name, method.GetScopeQualifiedName());
     }
   }
 }
 
+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)"));
   CPlusPlusLanguage::MethodName
       reference_2(ConstString("int foofoo::bar::func01(std::string a, int b)"));
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
       reference_4(ConstString("bar::baz::operator bool()"));