dsymutil: Follow references to clang modules and recursively clone the debug info

[aprantl]

Summary

This patch teaches llvm-dsymutil to follow references to clang modules and recursively clone the debug info therein.
This does not yet resolve external type references.

[aprantl]

Got rid of ModuleDIEAlloc.

[friss]

test/tools/dsymutil/X86/modules.map

1–8 ↗

(On Diff #35327)

The name of that file is a bit confusing because of the context it addresses. I'd like to see the actual module map file committed, and if you could rework that test to look like the other newer ones (and thus get rid of this map in favor of dummy-debug-map.map), this would be great.

test/tools/dsymutil/X86/modules.test

3 ↗

(On Diff #35327)

Is that copying really necessary?

4–5 ↗

(On Diff #35327)

I usually ditch the tmp file in favor of -o - | llvm-dwarfdump

tools/dsymutil/DwarfLinker.cpp

395

I think that for your work, this line would be sufficient in this loop. The rest will be needed for --update support, but I need to first figure out accelerator tables.

1114–1115

Can you do that refactoring in a preparatory NFC commit?

1452

StringSet?

1471–1473

Making that a static helper could be a separate NFC commit too, right?

2625–2631

Took me some time to remember why we need that (or rather why we didn't need it before). Usually, the last DIE is always a NULL. It can happen that the input file is just a single compile_unit DIE though and that would crash here because we don't have a terminating NULL. A comment explaining that we should do that optimization would be nice.

3093–3095

It might help the reader if there was a comment describing the layout of debug info containing a module reference so that it's trivial to understand how this function works.

3148

this should be sys::TimeValue::PosixZeroTime().

3162

That condition seems unlikely to trigger.

tools/dsymutil/dsymutil.h

31 ↗

(On Diff #35327)

Separate NFC patch?

[aprantl]

tools/dsymutil/DwarfLinker.cpp

1114–1115

248310

1471–1473

248311

tools/dsymutil/dsymutil.h

31 ↗

(On Diff #35327)

248312

[aprantl]

Addressed remaining review feedback. Thanks!

[friss]

LGTM once you address one nit:

tools/dsymutil/DwarfLinker.cpp

2625

The next DIE isn't usually NULL, but it is usually valid because there's nearly always at least a NULL after it.

[aprantl]

Thanks, r248331.

test/tools/dsymutil/X86/lit.local.cfg

 if not 'X86' in config.root.targets:
     config.unsupported = True
 
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.m', '.s']

test/tools/dsymutil/X86/modules.m

+/* Compile with:
+   cat >modules.modulemap <<EOF
+     module Foo {
+       header "Foo.h"
+       export *
+     }
+     module Bar {
+       header "Bar.h"
+       export *
+     }
+   EOF
+   clang -D BAR_H -E -o Bar.h
+   clang -D FOO_H -E -o Foo.h
+   clang -cc1 -emit-obj -fmodules -fmodule-map-file=modules.modulemap \
+     -fmodule-format=obj -g -dwarf-ext-refs -fmodules-cache-path=. \
+     -fdisable-module-hash modules.m -o 1.o
+*/
+
+// RUN: llvm-dsymutil -f -oso-prepend-path=%p/../Inputs/modules \
+// RUN:   -y %p/dummy-debug-map.map -o - \
+// RUN:     | llvm-dwarfdump --debug-dump=info - | FileCheck %s
+
+// ---------------------------------------------------------------------
+#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"
+
+struct Bar {
+  int value;
+};
+
+#else
+// ---------------------------------------------------------------------
+#ifdef FOO_H
+// ---------------------------------------------------------------------
+// CHECK: 55{{.*}}DW_TAG_compile_unit
+// CHECK:   DW_TAG_module
+// CHECK-NEXT: DW_AT_name {{.*}}"Foo"
+// CHECK:      DW_TAG_typedef
+@import Bar;
+typedef struct Bar Bar;
+struct S {};
+
+// ---------------------------------------------------------------------
+#else
+// ---------------------------------------------------------------------
+
+// CHECK: DW_TAG_compile_unit
+// CHECK:   DW_TAG_subprogram
+// CHECK:     DW_AT_name {{.*}}"main"
+@import Foo;
+int main(int argc, char **argv) {
+  Bar bar;
+  bar.value = 42;
+  return bar.value;
+}
+#endif
+#endif

tools/dsymutil/DwarfLinker.cpp

 //===- tools/dsymutil/DwarfLinker.cpp - Dwarf debug info linker -----------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "DebugMap.h"
 #include "BinaryHolder.h"
 #include "DebugMap.h"
 #include "dsymutil.h"
 #include "MachOUtils.h"
 #include "NonRelocatableStringpool.h"
 #include "llvm/ADT/IntervalMap.h"
-#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/DIE.h"
 #include "llvm/Config/config.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h"
 #include "llvm/DebugInfo/DWARF/DWARFFormValue.h"
 #include "llvm/MC/MCAsmBackend.h"
const std::vector<PatchLocation> &getRangesAttributes() const {
     return RangeAttributes;
   }
 
   const std::vector<std::pair<PatchLocation, int64_t>> &
   getLocationAttributes() const {
     return LocationAttributes;
   }
 
+  void setHasInterestingContent() { HasInterestingContent = true; }
+  bool hasInterestingContent() { return HasInterestingContent; }
+
+  /// Mark every DIE in this unit as kept. This function also
+  /// marks variables as InDebugMap so that they appear in the
+  /// reconstructed accelerator tables.
+  void markEverythingAsKept();
+
   /// \brief Compute the end offset for this unit. Must be
   /// called after the CU's DIEs have been cloned.
   /// \returns the next unit offset (which is also the current
   /// debug_info section size).
   uint64_t computeNextUnitOffset();
 
   /// \brief Keep track of a forward reference to DIE \p Die in \p
   /// RefUnit by \p Attr. The attribute should be fixed up later to
private:
   std::vector<AccelInfo> Pubtypes;
   /// @}
 
   /// Cached resolved paths from the line table.
   std::vector<std::string> ResolvedPaths;
 
   /// Is this unit subject to the ODR rule?
   bool HasODR;
+  /// Did a DIE actually contain a valid reloc?
+  bool HasInterestingContent;
 };
 
+void CompileUnit::markEverythingAsKept() {
+  for (auto &I : Info)
+    I.Keep = true;
+}
+
 uint64_t CompileUnit::computeNextUnitOffset() {
   NextUnitOffset = StartOffset + 11 /* Header size */;
   // The root DIE might be null, meaning that the Unit had nothing to
   // contribute to the linked output. In that case, we will emit the
   // unit header without any actual DIE.
   if (CUDie)
     NextUnitOffset += CUDie->getSize();
   return NextUnitOffset;
 }
 
 /// \brief Keep track of a forward cross-cu reference from this unit
 /// to \p Die that lives in \p RefUnit.
 void CompileUnit::noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
                                        DeclContext *Ctxt, PatchLocation Attr) {
   ForwardDIEReferences.emplace_back(Die, RefUnit, Ctxt, Attr);
 }
public:
   ~DwarfLinker() {
     for (auto *Abbrev : Abbreviations)
       delete Abbrev;
   }
 
   /// \brief Link the contents of the DebugMap.
   bool link(const DebugMap &);
 
   void reportWarning(const Twine &Warning, const DWARFUnit *Unit = nullptr,
                      const DWARFDebugInfoEntryMinimal *DIE = nullptr) const;
 
 private:
   /// \brief Called at the start of a debug object link.
   void startDebugObject(DWARFContext &, DebugMapObject &);
 
   /// \brief Called at the end of a debug object link.
   void endDebugObject();
 
private:
   /// @{
   /// \brief Recursively walk the \p DIE tree and look for DIEs to
   /// keep. Store that information in \p CU's DIEInfo.
   void lookForDIEsToKeep(RelocationManager &RelocMgr,
                          const DWARFDebugInfoEntryMinimal &DIE,
                          const DebugMapObject &DMO, CompileUnit &CU,
                          unsigned Flags);
 
+  /// If this compile unit is really a skeleton CU that points to a
+  /// clang module, register it in ClangModules and return true.
+  ///
+  /// A skeleton CU is a CU without children, a DW_AT_gnu_dwo_name
+  /// pointing to the module, and a DW_AT_gnu_dwo_id with the module
+  /// hash.
+  bool registerModuleReference(const DWARFDebugInfoEntryMinimal &CUDie,
+                               const DWARFUnit &Unit, DebugMap &ModuleMap,
+                               unsigned Indent = 0);
+
+  /// Recursively add the debug info in this clang module .pcm
+  /// 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);
+
   /// \brief Flags passed to DwarfLinker::lookForDIEsToKeep
   enum TravesalFlags {
     TF_Keep = 1 << 0,            ///< Mark the traversed DIEs as kept.
     TF_InFunctionScope = 1 << 1, ///< Current scope is a fucntion scope.
     TF_DependencyWalk = 1 << 2,  ///< Walking the dependencies of a kept DIE.
     TF_ParentWalk = 1 << 3,      ///< Walking up the parents of a kept DIE.
     TF_ODR = 1 << 4,             ///< Use the ODR whhile keeping dependants.
     TF_SkipPC = 1 << 5,          ///< Skip all location attributes.
private:
   bool createStreamer(Triple TheTriple, StringRef OutputFilename);
 
   /// \brief Attempt to load a debug object from disk.
   ErrorOr<const object::ObjectFile &> loadObject(BinaryHolder &BinaryHolder,
                                                  DebugMapObject &Obj,
                                                  const DebugMap &Map);
   /// @}
 
-private:
   std::string OutputFilename;
   LinkOptions Options;
   BinaryHolder BinHolder;
   std::unique_ptr<DwarfStreamer> Streamer;
   uint64_t OutputDebugInfoSize;
+  unsigned UnitID; ///< A unique ID that identifies each compile unit.
 
   /// The units of the current debug map object.
   std::vector<CompileUnit> Units;
 
   /// The debug map object curently under consideration.
   DebugMapObject *CurrentDebugObject;
 
   /// \brief The Dwarf string pool
private:
   /// section. The actual CIE data serves a the key to this StringMap,
   /// this takes care of comparing the semantics of CIEs defined in
   /// different object files.
   StringMap<uint32_t> EmittedCIEs;
 
   /// Offset of the last CIE that has been emitted in the output
   /// debug_frame section.
   uint32_t LastCIEOffset;
+
+  /// FIXME: We may need to use something more resilient than the PCM filename.
+  StringSet<> ClangModules;
 };
 
 /// Similar to DWARFUnitSection::getUnitForOffset(), but returning our
 /// CompileUnit object instead.
 static CompileUnit *getUnitForOffset(MutableArrayRef<CompileUnit> Units,
                                      unsigned Offset) {
   auto CU =
       std::upper_bound(Units.begin(), Units.end(), Offset,
                        [](uint32_t LHS, const CompileUnit &RHS) {
                          return LHS < RHS.getOrigUnit().getNextUnitOffset();
                        });
   return CU != Units.end() ? &*CU : nullptr;
 }
 
 /// Resolve the DIE attribute reference that has been
 /// extracted in \p RefValue. The resulting DIE migh be in another
 /// CompileUnit which is stored into \p ReferencedCU.
 /// \returns null if resolving fails for any reason.
 static const DWARFDebugInfoEntryMinimal *resolveDIEReference(
     const DwarfLinker &Linker, MutableArrayRef<CompileUnit> Units,
     const DWARFFormValue &RefValue, const DWARFUnit &Unit,
     const DWARFDebugInfoEntryMinimal &DIE, CompileUnit *&RefCU) {
   assert(RefValue.isFormClass(DWARFFormValue::FC_Reference));
   uint64_t RefOffset = *RefValue.getAsReference(&Unit);
 
   if ((RefCU = getUnitForOffset(Units, RefOffset)))
     if (const auto *RefDie = RefCU->getOrigUnit().getDIEForOffset(RefOffset))
       return RefDie;
 
if (Unit.hasODR() && Die->getTag() != dwarf::DW_TAG_namespace && Info.Ctxt &&
     // We are about to emit a DIE that is the root of its own valid
     // DeclContext tree. Make the current offset the canonical offset
     // for this context.
     Info.Ctxt->setCanonicalDIEOffset(OutOffset + Unit.getStartOffset());
   }
 
   // Extract and clone every attribute.
   DataExtractor Data = U.getDebugInfoExtractor();
-  uint32_t NextOffset = U.getDIEAtIndex(Idx + 1)->getOffset();
+  // Point to the next DIE (usually NULL). If this is a lone
+  // DW_TAG_compile_unit without any children, point to the next unit.
+  uint32_t NextOffset =
+    (Idx + 1 < U.getNumDIEs())
+    ? U.getDIEAtIndex(Idx + 1)->getOffset()
+    : U.getNextUnitOffset();
   AttributesInfo AttrInfo;
 
   // We could copy the data only if we need to aply a relocation to
   // it. After testing, it seems there is no performance downside to
   // doing the copy unconditionally, and it makes the code simpler.
   SmallString<40> DIECopy(Data.getData().substr(Offset, NextOffset - Offset));
   Data = DataExtractor(DIECopy, Data.isLittleEndian(), Data.getAddressSize());
   // Modify the copy with relocated addresses.
   if (RelocMgr.applyValidRelocs(DIECopy, Offset, Data.isLittleEndian())) {
for (const auto &Attr : Abbrev.attributes()) {
     if (hasODR && isODRAttribute(Attr.Attr))
       Form = dwarf::DW_FORM_ref_addr;
     Copy.AddAttribute(dwarf::Attribute(Attr.Attr), dwarf::Form(Form));
   }
 
   Linker.AssignAbbrev(Copy);
 }
 
+bool DwarfLinker::registerModuleReference(
+    const DWARFDebugInfoEntryMinimal &CUDie, const DWARFUnit &Unit,
+    DebugMap &ModuleMap, unsigned Indent) {
+  std::string PCMfile =
+      CUDie.getAttributeValueAsString(&Unit, dwarf::DW_AT_GNU_dwo_name, "");
+  if (PCMfile.empty())
+    return false;
+
+  // Clang module DWARF skeleton CUs abuse this for the path to the module.
+  std::string PCMpath =
+      CUDie.getAttributeValueAsString(&Unit, dwarf::DW_AT_comp_dir, "");
+
+  if (Options.Verbose) {
+    outs().indent(Indent);
+    outs() << "Found clang module reference " << PCMfile;
+  }
+
+  if (ClangModules.count(PCMfile)) {
+    if (Options.Verbose)
+      outs() << " [cached].\n";
+    return true;
+  }
+  if (Options.Verbose)
+    outs() << " ...\n";
+
+  // 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);
+  return true;
+}
+
 ErrorOr<const object::ObjectFile &>
 DwarfLinker::loadObject(BinaryHolder &BinaryHolder, DebugMapObject &Obj,
                         const DebugMap &Map) {
   auto ErrOrObjs =
       BinaryHolder.GetObjectFiles(Obj.getObjectFilename(), Obj.getTimestamp());
   if (std::error_code EC = ErrOrObjs.getError()) {
     reportWarning(Twine(Obj.getObjectFilename()) + ": " + EC.message());
     return EC;
   }
   auto ErrOrObj = BinaryHolder.Get(Map.getTriple());
   if (std::error_code EC = ErrOrObj.getError())
     reportWarning(Twine(Obj.getObjectFilename()) + ": " + EC.message());
   return ErrOrObj;
 }
 
+void DwarfLinker::loadClangModule(StringRef Filename, StringRef ModulePath,
+                                  DebugMap &ModuleMap, unsigned Indent) {
+  SmallString<80> Path(Options.PrependPath);
+  if (sys::path::is_relative(Filename))
+    sys::path::append(Path, ModulePath, Filename);
+  else
+    sys::path::append(Path, Filename);
+  BinaryHolder ObjHolder(Options.Verbose);
+  auto &Obj =
+      ModuleMap.addDebugMapObject(Path, sys::TimeValue::PosixZeroTime());
+  auto ErrOrObj = loadObject(ObjHolder, Obj, ModuleMap);
+  if (!ErrOrObj) {
+    ClangModules.erase(ClangModules.find(Filename));
+    return;
+  }
+
+  // FIXME: At this point dsymutil should verify the DW_AT_gnu_dwo_id
+  // against the module hash of the clang module.
+
+  CompileUnit *Unit = nullptr;
+
+  // Setup access to the debug info.
+  DWARFContextInMemory DwarfContext(*ErrOrObj);
+  RelocationManager RelocMgr(*this);
+  for (const auto &CU : DwarfContext.compile_units()) {
+    auto *CUDie = CU->getUnitDIE(false);
+    // Recursively get all modules imported by this one.
+    if (!registerModuleReference(*CUDie, *CU, ModuleMap, Indent)) {
+      // Add this module.
+      if (Unit) {
+        errs() << Filename << ": Clang modules are expected to have exactly"
+               << " 1 compile unit.\n";
+        exitDsymutil(1);
+      }
+      Unit = new CompileUnit(*CU, UnitID++, !Options.NoODR);
+      Unit->setHasInterestingContent();
+      gatherDIEParents(CUDie, 0, *Unit, &ODRContexts.getRoot(), StringPool,
+                       ODRContexts);
+      // Keep everything.
+      Unit->markEverythingAsKept();
+    }
+  }
+  if (Options.Verbose) {
+    outs().indent(Indent);
+    outs() << "cloning .debug_info from " << Filename << "\n";
+  }
+
+  DIECloner(*this, RelocMgr, DIEAlloc, MutableArrayRef<CompileUnit>(*Unit),
+            Options)
+      .cloneAllCompileUnits(DwarfContext);
+}
+
 void DwarfLinker::DIECloner::cloneAllCompileUnits(
     DWARFContextInMemory &DwarfContext) {
   if (!Linker.Streamer)
     return;
 
   for (auto &CurrentUnit : CompileUnits) {
     const auto *InputDIE = CurrentUnit.getOrigUnit().getUnitDIE();
     CurrentUnit.setStartOffset(Linker.OutputDebugInfoSize);

 bool DwarfLinker::link(const DebugMap &Map) {
 
   if (!createStreamer(Map.getTriple(), OutputFilename))
     return false;
 
   // Size of the DIEs (and headers) generated for the linked output.
   OutputDebugInfoSize = 0;
   // A unique ID that identifies each compile unit.
-  unsigned UnitID = 0;
+  UnitID = 0;
+  DebugMap ModuleMap(Map.getTriple(), Map.getBinaryPath());
+
   for (const auto &Obj : Map.objects()) {
     CurrentDebugObject = Obj.get();
 
     if (Options.Verbose)
       outs() << "DEBUG MAP OBJECT: " << Obj->getObjectFilename() << "\n";
     auto ErrOrObj = loadObject(BinHolder, *Obj, Map);
     if (!ErrOrObj)
       continue;
for (const auto &Obj : Map.objects()) {
     // In a first phase, just read in the debug info and store the DIE
     // parent links that we will use during the next phase.
     for (const auto &CU : DwarfContext.compile_units()) {
       auto *CUDie = CU->getUnitDIE(false);
       if (Options.Verbose) {
         outs() << "Input compilation unit:";
         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);
       }
+    }
 
     // Then mark all the DIEs that need to be present in the linked
     // output and collect some information about them. Note that this
     // loop can not be merged with the previous one becaue cross-cu
     // references require the ParentIdx to be setup for every CU in
     // the object file before calling this.
     for (auto &CurrentUnit : Units)
       lookForDIEsToKeep(RelocMgr, *CurrentUnit.getOrigUnit().getUnitDIE(), *Obj,