================
@@ -2482,6 +2485,134 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE 
&orig_die,
   return false;
 }
 
+static int ClangToItaniumCtorKind(clang::CXXCtorType kind) {
+  switch (kind) {
+  case clang::CXXCtorType::Ctor_Complete:
+    return 1;
+  case clang::CXXCtorType::Ctor_Base:
+    return 2;
+  case clang::CXXCtorType::Ctor_CopyingClosure:
+  case clang::CXXCtorType::Ctor_DefaultClosure:
+  case clang::CXXCtorType::Ctor_Comdat:
+    llvm_unreachable("Unexpected constructor kind.");
+  }
+}
+
+static int ClangToItaniumDtorKind(clang::CXXDtorType kind) {
+  switch (kind) {
+  case clang::CXXDtorType::Dtor_Deleting:
+    return 0;
+  case clang::CXXDtorType::Dtor_Complete:
+    return 1;
+  case clang::CXXDtorType::Dtor_Base:
+    return 2;
+  case clang::CXXDtorType::Dtor_Comdat:
+    llvm_unreachable("Unexpected destructor kind.");
+  }
+}
+
+static std::optional<int>
+GetItaniumCtorDtorVariant(llvm::StringRef discriminator) {
+  const bool is_ctor = discriminator.consume_front("C");
+  if (!is_ctor && !discriminator.consume_front("D"))
+    return std::nullopt;
+
+  uint64_t structor_kind;
+  if (!llvm::to_integer(discriminator, structor_kind))
+    return std::nullopt;
+
+  if (is_ctor) {
+    if (structor_kind > clang::CXXCtorType::Ctor_DefaultClosure)
+      return std::nullopt;
+
+    return ClangToItaniumCtorKind(
+        static_cast<clang::CXXCtorType>(structor_kind));
+  }
+
+  if (structor_kind > clang::CXXDtorType::Dtor_Comdat)
+    return std::nullopt;
+
+  return 
ClangToItaniumDtorKind(static_cast<clang::CXXDtorType>(structor_kind));
+}
+
+DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel 
&label,
+                                                 const DWARFDIE &declaration) {
+  DWARFDIE definition;
+  llvm::DenseMap<int, DWARFDIE> structor_variant_to_die;
+
+  // eFunctionNameTypeFull for mangled name lookup.
+  // eFunctionNameTypeMethod is required for structor lookups (since we look
+  // those up by DW_AT_name).
+  Module::LookupInfo info(ConstString(label.lookup_name),
+                          lldb::eFunctionNameTypeFull |
+                              lldb::eFunctionNameTypeMethod,
+                          lldb::eLanguageTypeUnknown);
+
+  m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) {
+    if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0))
+      return IterationAction::Continue;
+
+    auto spec = entry.GetAttributeValueAsReferenceDIE(DW_AT_specification);
+    if (!spec)
+      return IterationAction::Continue;
+
+    if (spec != declaration)
----------------
labath wrote:

> is actually redundant because we can just rely on the fallback lookup

Is it? I can see how the fallback lookup can work for functions where you have 
mangled names, as those are more-or-less unique. However, I don't see how we 
could find the correct constructor if all we have is the name of the class.

> Happy to try and change it to a structural match though.

I think that is necessary for this to work. Otherwise, this will fail whenever 
we parse a type from a CU which does not contain the definition of the methods 
(at least constructors) of that type. And I think that can happen pretty easily 
in a big binary.

> Here a in lib.o would point to the DW_TAG_structure_type of main.o, but the 
> subprogram definitions for the constructors are different (and local to each 
> CU).


Hm... That's confusing. I was under the impression that ld64 doesn't touch 
debug info (that it just leaves breadcrumbs so that the debug info can be 
linked later. Did that change? If not, how can lib.o point to a different file. 
Surely clang does not inspect main.o when compiling lib.o.

Or did you mean that this happens after running dsymutil over the binary? 
Because this matches what I would expect dsymutil to do.

> I don't think we had reservations about encoding ABI-tag information into 
> DWARF. The concerns were:
> 
>     1. debug-info size (libc++ uses these tags on a ton of APIs).

Yes, I can see how that could be an issue.

>     2. whether we want to rely on AST fidelity again

Yeah, I don't want to rely on being able to roundtrip clang ASTs. The thing 
which changed the calculus for me is that the realisation that this is 
necessary even to just understand the DWARF correctly. Like, with ABI tags, I 
can safely link a binary which contains two versions of `struct Foo` (as long 
as they are not used in the same CU). However, if I now have a third CU, which 
defines a constructor for `Foo`, I have no way to tell which of the two struct 
definitions it refers to.

>     3. libc++ makes no guarantee that they'll continue annotating individual 
> APIs in the future and might put the tags on namespaces/structures. At which 
> point LLDB will need to take care to propagate the tags appropriately (maybe 
> this will be handled by Clang out-of-the-box, though I haven't thought about 
> this specific point in detail)

I'm not particularly concerned by that, for one because I don't (primarily) 
want to feed these tags into clang. My main worry is not being able to 
differentiate these classes in DWARF.




> Do you prefer us looking into alternatives to the approach in this PR? E.g., 
> make DWARF do some of the heavy lifting? We could put e.g., the structor 
> variant info into DWARF, alleviating the need for demangling, etc.

No, I still think this is the best approach for this. I'm just disappointed 
that it's turning out to be more complicated than I expected. I view the 
introduction of abi tags as an orthogonal issue -- if it gets included in 
DWARF, we can include it in the structural match.

I would love if DWARF contained more information on structor variants. I find 
it disappointing that the only way to locate the complete object constructor is 
to remangle the name of the subobject constructor and look it up in the symbol 
table. I think that's bad because DWARF should be usable without a symbol 
table. However, I suspect that's not going to be easy...

https://github.com/llvm/llvm-project/pull/149827
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to