================ @@ -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) ---------------- Michael137 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. Yup that's correct. I meant it should work for mangled name lookup. For constructors we just would behave as we do today. Not ideal, but not a regression. But yea, ideally we'd fix this as you suggest. All that is to say, the type-unit check in this PR is redundant (regardless of which route we take). > > 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. Would a structural match here help though? If the declaration DIE for a ctor got encoded into the `AsmLabel` but the definition lives in a different CU (and thus `SymbolFile`), we wouldn't be able to find it anyway using our approach right? AFAIU, the case where a structural match would help is if: 1. constructor declaration DIE (and its module) got encoded into the `AsmLabel` 2. the definition lives in the same module but the `DW_AT_specification` points outside of the module I have been having a hard time coming up with an example of (2) happening. But I'm sure compilers are allowed to do this (and probably already do?). With type-units this can happen because the declaration lives in `.debug_types` but the definition (and its specification) lives in `.debug_info. So the `SymbolFile` lookup will find the definition but the specification DIE is not the same as the declaration DIE. So there a structural match would help. > > 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. Ah yes, sorry, forgot about `dsymutil` here. > > 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. Ah I see, agreed > 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... I wonder how GDB makes use of the placeholder mangling (i.e., `C4`) that GCC puts on declarations. Or if not for GDB, why does GCC do it? I guess if we had such a placeholder mangling, our lookup could become "find all DIEs with name Foo whose specification has _Z3FooC4 linkage name (regardless of which module it lives in)". That does make our lives a bit easier. But maybe that's not a blocker for this PR (unless it turns out the structural match is a big problem). 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