[Lldb-commits] [lldb] [lldb/DWARF] Fix type definition search with simple template names (PR #95905)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 18 03:49:40 PDT 2024


https://github.com/labath created https://github.com/llvm/llvm-project/pull/95905

With simple template names the template arguments aren't embedded in the DW_AT_name attribute of the type. The code in
FindDefinitionTypeForDWARFDeclContext was comparing the synthesized template arguments on the leaf (most deeply nested) DIE, but was not sufficient, as the difference get be at any level above that (Foo<T>::Bar vs. Foo<U>::Bar). This patch makes sure we compare the entire context.

As a drive-by I also remove the completely unnecessary ConstStringification of the GetDIEClassTemplateParams result.

>From 9429eefc79b310731bafa63cf4db46eea3b0756e Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Mon, 17 Jun 2024 13:45:24 +0200
Subject: [PATCH] [lldb/DWARF] Fix type definition search with simple template
 names

With simple template names the template arguments aren't embedded in the
DW_AT_name attribute of the type. The code in
FindDefinitionTypeForDWARFDeclContext was comparing the synthesized
template arguments on the leaf (most deeply nested) DIE, but was not
sufficient, as the difference get be at any level above that
(Foo<T>::Bar vs. Foo<U>::Bar). This patch makes sure we compare the
entire context.

As a drive-by I also remove the completely unnecessary
ConstStringification of the GetDIEClassTemplateParams result.
---
 .../Plugins/SymbolFile/DWARF/DWARFASTParser.h |  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 16 ++---
 .../SymbolFile/DWARF/DWARFASTParserClang.h    |  4 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 70 ++++++++++---------
 .../x86/simple-template-names-context.cpp     |  2 +-
 5 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index 66db396279e06..abaeb2502cbbd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -58,7 +58,7 @@ class DWARFASTParser {
   virtual void EnsureAllDIEsInDeclContextHaveBeenParsed(
       CompilerDeclContext decl_context) = 0;
 
-  virtual ConstString GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
+  virtual std::string GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
 
   static std::optional<SymbolFile::ArrayInfo>
   ParseChildArrayInfo(const DWARFDIE &parent_die,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 02c91ddc57f83..cffe3f7dd6128 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -838,16 +838,16 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
   return type_sp;
 }
 
-ConstString
+std::string
 DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
   if (llvm::StringRef(die.GetName()).contains("<"))
-    return ConstString();
+    return "";
 
   TypeSystemClang::TemplateParameterInfos template_param_infos;
-  if (ParseTemplateParameterInfos(die, template_param_infos)) {
-    return ConstString(m_ast.PrintTemplateParams(template_param_infos));
-  }
-  return ConstString();
+  if (ParseTemplateParameterInfos(die, template_param_infos))
+    return m_ast.PrintTemplateParams(template_param_infos);
+
+  return "";
 }
 
 TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
@@ -1632,7 +1632,7 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
     case DW_TAG_union_type: {
       if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
         qualified_name.insert(
-            0, GetDIEClassTemplateParams(parent_decl_ctx_die).AsCString(""));
+            0, GetDIEClassTemplateParams(parent_decl_ctx_die));
         qualified_name.insert(0, "::");
         qualified_name.insert(0, class_union_struct_name);
       }
@@ -1650,7 +1650,7 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
     qualified_name.append("::");
 
   qualified_name.append(name);
-  qualified_name.append(GetDIEClassTemplateParams(die).AsCString(""));
+  qualified_name.append(GetDIEClassTemplateParams(die));
 
   return qualified_name;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 136a49e462fbb..7b5ddbaa2a6b5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -104,9 +104,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
   ///
   /// \param die The struct/class DWARFDIE containing template parameters.
   /// \return A string, including surrounding '<>', of the template parameters.
-  /// If the DIE's name already has '<>', returns an empty ConstString because
+  /// If the DIE's name already has '<>', returns an empty string because
   /// it's assumed that the caller is using the DIE name anyway.
