Index: test/tools/dsymutil/X86/modules.m =================================================================== --- test/tools/dsymutil/X86/modules.m +++ test/tools/dsymutil/X86/modules.m @@ -23,11 +23,16 @@ // --------------------------------------------------------------------- #ifdef BAR_H // --------------------------------------------------------------------- -// CHECK: DW_TAG_compile_unit -// CHECK: DW_TAG_module -// CHECK-NEXT: DW_AT_name {{.*}}"Bar" -// CHECK: DW_TAG_member -// CHECK: DW_AT_name {{.*}}"value" +// CHECK: DW_TAG_compile_unit +// CHECK-NOT: DW_TAG +// CHECK: DW_TAG_module +// CHECK-NEXT: DW_AT_name{{.*}}"Bar" +// CHECK-NOT: DW_TAG +// CHECK: 0x0[[BAR:.*]]: DW_TAG_structure_type +// CHECK: DW_AT_name {{.*}}"Bar" +// CHECK-NOT: DW_TAG +// CHECK: DW_TAG_member +// CHECK: DW_AT_name {{.*}}"value" struct Bar { int value; @@ -37,10 +42,12 @@ // --------------------------------------------------------------------- #ifdef FOO_H // --------------------------------------------------------------------- -// CHECK: 55{{.*}}DW_TAG_compile_unit -// CHECK: DW_TAG_module -// CHECK-NEXT: DW_AT_name {{.*}}"Foo" -// CHECK: DW_TAG_typedef +// CHECK: DW_TAG_compile_unit +// CHECK-NOT: DW_TAG +// CHECK: 0x0[[FOO:.*]]: DW_TAG_module +// CHECK-NEXT: DW_AT_name{{.*}}"Foo" +// CHECK-NOT: DW_TAG +// CHECK: DW_TAG_typedef @import Bar; typedef struct Bar Bar; struct S {}; @@ -49,7 +56,20 @@ #else // --------------------------------------------------------------------- -// CHECK: DW_TAG_compile_unit +// CHECK: DW_TAG_compile_unit +// CHECK: DW_TAG_module +// CHECK-NEXT: DW_AT_name{{.*}}"Bar" +// CHECK: DW_TAG_module +// CHECK-NEXT: DW_AT_name{{.*}}"Foo" +// CHECK-NOT: DW_TAG +// CHECK: DW_TAG_typedef +// CHECK-NOT: DW_TAG +// CHECK: DW_AT_type [DW_FORM_ref_addr] (0x{{0*}}[[BAR]]) +// +// CHECK: DW_TAG_imported_declaration +// CHECK-NOT: DW_TAG +// CHECK: DW_AT_import [DW_FORM_ref_addr] (0x{{0*}}[[FOO]] +// // CHECK: DW_TAG_subprogram // CHECK: DW_AT_name {{.*}}"main" @import Foo; Index: tools/dsymutil/DwarfLinker.cpp =================================================================== --- tools/dsymutil/DwarfLinker.cpp +++ tools/dsymutil/DwarfLinker.cpp @@ -188,13 +188,15 @@ DeclContext *Ctxt; ///< ODR Declaration context. DIE *Clone; ///< Cloned version of that DIE. uint32_t ParentIdx; ///< The index of this DIE's parent. - bool Keep; ///< Is the DIE part of the linked output? - bool InDebugMap; ///< Was this DIE's entity found in the map? + bool Keep : 1; ///< Is the DIE part of the linked output? + bool InDebugMap : 1;///< Was this DIE's entity found in the map? + bool Prune : 1; ///< Is this a pure forward declaration we can strip? }; - CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR) + CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR, + StringRef ClangModuleName) : OrigUnit(OrigUnit), ID(ID), LowPc(UINT64_MAX), HighPc(0), RangeAlloc(), - Ranges(RangeAlloc) { + Ranges(RangeAlloc), ClangModuleName(ClangModuleName) { Info.resize(OrigUnit.getNumDIEs()); const auto *CUDie = OrigUnit.getUnitDIE(false); @@ -224,6 +226,8 @@ void setOutputUnitDIE(DIE *Die) { CUDie = Die; } bool hasODR() const { return HasODR; } + bool isClangModule() const { return !ClangModuleName.empty(); } + const std::string &getClangModuleName() const { return ClangModuleName; } DIEInfo &getInfo(unsigned Idx) { return Info[Idx]; } const DIEInfo &getInfo(unsigned Idx) const { return Info[Idx]; } @@ -378,11 +382,14 @@ bool HasODR; /// Did a DIE actually contain a valid reloc? bool HasInterestingContent; + /// If this is a Clang module, this holds the module's name. + std::string ClangModuleName; }; void CompileUnit::markEverythingAsKept() { for (auto &I : Info) - I.Keep = true; + // Mark everything that wasn't explicity marked for pruning. + I.Keep = !I.Prune; } uint64_t CompileUnit::computeNextUnitOffset() { @@ -1206,7 +1213,8 @@ /// file (and all the modules imported by it in a bottom-up fashion) /// to Units. void loadClangModule(StringRef Filename, StringRef ModulePath, - DebugMap &ModuleMap, unsigned Indent = 0); + StringRef ModuleName, DebugMap &ModuleMap, + unsigned Indent = 0); /// \brief Flags passed to DwarfLinker::lookForDIEsToKeep enum TravesalFlags { @@ -1549,8 +1557,9 @@ default: // By default stop gathering child contexts. return PointerIntPair(nullptr); + case dwarf::DW_TAG_module: + break; case dwarf::DW_TAG_compile_unit: - // FIXME: Add support for DW_TAG_module. return PointerIntPair(&Context); case dwarf::DW_TAG_subprogram: // Do not unique anything inside CU local functions. @@ -1609,10 +1618,12 @@ // creating: File, line number and byte size. This shouldn't be // necessary, because the ODR is just about names, but given that we // do some approximations with overloaded functions and anonymous - // namespaces, use these additional data points to make the process safer. + // namespaces, use these additional data points to make the process + // safer. This is disabled for clang modules, because forward + // declarations of module-defined types do not have a file and line. ByteSize = DIE->getAttributeValueAsUnsignedConstant( &U.getOrigUnit(), dwarf::DW_AT_byte_size, UINT64_MAX); - if (Tag != dwarf::DW_TAG_namespace || !Name) { + if (!U.isClangModule() && (Tag != dwarf::DW_TAG_namespace || !Name)) { if (unsigned FileNum = DIE->getAttributeValueAsUnsignedConstant( &U.getOrigUnit(), dwarf::DW_AT_decl_file, 0)) { if (const auto *LT = U.getOrigUnit().getContext().getLineTableForUnit( @@ -1652,12 +1663,17 @@ if (!Line && NameRef.empty()) return PointerIntPair(nullptr); - - // FIXME: dsymutil-classic compat won't unique the same type - // presented once as a struct and once as a class. Use the Tag in - // the fully qualified name hash to get the same effect. + // We hash NameRef, which is the mangled name, in order to get most - // overloaded functions resolvec correctly. + // overloaded functions resolve correctly. + // + // Strictly speaking, hashing the Tag is only necessary for a + // DW_TAG_module, to prevent uniquing of a module and a namespace + // with the same name. + // + // FIXME: dsymutil-classic won't unique the same type presented + // once as a struct and once as a class. Using the Tag in the fully + // qualified name hash to get the same effect. unsigned Hash = hash_combine(Context.getQualifiedNameHash(), Tag, NameRef); // FIXME: dsymutil-classic compatibility: when we don't have a name, @@ -1737,18 +1753,38 @@ return Streamer->init(TheTriple, OutputFilename); } -/// \brief Recursive helper to gather the child->parent relationships in the -/// original compile unit. -static void gatherDIEParents(const DWARFDebugInfoEntryMinimal *DIE, - unsigned ParentIdx, CompileUnit &CU, - DeclContext *CurrentDeclContext, - NonRelocatableStringpool &StringPool, - DeclContextTree &Contexts) { +/// Recursive helper to build the global DeclContext information and +/// gather the child->parent relationships in the original compile unit. +/// +/// \return true when this DIE and all of its children are only +/// forward declarations to types defined in external clang modules +/// (i.e., forward declarations that are children of a DW_TAG_module). +static bool analyzeContextInfo(const DWARFDebugInfoEntryMinimal *DIE, + unsigned ParentIdx, CompileUnit &CU, + DeclContext *CurrentDeclContext, + NonRelocatableStringpool &StringPool, + DeclContextTree &Contexts, + bool InTagModule = false) { unsigned MyIdx = CU.getOrigUnit().getDIEIndex(DIE); CompileUnit::DIEInfo &Info = CU.getInfo(MyIdx); + // Clang imposes an ODR on modules(!) regardless of the language: + // "The module-id should consist of only a single identifier, + // which provides the name of the module being defined. Each + // module shall have a single definition." + // + // This does not extend to the types inside the modules: + // "[I]n C, this implies that if two structs are defined in + // different submodules with the same name, those two types are + // distinct types (but may be compatible types if their + // definitions match)." + // + // We treat non-C++ modules like namespaces for this reason. + if (DIE->getTag() == dwarf::DW_TAG_module) + InTagModule = true; + Info.ParentIdx = ParentIdx; - if (CU.hasODR()) { + if (CU.hasODR() || CU.isClangModule() || InTagModule) { if (CurrentDeclContext) { auto PtrInvalidPair = Contexts.getChildDeclContext(*CurrentDeclContext, DIE, CU, StringPool); @@ -1759,11 +1795,21 @@ Info.Ctxt = CurrentDeclContext = nullptr; } + Info.Prune = InTagModule; if (DIE->hasChildren()) for (auto *Child = DIE->getFirstChild(); Child && !Child->isNULL(); Child = Child->getSibling()) - gatherDIEParents(Child, MyIdx, CU, CurrentDeclContext, StringPool, - Contexts); + Info.Prune &= analyzeContextInfo(Child, MyIdx, CU, CurrentDeclContext, + StringPool, Contexts, InTagModule); + + // Prune this DIE if it is either a forward declaration inside a + // DW_TAG_module or a DW_TAG_module that contains nothing but + // forward declarations. + Info.Prune &= (DIE->getTag() == dwarf::DW_TAG_module) || + DIE->getAttributeValueAsUnsignedConstant( + &CU.getOrigUnit(), dwarf::DW_AT_declaration, 0); + + return Info.Prune; } static bool dieNeedsChildrenToBeMeaningful(uint32_t Tag) { @@ -2174,6 +2220,8 @@ unsigned Idx = CU.getOrigUnit().getDIEIndex(&Die); CompileUnit::DIEInfo &MyInfo = CU.getInfo(Idx); bool AlreadyKept = MyInfo.Keep; + if (MyInfo.Prune && !AlreadyKept) + return; // If the Keep flag is set, we are marking a required DIE's // dependencies. If our target is already marked as kept, we're all @@ -2611,7 +2659,8 @@ Die = Info.Clone = DIE::get(DIEAlloc, dwarf::Tag(InputDIE.getTag())); assert(Die->getTag() == InputDIE.getTag()); Die->setOffset(OutOffset); - if (Unit.hasODR() && Die->getTag() != dwarf::DW_TAG_namespace && Info.Ctxt && + if ((Unit.hasODR() || Unit.isClangModule()) && + Die->getTag() != dwarf::DW_TAG_namespace && Info.Ctxt && Info.Ctxt != Unit.getInfo(Info.ParentIdx).Ctxt && !Info.Ctxt->getCanonicalDIEOffset()) { // We are about to emit a DIE that is the root of its own valid @@ -3103,6 +3152,13 @@ std::string PCMpath = CUDie.getAttributeValueAsString(&Unit, dwarf::DW_AT_comp_dir, ""); + std::string Name = + CUDie.getAttributeValueAsString(&Unit, dwarf::DW_AT_name, ""); + if (Name.empty()) { + reportWarning("Anonymous module skeleton CU for " + PCMfile); + return true; + } + if (Options.Verbose) { outs().indent(Indent); outs() << "Found clang module reference " << PCMfile; @@ -3119,7 +3175,7 @@ // Cyclic dependencies are disallowed by Clang, but we still // shouldn't run into an infinite loop, so mark it as processed now. ClangModules.insert(PCMfile); - loadClangModule(PCMfile, PCMpath, ModuleMap, Indent + 2); + loadClangModule(PCMfile, PCMpath, Name, ModuleMap, Indent + 2); return true; } @@ -3139,7 +3195,8 @@ } void DwarfLinker::loadClangModule(StringRef Filename, StringRef ModulePath, - DebugMap &ModuleMap, unsigned Indent) { + StringRef ModuleName, DebugMap &ModuleMap, + unsigned Indent) { SmallString<80> Path(Options.PrependPath); if (sys::path::is_relative(Filename)) sys::path::append(Path, ModulePath, Filename); @@ -3172,10 +3229,10 @@ << " 1 compile unit.\n"; exitDsymutil(1); } - Unit = new CompileUnit(*CU, UnitID++, !Options.NoODR); + Unit = new CompileUnit(*CU, UnitID++, !Options.NoODR, ModuleName); Unit->setHasInterestingContent(); - gatherDIEParents(CUDie, 0, *Unit, &ODRContexts.getRoot(), StringPool, - ODRContexts); + analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(), StringPool, + ODRContexts); // Keep everything. Unit->markEverythingAsKept(); } @@ -3270,9 +3327,9 @@ CUDie->dump(outs(), CU.get(), 0); } if (!registerModuleReference(*CUDie, *CU, ModuleMap)) { - Units.emplace_back(*CU, UnitID++, !Options.NoODR); - gatherDIEParents(CUDie, 0, Units.back(), &ODRContexts.getRoot(), - StringPool, ODRContexts); + Units.emplace_back(*CU, UnitID++, !Options.NoODR, ""); + analyzeContextInfo(CUDie, 0, Units.back(), &ODRContexts.getRoot(), + StringPool, ODRContexts); } }