[dsymutil] Don't clone empty CUs

[JDevlieghere]

Summary

The DWARF standard says that an empty compile unit is not valid:

Each such contribution consists of a compilation unit header (see Section 7.5.1.1 on page 200) followed by a single DW_TAG_compile_unit or DW_TAG_partial_unit debugging information entry, together with its children.

Therefore we shouldn't clone them in dsymutil.

[friss]

I might be missing something, but empty as in "has no DIEs at all" is invalid. I don't think having one DIE with no children counts as invalid (it might not be really useful, but this review is about it being invalid, right?)

[aprantl]

You mean an empty compile unit *header* is not valid, right?

[JDevlieghere]

Only skip empty CUs, not DW_TAG_compile_units without children.

[aprantl]

llvm/tools/dsymutil/CompileUnit.cpp

60 ↗

(On Diff #186070)

@friss Do you remember why that comment is there? Isn't a header without a DW_TAG_compile_unit invalid DWARF?

llvm/tools/dsymutil/DwarfLinker.cpp

2274 ↗

(On Diff #186070)

This change looks reasonable.

[dblaikie]

llvm/test/tools/dsymutil/X86/generate-empty-CU.test

5 ↗

(On Diff #186070)

This sentence seems incomplete ("Dsymutil should not generate an empty CU and it", "and it" what?)

[friss]

llvm/tools/dsymutil/CompileUnit.cpp

60 ↗

(On Diff #186070)

Must have been because dsymutil-classic was doing it. I don't recall any good reason we would generate this.

[aprantl]

llvm/tools/dsymutil/DwarfLinker.cpp

2259 ↗

(On Diff #186070)

This looks like an unrelated NFC commit?

llvm/trunk/test/tools/dsymutil/X86/empty-CU.test

 RUN: dsymutil --update -f %p/../Inputs/empty-CU.o -o - | llvm-dwarfdump -v - -debug-info | FileCheck %s
 
 CHECK: .debug_info contents:
 CHECK: 0x00000000: Compile Unit: length = 0x00000008 version = 0x0003 abbr_offset = 0x0000 addr_size = 0x04 (next unit at 0x0000000c)
 
 CHECK: 0x0000000b: DW_TAG_compile_unit [1]
-CHECK: 0x0000000c: Compile Unit: length = 0x00000007 version = 0x0003 abbr_offset = 0x0000 addr_size = 0x04 (next unit at 0x00000017)

llvm/trunk/test/tools/dsymutil/X86/generate-empty-CU.test

 # RUN: dsymutil -f -o - -oso-prepend-path=%p/.. -y %s | llvm-dwarfdump -v - | FileCheck %s
 
 # This test on links the Dwarf for an LTO binary and on purpose doesn't retain
-# any symbol in the second CU out of 3. This is the only case where dsymutil
-# will generate an empty CU and it requires special handling.
+# any symbol in the second CU out of 3. To be valid DWARF ssymutil must not
+# generate an empty CU but omit it.
 
 ---
 triple:          'x86_64-apple-darwin'
 objects:
   - filename:        /Inputs/basic-lto.macho.x86_64.o
     timestamp:       1417654896
     symbols:
       - { sym: _main, objAddr: 0x0000000000000000, binAddr: 0x0000000100000F40, size: 0x00000010 }
       - { sym: _bar, objAddr: 0x0000000000000050, binAddr: 0x0000000100000F90, size: 0x00000024 }
 ...
 
 .debug_info contents:
 CHECK: Compile Unit: length = 0x0000007d version = 0x0002 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x00000081)
 
 CHECK: DW_TAG_compile_unit
 CHECK:        DW_AT_name {{.*}} "basic1.c"
 CHECK:   DW_TAG_subprogram
                 DW_AT_name {{.*}} "main"
 
-CHECK: Compile Unit: length = 0x00000007 version = 0x0002 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x0000008c)
-
-CHECK: Compile Unit: length = 0x00000089 version = 0x0002 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x00000119)
+CHECK: 0x00000081: Compile Unit: length = 0x00000089 version = 0x0002 abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x0000010e)
 
 CHECK: DW_TAG_compile_unit
 CHECK:        DW_AT_name {{.*}} "basic3.c"
 
 CHECK:   DW_TAG_subprogram [7] *
 CHECK:          DW_AT_name {{.*}} = "bar"

llvm/trunk/tools/dsymutil/CompileUnit.cpp

if (auto Block = Value->getAsBlock()) {
       if (Block->size() > OrigUnit.getAddressByteSize() &&
           (*Block)[0] == dwarf::DW_OP_addr)
         I.InDebugMap = 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 (NewUnit)
+  NextUnitOffset = StartOffset;
+  if (NewUnit) {
+    NextUnitOffset += 11 /* Header size */;
     NextUnitOffset += NewUnit->getUnitDie().getSize();
+  }
   return NextUnitOffset;
 }
 
 /// 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);

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

void DwarfLinker::DIECloner::cloneAllCompileUnits(
   if (Linker.Options.NoOutput)
     return;
 
   // Emit all the compile unit's debug information.
   for (auto &CurrentUnit : CompileUnits) {
     if (LLVM_LIKELY(!Linker.Options.Update))
       Linker.generateUnitRanges(*CurrentUnit);
     CurrentUnit->fixupForwardReferences();
+    if (CurrentUnit->getOutputUnitDIE()) {
       Linker.Streamer->emitCompileUnitHeader(*CurrentUnit);
-    if (!CurrentUnit->getOutputUnitDIE())
-      continue;
       Linker.Streamer->emitDIE(*CurrentUnit->getOutputUnitDIE());
     }
   }
+}
 
 void DwarfLinker::updateAccelKind(DWARFContext &Dwarf) {
   if (Options.TheAccelTableKind != AccelTableKind::Default)
     return;
 
   auto &DwarfObj = Dwarf.getDWARFObj();
 
   if (!AtLeastOneDwarfAccelTable &&