Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lldb simplified template names rebuild without clang ast #90008

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dwblaikie
Copy link
Collaborator

  • DO NOT SUBMIT: lldb DWARF-Clang AST parsing tracing
  • Fix scope operator ordering
  • DO NOT SUBMIT: Really dodgy demonstration of DWARFTypePrinter reuse in lldb

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 396cdabe47f3596464b289d0937c0066f50a0ac6 9a654b056d9c05c0aa4856db161c1f1b08b9dfe9 -- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp llvm/include/llvm-c/Error.h llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h llvm/lib/DebugInfo/DWARF/DWARFDie.cpp llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
View the diff from clang-format here.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 962844af28..1232f29e36 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -6,9 +6,9 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <algorithm>
 #include <cstdlib>
 #include <iostream>
-#include <algorithm>
 
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
@@ -1569,7 +1569,8 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
   if (!name)
     return "";
   static int indent = 0;
-  std::cerr << std::string(indent, ' ') << "starting qualified name for: " << name << '\n';
+  std::cerr << std::string(indent, ' ')
+            << "starting qualified name for: " << name << '\n';
   auto &FS = die.GetCU()->GetSymbolFileDWARF().GetObjectFile()->GetFileSpec();
   std::string Directory = FS.GetDirectory().AsCString("");
   std::cerr << std::string(indent, ' ')
@@ -1623,7 +1624,8 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
   qualified_name.append(GetDIEClassTemplateParams(die).AsCString(""));
 
   --indent;
-  std::cerr << std::string(indent, ' ') << "computed qualified name: " << qualified_name << '\n';
+  std::cerr << std::string(indent, ' ')
+            << "computed qualified name: " << qualified_name << '\n';
 
   return qualified_name;
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 6a5ef1f83a..10c946a292 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -87,9 +87,7 @@ public:
 
   // Accessing information about a DIE
   dw_tag_t Tag() const;
-  dw_tag_t getTag() const {
-    return Tag();
-  }
+  dw_tag_t getTag() const { return Tag(); }
   using DWARFFormValue = dwarf::DWARFFormValue;
 
   const char *GetTagAsCString() const;
@@ -103,9 +101,7 @@ public:
   lldb::user_id_t GetID() const;
 
   const char *GetName() const;