-  lldb_private::ConstString GetDIEClassTemplateParams(
+  std::string GetDIEClassTemplateParams(
       const lldb_private::plugin::dwarf::DWARFDIE &die) override;
 
 protected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 369c742a5ee02..1dd19dbaac137 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3073,14 +3073,43 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
 
     // See comments below about -gsimple-template-names for why we attempt to
     // compute missing template parameter names.
-    ConstString template_params;
-    if (type_system) {
-      DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
-      if (dwarf_ast)
-        template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+    std::vector<std::string> template_params;
+    DWARFDeclContext die_dwarf_decl_ctx;
+    DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() : nullptr;
+    for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
+         ctx_die = ctx_die.GetParentDeclContextDIE()) {
+      die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
+      template_params.push_back(
+          (ctx_die.IsStructUnionOrClass() && dwarf_ast)
+              ? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
+              : "");
     }
+    const bool any_template_params = llvm::any_of(
+        template_params, [](llvm::StringRef p) { return !p.empty(); });
 
-    const DWARFDeclContext die_dwarf_decl_ctx = die.GetDWARFDeclContext();
+    auto die_matches = [&](DWARFDIE type_die) {
+      // Resolve the type if both have the same tag or {class, struct} tags.
+      const bool tag_matches =
+          type_die.Tag() == tag ||
+          (IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
+      if (!tag_matches)
+        return false;
+      if (any_template_params) {
+        size_t pos = 0;
+        for (DWARFDIE ctx_die = type_die;
+             type_die && !isUnitType(ctx_die.Tag()) &&
+             pos < template_params.size();
+             ctx_die = ctx_die.GetParentDeclContextDIE(), ++pos) {
+          if (template_params[pos].empty())
+            continue;
+          if (template_params[pos] != dwarf_ast->GetDIEClassTemplateParams(ctx_die))
+            return false;
+        }
+        if (pos != template_params.size())
+          return false;
+      }
+      return true;
+    };
     m_index->GetFullyQualifiedType(die_dwarf_decl_ctx, [&](DWARFDIE type_die) {
       // Make sure type_die's language matches the type system we are
       // looking for. We don't want to find a "Foo" type from Java if we
@@ -3089,13 +3118,7 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
           !type_system->SupportsLanguage(GetLanguage(*type_die.GetCU())))
         return true;
 
-      const dw_tag_t type_tag = type_die.Tag();
-      // Resolve the type if both have the same tag or {class, struct} tags.
-      const bool try_resolving_type =
-          type_tag == tag ||
-          (IsStructOrClassTag(type_tag) && IsStructOrClassTag(tag));
-
-      if (!try_resolving_type) {
+      if (!die_matches(type_die)) {
         if (log) {
           GetObjectFile()->GetModule()->LogMessage(
               log,
@@ -3123,27 +3146,6 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
       if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
         return true;
 
-      // With -gsimple-template-names, the DIE name may not contain the template
-      // parameters. If the declaration has template parameters but doesn't
-      // contain '<', check that the child template parameters match.
-      if (template_params) {
-        llvm::StringRef test_base_name =
-            GetTypeForDIE(type_die)->GetBaseName().GetStringRef();
-        auto i = test_base_name.find('<');
-
-        // Full name from clang AST doesn't contain '<' so this type_die isn't
-        // a template parameter, but we're expecting template parameters, so
-        // bail.
-        if (i == llvm::StringRef::npos)
-          return true;
-
-        llvm::StringRef test_template_params =
-            test_base_name.slice(i, test_base_name.size());
-        // Bail if template parameters don't match.
-        if (test_template_params != template_params.GetStringRef())
-          return true;
-      }
-
       type_sp = resolved_type->shared_from_this();
       return false;
     });
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
index a8a4d3b8fbd5f..8070b7a19abcc 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
@@ -12,7 +12,7 @@
 // CHECK: (lldb) target variable
 // CHECK-NEXT: (ReferencesBoth<'A'>) both_a = {
 // CHECK-NEXT:   (Outer<'A'>::Inner *) a = 0x{{[0-9A-Fa-f]*}} {}
-// CHECK-NEXT:   (Outer<'A'>::Inner *) b = 0x{{[0-9A-Fa-f]*}} {}
+// CHECK-NEXT:   (Outer<'B'>::Inner *) b = 0x{{[0-9A-Fa-f]*}} {}
 // CHECK-NEXT: }
 // CHECK-NEXT: (ReferencesBoth<'B'>) both_b = {
 // CHECK-NEXT:   (Outer<'A'>::Inner *) a = 0x{{[0-9A-Fa-f]*}} {}



More information about the lldb-commits mailing list