[libcxxabi][Demangle] Don't drop ctor/dtor name for abi-tagged structures

[Michael137]

Summary

Before this patch we would demangle abi-tagged structures as follows:

$ c++filt -n _ZN1SB5OuterC2Ev
S[abi:Outer]:()

$ c++filt -n _ZN1SB5OuterD2Ev
S[abi:Outer]::~()

This is because Node::getBaseName was unimplemented for the
AbiTagAttr node, which meant that when we tried printing CtorDtorName
where its Basename Node was an AbiTagAttr, we'd drop the
name.

Addresses https://github.com/llvm/llvm-project/issues/61213

[Michael137]

• Update commit message

[jhenderson]

Your test is failing the pre-merge checks.

llvm/test/tools/llvm-cxxfilt/abitag.test

1

This test is testing llvm-cxxfilt, but llvm-cxxfilt doesn't use the libcxxabi code. It uses the demangler code in libDemangle, so it's not surprising it fails.

[Michael137]

In D145492#4177222, @jhenderson wrote:

Your test is failing the pre-merge checks.

Yup, I split the llvm changes to here: https://reviews.llvm.org/D145493

Want me to merge the two commits into a single revision/commit? Or move the test to the LLVM diff?

[ldionne]

Thanks for the catch, this makes sense to me!

In D145492#4177344, @Michael137 wrote:

Want me to merge the two commits into a single revision/commit? Or move the test to the LLVM diff?

FWIW, no strong opinion here but I think it would make sense to apply the changes to both directories in the same patch. You may want to sync it once before your patch if it's out-of-date right now as a NFC, just so that this patch stays consistent.

llvm/test/tools/llvm-cxxfilt/abitag.test

7

Would it make sense to add a test to libcxxabi/test/test_demangle.pass.cpp as well? I think that's where those are usually added.

[Michael137]

llvm/test/tools/llvm-cxxfilt/abitag.test

7

Good idea, will do

[Michael137]

• Sync changes to llvm
• Add test case to libcxx test-suite

[jhenderson]

Approving to clear my objection, but probably this could do with a pair of eyes more familiar with ABI tagging (or at least the mangling parts in relation to it).

llvm/include/llvm/Demangle/ItaniumDemangle.h

35–41

Are these changes unrelated and simply because the two haven't been synced properly? If so, I'd do an NFC patch before this one to sync up the changes first.

[Michael137]

llvm/include/llvm/Demangle/ItaniumDemangle.h

35–41

Makes sense, done

libcxxabi/src/demangle/ItaniumDemangle.h

struct AbiTagAttr : Node {
 
   AbiTagAttr(Node* Base_, StringView Tag_)
       : Node(KAbiTagAttr, Base_->RHSComponentCache,
              Base_->ArrayCache, Base_->FunctionCache),
         Base(Base_), Tag(Tag_) {}
 
   template<typename Fn> void match(Fn F) const { F(Base, Tag); }
 
+  StringView getBaseName() const override { return Base->getBaseName(); }
+
   void printLeft(OutputBuffer &OB) const override {
     Base->printLeft(OB);
     OB += "[abi:";
     OB += Tag;
     OB += "]";
   }
 };
 

libcxxabi/test/test_demangle.pass.cpp

//    {"_Z1fM1XRFivEMS_OFivEMS_KOFivE", "f(int (X::*)() &, int (X::*)() &&, int (X::*)() const &&)"},
     {"_ZNSdC1Ev", "std::basic_iostream<char, std::char_traits<char>>"
      "::basic_iostream()"},
     {"_ZNSiC1Ev", "std::basic_istream<char, std::char_traits<char>>"
      "::basic_istream()"},
     {"_ZNSoC1Ev", "std::basic_ostream<char, std::char_traits<char>>"
      "::basic_ostream()"},
     {"_ZNSsC1Ev", "std::basic_string<char, std::char_traits<char>,"
      " std::allocator<char>>::basic_string()"},
+    {"_ZN1SB8ctor_tagC2Ev", "S[abi:ctor_tag]::S()"},
+    {"_ZN1SB8ctor_tagD2Ev", "S[abi:ctor_tag]::~S()"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);
 
 struct FPLiteralCase {
   const char* mangled;
   // There are four possible demanglings of a given float.
   std::string expecting[4];

llvm/include/llvm/Demangle/ItaniumDemangle.h

 #include <cstring>
 #include <limits>
 #include <new>
 #include <utility>
 
 DEMANGLE_NAMESPACE_BEGIN
 
 template <class T, size_t N> class PODSmallVector {
   static_assert(std::is_pod<T>::value,
                 "T is required to be a plain old data type");
 
   T *First = nullptr;
   T *Last = nullptr;
   T *Cap = nullptr;
   T Inline[N] = {0};
 
   bool isInline() const { return First == Inline; }
 
   void clearInline() {
     First = Inline;
     Last = Inline;
     Cap = Inline + N;
   }
struct AbiTagAttr : Node {
 
   AbiTagAttr(Node* Base_, StringView Tag_)
       : Node(KAbiTagAttr, Base_->RHSComponentCache,
              Base_->ArrayCache, Base_->FunctionCache),
         Base(Base_), Tag(Tag_) {}
 
   template<typename Fn> void match(Fn F) const { F(Base, Tag); }
 
+  StringView getBaseName() const override { return Base->getBaseName(); }
+
   void printLeft(OutputBuffer &OB) const override {
     Base->printLeft(OB);
     OB += "[abi:";
     OB += Tag;
     OB += "]";
   }
 };
 

llvm/test/tools/llvm-cxxfilt/abitag.test

-RUN: llvm-cxxfilt -n _Z14returns_stringB5cxx11v _Z6globalB5cxx11 _Z6globalB12a_longer_tag | FileCheck %s
+RUN: llvm-cxxfilt -n _Z14returns_stringB5cxx11v _Z6globalB5cxx11 _Z6globalB12a_longer_tag _ZN6globalB3TagC2Ev _ZN6globalB3TagD2Ev | FileCheck %s
 
 CHECK: returns_string[abi:cxx11]()
 CHECK-NEXT: global[abi:cxx11]
 CHECK-NEXT: global[abi:a_longer_tag]
-
+CHECK-NEXT: global[abi:Tag]::global()
+CHECK-NEXT: global[abi:Tag]::~global()