[lldb][DWARFASTParserClang] Add support for DW_TAG_LLVM_annotation

[Michael137]

Summary

This patch adds support for parsing DW_TAG_LLVM_annotations for
abi_tag's. This allows us to recreate more accurate ASTs nodes for
constructors/destructors, where previously we relied on guessing the
correct name mangling to resolve a ctor/dtor call. If a ctor/dtor
had an abi_tag we would fail to resolve the symbol because the
"guessing algorithm" (see CollectCandidateCPlusPlusNames) doesn't
account for them.

This is necessary to support calling destructors/constructors
of numerous libcxx types, since they recently got abi-tagged.

Testing

• Added API test

[Michael137]

Alternative to https://reviews.llvm.org/D143652

[Michael137]

• Update test

[dblaikie]

Any chance we can make these work more like member functions (could the ctors include their mangled names, for instance)? Or is it the innate nature of ctors having the various C1/C2/etc versions?

[dblaikie]

In D144182#4132736, @dblaikie wrote:

Any chance we can make these work more like member functions (could the ctors include their mangled names, for instance)? Or is it the innate nature of ctors having the various C1/C2/etc versions?

Oh, sorry, this is more suitable on the LLVM side... I'll make the comment over there instead.

[Michael137]

• Fix test on Linux
• Add -glldb flag to test

[Michael137]

• Add test case for C2 constructor

[Michael137]

• Update tests

[Michael137]

• Update tets

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

protected:
   bool ParseChildMembers(
       const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
       std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
       std::vector<DWARFDIE> &member_function_dies,
       DelayedPropertyList &delayed_properties,
       const lldb::AccessType default_accessibility,
       lldb_private::ClangASTImporter::LayoutInfo &layout_info);
 
-  size_t
-  ParseChildParameters(clang::DeclContext *containing_decl_ctx,
-                       const DWARFDIE &parent_die, bool skip_artificial,
-                       bool &is_static, bool &is_variadic,
-                       bool &has_template_params,
-                       std::vector<lldb_private::CompilerType> &function_args,
-                       std::vector<clang::ParmVarDecl *> &function_param_decls,
-                       unsigned &type_quals);
+  size_t ParseChildParameters(
+      clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
+      bool skip_artificial, bool &is_static, bool &is_variadic,
+      unsigned &type_quals, llvm::SmallVector<llvm::StringRef> &abi_tags);
 
   size_t ParseChildEnumerators(lldb_private::CompilerType &compiler_type,
                                bool is_signed, uint32_t enumerator_byte_size,
                                const DWARFDIE &parent_die);
 
   /// Parse a structure, class, or union type DIE.
   lldb::TypeSP ParseStructureLikeDIE(const lldb_private::SymbolContext &sc,
                                      const DWARFDIE &die,

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
   bool is_cxx_method = DeclKindIsCXXClass(containing_decl_kind);
   // Start off static. This will be set to false in
   // ParseChildParameters(...) if we find a "this" parameters as the
   // first parameter
   if (is_cxx_method) {
     is_static = true;
   }
 
+  llvm::SmallVector<llvm::StringRef> abi_tags;
   if (die.HasChildren()) {
     bool skip_artificial = true;
     ParseChildParameters(containing_decl_ctx, die, skip_artificial, is_static,
-                         is_variadic, has_template_params,
-                         function_param_types, function_param_decls,
-                         type_quals);
+                         is_variadic, has_template_params, function_param_types,
+                         function_param_decls, type_quals, abi_tags);
   }
 
   bool ignore_containing_context = false;
   // Check for templatized class member functions. If we had any
   // DW_TAG_template_type_parameter or DW_TAG_template_value_parameter
   // the DW_TAG_subprogram DIE, then we can't let this become a method in
   // a class. Why? Because templatized functions are only emitted if one
   // of the templatized methods is used in the current compile unit and
if (tag == DW_TAG_subprogram || tag == DW_TAG_inlined_subroutine) {
                   clang::CXXMethodDecl *cxx_method_decl =
                       m_ast.AddMethodToCXXRecordType(
                           class_opaque_type.GetOpaqueQualType(),
                           attrs.name.GetCString(), attrs.mangled_name,
                           clang_type, attrs.accessibility, attrs.is_virtual,
                           is_static, attrs.is_inline, attrs.is_explicit,
                           is_attr_used, attrs.is_artificial);
 
+                  if (!abi_tags.empty()) {
+                    auto *abi_tag_attr = clang::AbiTagAttr::Create(
+                        m_ast.getASTContext(), abi_tags.data(), abi_tags.size(),
+                        {}, {});
+                    cxx_method_decl->addAttr(abi_tag_attr);
+                  }
+
                   type_handled = cxx_method_decl != nullptr;
                   // Artificial methods are always handled even when we
                   // don't create a new declaration for them.
                   type_handled |= attrs.is_artificial;
 
                   if (cxx_method_decl) {
                     LinkDeclContextToDIE(cxx_method_decl, die);
 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   unsigned type_quals = 0;
   std::vector<CompilerType> param_types;
   std::vector<clang::ParmVarDecl *> param_decls;
   StreamString sstr;
 
   DWARFDeclContext decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
   sstr << decl_ctx.GetQualifiedName();
 
+  llvm::SmallVector<llvm::StringRef> abi_tags;
   clang::DeclContext *containing_decl_ctx =
       GetClangDeclContextContainingDIE(die, nullptr);
   ParseChildParameters(containing_decl_ctx, die, true, is_static, is_variadic,
                        has_template_params, param_types, param_decls,
-                       type_quals);
+                       type_quals, abi_tags);
+
   sstr << "(";
   for (size_t i = 0; i < param_types.size(); i++) {
     if (i > 0)
       sstr << ", ";
     sstr << param_types[i].GetTypeName();
   }
   if (is_variadic)
     sstr << ", ...";
case DW_TAG_subprogram:
       // Let the type parsing code handle this one for us.
       member_function_dies.push_back(die);
       break;
 
     case DW_TAG_inheritance:
       ParseInheritance(die, parent_die, class_clang_type, default_accessibility,
                        module_sp, base_classes, layout_info);
       break;
-
     default:
       break;
     }
   }
 
   return true;
 }
 
 size_t DWARFASTParserClang::ParseChildParameters(
     clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
     bool skip_artificial, bool &is_static, bool &is_variadic,
     bool &has_template_params, std::vector<CompilerType> &function_param_types,
     std::vector<clang::ParmVarDecl *> &function_param_decls,
-    unsigned &type_quals) {
+    unsigned &type_quals, llvm::SmallVector<llvm::StringRef> &abi_tags) {
   if (!parent_die)
     return 0;
 
   size_t arg_idx = 0;
   for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
     switch (tag) {
     case DW_TAG_formal_parameter: {
case DW_TAG_GNU_template_parameter_pack:
       // The one caller of this was never using the template_param_infos, and
       // the local variable was taking up a large amount of stack space in
       // SymbolFileDWARF::ParseType() so this was removed. If we ever need the
       // template params back, we can add them back.
       // ParseTemplateDIE (dwarf_cu, die, template_param_infos);
       has_template_params = true;
       break;
 
+    case DW_TAG_LLVM_annotation: {
+      DWARFAttributes attributes;
+      const char *name = die.GetName();
+      if (!name || ::strcmp(name, "abi_tag") != 0)
+        break;
+
+      const char *tag =
+          die.GetAttributeValueAsString(DW_AT_const_value, nullptr);
+      if (tag)
+        abi_tags.push_back(tag);
+    } break;
     default:
       break;
     }
   }
   return arg_idx;
 }
 
 Type *DWARFASTParserClang::GetTypeForDIE(const DWARFDIE &die) {

lldb/test/API/lang/cpp/external_ctor_dtor_lookup/Makefile

+CXX_SOURCES := main.cpp lib.cpp
+CXXFLAGS_EXTRAS := -glldb 
+                       
+include Makefile.rules 

lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py

+"""
+Test that we can constructors/destructors
+without a linkage name because they are
+marked DW_AT_external and the fallback
+mangled-name-guesser in LLDB doesn't account
+for ABI tags.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ExternalCtorDtorLookupTestCase(TestBase):
+
+    @skipIfWindows
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, 'b\.getWrapper\(\)',
+                lldb.SBFileSpec('main.cpp', False))
+
+        self.expect_expr('b.sinkWrapper(b.getWrapper())', result_type='int', result_value='-1')
+        self.expect_expr('B{}.m_int', result_type='int', result_value='47')
+        self.expect_expr('D{}.m_int', result_type='int', result_value='-1')
+        self.filecheck("target module dump ast", __file__)
+# CHECK: |-CXXRecordDecl {{.*}} struct B definition
+# CHECK: | |-virtual public 'A'
+# CHECK: | `-CXXConstructorDecl {{.*}} B 'void ()'
+# CHECK: |   `-AbiTagAttr {{.*}} TagB
+# CHECK: |-CXXRecordDecl {{.*}} struct D definition
+# CHECK: | `-CXXConstructorDecl {{.*}} D 'void ()'
+# CHECK: |   `-AbiTagAttr {{.*}} TagD
+# CHECK: |-CXXRecordDecl {{.*}} struct A definition
+# CHECK: | |-CXXConstructorDecl {{.*}} A 'void (int)'
+# CHECK: | | |-ParmVarDecl {{.*}} 'int'
+# CHECK: | | `-AbiTagAttr {{.*}} Ctor Int
+# CHECK: | |-CXXConstructorDecl {{.*}} A 'void (float)'
+# CHECK: | | |-ParmVarDecl {{.*}} 'float'
+# CHECK: | | `-AbiTagAttr {{.*}} Ctor Float
+# CHECK: | |-CXXConstructorDecl {{.*}} A 'void ()'
+# CHECK: | | `-AbiTagAttr {{.*}} Ctor Default                    
+# CHECK: | `-CXXDestructorDecl {{.*}} ~A 'void ()'
+# CHECK: |   `-AbiTagAttr {{.*}}  Default Dtor
+        
+        # Confirm that we can call the C2 constructor for 'struct A'
+        self.expect('expression --top-level -- class Derived : virtual A {};')
+        self.expect_expr('Derived{}.m_int', result_type='int', result_value='-1')
+
+        # Confirm that we can call the D2 destructor for 'struct A'
+        self.expect('expression Derived* $derived_ptr = new Derived()')
+        self.expect('expression delete $derived_ptr')

lldb/test/API/lang/cpp/external_ctor_dtor_lookup/lib.h

+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+template <typename T> class Wrapper {
+public:
+  [[gnu::abi_tag("test", "ctor")]] Wrapper(){};
+
+  [[gnu::abi_tag("test", "dtor")]] ~Wrapper(){};
+};
+
+struct Foo {};
+
+Wrapper<Foo> getFooWrapper();
+
+#endif // _H_IN

lldb/test/API/lang/cpp/external_ctor_dtor_lookup/lib.cpp

+#include "lib.h"
+
+Wrapper<Foo> getFooWrapper() { return {}; }

lldb/test/API/lang/cpp/external_ctor_dtor_lookup/main.cpp

+#include "lib.h"
+
+struct Bar {
+  Wrapper<Foo> getWrapper() { return Wrapper<Foo>(); }
+  int sinkWrapper(Wrapper<Foo>) { return -1; }
+};
+
+struct A {
+  int m_int = -1;
+  float m_float = -1.0;
+  [[gnu::abi_tag("Ctor", "Int")]] A(int a) : m_int(a) {}
+  [[gnu::abi_tag("Ctor", "Float")]] A(float f) : m_float(f) {}
+  [[gnu::abi_tag("Ctor", "Default")]] A() {}
+  [[gnu::abi_tag("Dtor", "Default")]] ~A() {}
+};
+
+struct B : virtual A {
+  [[gnu::abi_tag("TagB")]] B();
+};
+B::B() : A(47) {}
+
+struct D : B {
+  [[gnu::abi_tag("TagD")]] D() : A(4.7f) {}
+};
+
+struct X : public virtual A {};
+
+int main() {
+  Bar b;
+  Wrapper<int> w1;
+  Wrapper<double> w2;
+  Wrapper<Foo> w3 = getFooWrapper();
+  Wrapper<Foo> w4;
+  B b2;
+  D d;
+  A a;
+  auto *ptr = new X();
+  delete ptr;
+  return b.sinkWrapper(b.getWrapper());
+}