[lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

[Michael137]

Summary

Summary

The primary motivation for this patch is to make sure we handle
the step-in behaviour for functions in the std namespace which
have an auto return type. Currently the default step-avoid-regex
setting is ^std:: but LLDB will still step into template functions
with auto return types in the std namespace.

Details
When we hit a breakpoint and check whether we should stop, we call
into ThreadPlanStepInRange::FrameMatchesAvoidCriteria. We then ask
for the frame function name via SymbolContext::GetFunctionName(Mangled::ePreferDemangledWithoutArguments).
This ends up trying to parse the function name using CPlusPlusLanguage::MethodName::GetBasename which
parses the raw demangled name string.

CPlusPlusNameParser::ParseFunctionImpl calls ConsumeTypename to skip
the (in our case auto) return type of the demangled name (according to the
Itanium ABI this is a valid thing to encode into the mangled name). However,
ConsumeTypename doesn't strip out a plain auto identifier
(it will strip a `decltype(auto) return type though). So we are now left with
a basename that still has the return type in it, thus failing to match the ^std::
regex.

Example frame where the return type is still part of the function name:

Process 1234 stopped
* thread #1, stop reason = step in
    frame #0: 0x12345678 repro`auto std::test_return_auto<int>() at main.cpp:12:5
   9
   10   template <class>
   11   auto test_return_auto() {
-> 12       return 42;
   13   }

This is another case where the CPlusPlusNameParser breaks us in subtle ways
due to evolving C++ syntax. There are longer-term plans of replacing the hand-rolled
C++ parser with an alternative that uses the mangle tree API to do the parsing for us.

Testing

• Added API and unit-tests
• Adding support for ABI tags into the parser is a larger undertaking which we would rather solve properly by using libcxxabi's mangle tree parser

[Michael137]

lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

68

git clang-format

[labath]

There are longer-term plans of replacing the hand-rolled C++ parser with an alternative that uses the mangle tree API to do the parsing for us.

You may be aware of this, but I feel I should mention that there are cases when a function just does not have a mangled name, either because it is in an extern "C" block, or because it was complied with --dwarf-linkage-names=Abstract (default for -gsce). In this case, we construct a fake demangled name from the DWARF debug info (the names of enclosing (DW_TAG_)namespaces, and the types for (DW_TAG_)formal_parameters. Of course, in this case, it makes even less sense to parse the resulting string, since we're the ones who constructed it in the first place. However, it may not be sufficient to assume that one can just start with a mangled name, and get everything out that way.

[Michael137]

• Reword commit message
• Remove redundant header from test

[Michael137]

In D135413#3846390, @labath wrote:

There are longer-term plans of replacing the hand-rolled C++ parser with an alternative that uses the mangle tree API to do the parsing for us.

You may be aware of this, but I feel I should mention that there are cases when a function just does not have a mangled name, either because it is in an extern "C" block, or because it was complied with --dwarf-linkage-names=Abstract (default for -gsce). In this case, we construct a fake demangled name from the DWARF debug info (the names of enclosing (DW_TAG_)namespaces, and the types for (DW_TAG_)formal_parameters. Of course, in this case, it makes even less sense to parse the resulting string, since we're the ones who constructed it in the first place. However, it may not be sufficient to assume that one can just start with a mangled name, and get everything out that way.

Good points, thanks! Didn't know about the --dwarf-linkage-names=Abstract option

[stella.stamenova]

Looks like this change broke the windows lldb bot: https://lab.llvm.org/buildbot/#/builders/83/builds/24631

[Michael137]

In D135413#3846954, @stella.stamenova wrote:

Looks like this change broke the windows lldb bot: https://lab.llvm.org/buildbot/#/builders/83/builds/24631

I thought I had skipped it on Windows. Will fix now

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

 //===-- CPlusPlusNameParser.cpp -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CPlusPlusNameParser.h"
 
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Threading.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using llvm::Optional;
 using llvm::None;
 using ParsedFunction = lldb_private::CPlusPlusNameParser::ParsedFunction;
clang::Token &CPlusPlusNameParser::Peek() {
   return m_tokens[m_next_token_index];
 }
 
 Optional<ParsedFunction>
 CPlusPlusNameParser::ParseFunctionImpl(bool expect_return_type) {
   Bookmark start_position = SetBookmark();
   if (expect_return_type) {
     // Consume return type if it's expected.
-    if (!ConsumeTypename())
+    if (!ConsumeToken(tok::kw_auto) && !ConsumeTypename())
       return None;
   }
 
   auto maybe_name = ParseFullNameImpl();
   if (!maybe_name) {
     return None;
   }
 

lldb/test/API/functionalities/step-avoids-regexp/Makefile

+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++20
+
+include Makefile.rules

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

+"""
+Test thread step-in ignores frames according to the "Avoid regexp" option.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepAvoidsRegexTestCase(TestBase):
+    def hit_correct_function(self, pattern):
+        name = self.thread.frames[0].GetFunctionName()
+        self.assertTrue(
+            pattern in name, "Got to '%s' not the expected function '%s'." %
+            (name, pattern))
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.dbg.HandleCommand(
+                "settings set target.process.thread.step-avoid-regexp ^ignore::")
+
+    def test_step_avoid_regex(self):
+        """Tests stepping into a function which matches the avoid regex"""
+        self.build()
+        (_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "main", lldb.SBFileSpec('main.cpp'))
+
+        # Try to step into ignore::auto_ret
+        self.thread.StepInto()
+        self.hit_correct_function("main")
+
+        # 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")
+
+    @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")

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

+#include <cstdio>
+
+namespace ignore {
+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
+
+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>();
+}

lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

TestCase test_cases[] = {
        "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
        "SAEC::checkFunction(context&) const::CallBack::CallBack"},
       // Anonymous namespace
       {"XX::(anonymous namespace)::anon_class::anon_func() const",
        "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
        "XX::(anonymous namespace)::anon_class::anon_func"},
 
       // Lambda
-      {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() const",
-       "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()", "()", "const",
+      {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() "
+       "const",
+       "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()",
+       "()", "const",
        "main::{lambda()#1}::operator()() const::{lambda()#1}::operator()"},
 
       // Function pointers
       {"string (*f(vector<int>&&))(float)", "", "f", "(vector<int>&&)", "",
        "f"},
       {"void (*&std::_Any_data::_M_access<void (*)()>())()", "std::_Any_data",
        "_M_access<void (*)()>", "()", "",
        "std::_Any_data::_M_access<void (*)()>"},
TestCase test_cases[] = {
        "llvm::countTrailingOnes<unsigned int>"},
       {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
        "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*"}};
+       "llvm::Optional<llvm::MCFixupKind>", "operator*", "()",
+       "const volatile &&", "llvm::Optional<llvm::MCFixupKind>::operator*"},
+
+      // 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>"}};
 
   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());