-  const char *getShortName() const {
-    return GetName();
-  }
+  const char *getShortName() const { return GetName(); }
 
   lldb::ModuleSP GetModule() const;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 064c510eea..eb8147e01c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -532,9 +532,7 @@ llvm::iterator_range<DWARFDIE::child_iterator> DWARFDIE::children() const {
 DWARFDIE::child_iterator DWARFDIE::begin() const {
   return child_iterator(*this);
 }
-DWARFDIE::child_iterator DWARFDIE::end() const {
-  return child_iterator();
-}
+DWARFDIE::child_iterator DWARFDIE::end() const { return child_iterator(); }
 std::optional<DWARFFormValue> DWARFDIE::find(const dw_attr_t attr) const {
   DWARFFormValue form_value;
   if (m_die->GetAttributeValue(m_cu, attr, form_value, nullptr, false))
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
index 42a9b9e8e7..c4e2db2407 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -118,15 +118,16 @@ protected:
   ValueType m_value;                 // Contains all data for the form
 };
 
-inline const char* toString(DWARFFormValue Value, const char* Default) {
-  if (const char* R = Value.AsCString())
+inline const char *toString(DWARFFormValue Value, const char *Default) {
+  if (const char *R = Value.AsCString())
     return R;
   return Default;
 }
-inline const char* toString(std::optional<DWARFFormValue> Value, const char* Default) {
+inline const char *toString(std::optional<DWARFFormValue> Value,
+                            const char *Default) {
   if (!Value)
     return Default;
-  if (const char* R = Value->AsCString())
+  if (const char *R = Value->AsCString())
     return R;
   return Default;
 }
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index abc0ab8e27..cadc47caf1 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -82,11 +82,14 @@ void DWARFTypePrinter<DieType>::appendArrayType(const DieType &D) {
     std::optional<uint64_t> Count;
     std::optional<uint64_t> UB;
     std::optional<unsigned> DefaultLB;
-    if (std::optional<typename DieType::DWARFFormValue> L = C.find(dwarf::DW_AT_lower_bound))
+    if (std::optional<typename DieType::DWARFFormValue> L =
+            C.find(dwarf::DW_AT_lower_bound))
       LB = L->getAsUnsignedConstant();
-    if (std::optional<typename DieType::DWARFFormValue> CountV = C.find(dwarf::DW_AT_count))
+    if (std::optional<typename DieType::DWARFFormValue> CountV =
+            C.find(dwarf::DW_AT_count))
       Count = CountV->getAsUnsignedConstant();
-    if (std::optional<typename DieType::DWARFFormValue> UpperV = C.find(dwarf::DW_AT_upper_bound))
+    if (std::optional<typename DieType::DWARFFormValue> UpperV =
+            C.find(dwarf::DW_AT_upper_bound))
       UB = UpperV->getAsUnsignedConstant();
     /*
     if (std::optional<typename DieType::DWARFFormValue> LV =
@@ -124,10 +127,11 @@ void DWARFTypePrinter<DieType>::appendArrayType(const DieType &D) {
 }
 
 namespace detail {
-template<typename DieType>
+template <typename DieType>
 DieType resolveReferencedType(DieType D,
-                                     dwarf::Attribute Attr = dwarf::DW_AT_type) {
-  return D.getAttributeValueAsReferencedDie(Attr); // .resolveTypeUnitReference();
+                              dwarf::Attribute Attr = dwarf::DW_AT_type) {
+  return D.getAttributeValueAsReferencedDie(
+      Attr); // .resolveTypeUnitReference();
 }
 template <typename DieType>
 DieType resolveReferencedType(DieType D, typename DieType::DWARFFormValue F) {
@@ -204,7 +208,8 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
       OS << '(';
     else if (Word)
       OS << ' ';
-    if (DieType Cont = detail::resolveReferencedType(D, dwarf::DW_AT_containing_type)) {
+    if (DieType Cont =
+            detail::resolveReferencedType(D, dwarf::DW_AT_containing_type)) {
       appendQualifiedName(Cont);
       EndedWithTemplate = false;
       OS << "::";
@@ -275,7 +280,8 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
 }
 
 template <typename DieType>
-void DWARFTypePrinter<DieType>::appendAndTerminateTemplateParameters(DieType D) {
+void DWARFTypePrinter<DieType>::appendAndTerminateTemplateParameters(
+    DieType D) {
   if (!appendTemplateParameters(D))
     return;
 
@@ -354,8 +360,11 @@ void DWARFTypePrinter<DieType>::appendUnqualifiedNameAfter(
     llvm::raw_string_ostream PtrauthStream(PtrauthString);
     PtrauthStream
         << "__ptrauth(" << getValOrNull(dwarf::DW_AT_LLVM_ptrauth_key) << ", "
-        << getValOrNull(dwarf::DW_AT_LLVM_ptrauth_address_discriminated) << ", 0x0"
-        << utohexstr(getValOrNull(dwarf::DW_AT_LLVM_ptrauth_extra_discriminator), true)
+        << getValOrNull(dwarf::DW_AT_LLVM_ptrauth_address_discriminated)
+        << ", 0x0"
+        << utohexstr(
+               getValOrNull(dwarf::DW_AT_LLVM_ptrauth_extra_discriminator),
+               true)
         << options << ")";
     OS << PtrauthStream.str();
     break;
@@ -470,8 +479,8 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
       } else if (Name == "char" ||
                  (IsQualifiedChar =
                       (Name == "unsigned char" || Name == "signed char"))) {
-        // FIXME: check T's dwarf::DW_AT_type to see if it's signed or not (since
-        // char signedness is implementation defined).
+        // FIXME: check T's dwarf::DW_AT_type to see if it's signed or not
+        // (since char signedness is implementation defined).
         auto Val = *V->getAsSignedConstant();
         // Copied/hacked up from Clang's CharacterLiteral::print - incomplete
         // (doesn't actually support different character types/widths, sign
@@ -573,8 +582,8 @@ void DWARFTypePrinter<DieType>::appendConstVolatileQualifierAfter(DieType N) {
   DieType T;
   decomposeConstVolatile(N, T, C, V);
   if (T && T.getTag() == dwarf::DW_TAG_subroutine_type)
-    appendSubroutineNameAfter(T, detail::resolveReferencedType(T), false, C.isValid(),
-                              V.isValid());
+    appendSubroutineNameAfter(T, detail::resolveReferencedType(T), false,
+                              C.isValid(), V.isValid());
   else
     appendUnqualifiedNameAfter(T, detail::resolveReferencedType(T));
 }
@@ -632,7 +641,8 @@ void DWARFTypePrinter<DieType>::appendSubroutineNameAfter(
         P.getTag() != dwarf::DW_TAG_unspecified_parameters)
       return;
     DieType T = detail::resolveReferencedType(P);
-    if (SkipFirstParamIfArtificial && RealFirst && P.find(dwarf::DW_AT_artificial)) {
+    if (SkipFirstParamIfArtificial && RealFirst &&
+        P.find(dwarf::DW_AT_artificial)) {
       FirstParamIfArtificial = T;
       RealFirst = false;
       continue;
@@ -752,7 +762,7 @@ void DWARFTypePrinter<DieType>::appendScopes(DieType D) {
     return;
   if (D.getTag() == dwarf::DW_TAG_lexical_block)
     return;
-  //D = D.resolveTypeUnitReference();
+  // D = D.resolveTypeUnitReference();
   if (DieType P = D.getParent())
     appendScopes(P);
   appendUnqualifiedName(D);

dwblaikie added 7 commits May 31, 2024 19:43
…n lldb

The hacks necessary to make lldb's DWARFDIE APIs sufficiently compatible
with LLVM's DWARFDie API aren't shippable, but maybe somewhere to start
the conversation.

With all these changes, an internal example that would crash expanding
too many types (computing the fully qualified name for 414671 types before
crashing due to running out of stack) - but with these patches applied,
it comes down to 856 expansions (compared to 848 for non-simplified
template names inputs)
Rough prototype, doesn't handle namespace/context
@dwblaikie dwblaikie force-pushed the lldb_simplified_template_names_rebuild_without_clang_ast branch from 9a654b0 to 334301a Compare September 11, 2024 16:20
@Michael137
Copy link
Member

Very cool!

@dwblaikie
Copy link
Collaborator Author

Very cool!

Yeah, I had loftier goals of taking this even further (some of lldb's comparisons are DIE-to-DIE (eg: when trying to find a definition from a declaration) and in that case it'd be nice to not create any strings (when not using simplified template names, lldb sort of does this - by having the unqualified name, and the list of contexts, each context as the separate original string I think - and just walks the list to compare, rather than concatenating first - basically wanting to do something more like that but for simplified template names too, in which case it's not sufficient to just build a list of strings for context, you have to be able to walk all over the DWARF, etc) - this would allow recurring entities to be compared cheaply/once, and cache the subcomparison to reuse (eg: t1<t2<int>, t2<int>> could compare the t2<int> DIE on one side of the comparison with the one on the other side, then next time it sees the same DIE on one side, it could check that it's equal to the expected equivalent node on the other side and skip further comparison)

But for now, getting what's already here in would be good - and the further generalization can wait for another day :)

ZequanWu added a commit that referenced this pull request Oct 8, 2024
This generalizes DWARFTypePrinter class to a template class so that it
can be reused for lldb's DWARFDIE type.

This is a split of #90008. The
difference is that this doesn't have `Visitor` template parameter which
can be added later if necessary.
ZequanWu added a commit that referenced this pull request Nov 18, 2024
…mes with DWARFTypePrinter (#112811)

This is the second half of
#90008.

Essentially, it replaces the work of resolving template types when we
just need the qualified names with walking the DIE tree using
`DWARFTypePrinter`.

### Result
For an internal target, the time spent on `expr *this` for the first
time reduced from 28 secs to 17 secs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants