Michael137 created this revision.
Michael137 added reviewers: aprantl, labath, jingham.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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

**Testing**

- Un-XFAILed step-in API test
- Added parser unit-tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136306

Files:
  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

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -120,7 +120,35 @@
        "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>"}};
+       "std::test_return_auto<int>"},
+  
+      // abi_tag on class
+      {"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>>>"}
+  };
 
   for (const auto &test : test_cases) {
     CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===================================================================
--- lldb/test/API/functionalities/step-avoids-regexp/main.cpp
+++ 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> 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);
 }
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===================================================================
--- lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -37,13 +37,10 @@
         self.thread.StepInto()
         self.hit_correct_function("main")
 
-    @skipIfWindows
-    @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 (outside 'ignore::' namespace)
+        self.thread.StepInto()
+        self.hit_correct_function("with_tag_template_returns_ignore")
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
@@ -117,6 +117,9 @@
   void Advance();
   void TakeBack();
   bool ConsumeToken(clang::tok::TokenKind kind);
+
+  /// Consumes ABI tags enclosed within '[abi:' ... ']'
+  bool ConsumeAbiTag();
   template <typename... Ts> bool ConsumeToken(Ts... kinds);
   Bookmark SetBookmark();
   size_t GetCurrentPosition();
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -226,9 +226,15 @@
       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,34 @@
   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;
+
+  if (HasMoreTokens() && Peek().is(tok::raw_identifier))
+    Advance();
+  else
+    return false;
+
+  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 +553,19 @@
       Advance();
       state = State::AfterIdentifier;
       break;
+    case tok::l_square: // ABI tag
+      // 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]()
+      if (state == State::AfterIdentifier) {
+        // Anything other than an ABI tag is invalid at this point.
+        if (!ConsumeAbiTag()) {
+          continue_parsing = false;
+        }
+      }
+      break;
     case tok::l_paren: {
       if (state == State::Beginning || state == State::AfterTwoColons) {
         // (anonymous namespace)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to