================ @@ -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