[dsymutil] Correctly handle DW_TAG_label

[JDevlieghere]

Summary

This patch contains logic for handling DW_TAG_label that's present in darwin's dsymutil implementation, but not yet upstream.

[aprantl]

You should be able to generate a testcase using an assembler source.

tools/dsymutil/DwarfLinker.cpp

2496 ↗

(On Diff #134831)

put

[JDevlieghere]

Add test case

[aprantl]

Thanks!

llvm/test/tools/dsymutil/X86/label.test

+# RUN: llvm-dsymutil -oso-prepend-path %p/../Inputs -y %s -f -o - | llvm-dwarfdump - --debug-info | FileCheck %s
+
+# Compile with:
+#  echo -e ".global _foo;\nfoo:\nnop" | clang -x assembler -g - -c -o /tmp/label.o
+
+# CHECK: DW_TAG_label
+# CHECK-NEXT: DW_AT_name  ("foo")
+
+---
+triple:          'x86_64-apple-darwin'
+objects:
+  - filename: label.o
+    symbols:
+      - { sym: _foo, objAddr: 0x0, binAddr: 0x1000, size: 0x1 }
+...
+

llvm/tools/dsymutil/DwarfLinker.cpp

public:
   const DIEInfo &getInfo(unsigned Idx) const { return Info[Idx]; }
 
   uint64_t getStartOffset() const { return StartOffset; }
   uint64_t getNextUnitOffset() const { return NextUnitOffset; }
   void setStartOffset(uint64_t DebugInfoSize) { StartOffset = DebugInfoSize; }
 
   uint64_t getLowPc() const { return LowPc; }
   uint64_t getHighPc() const { return HighPc; }
+  bool hasLabelAt(uint64_t Addr) const { return Labels.count(Addr); }
 
   Optional<PatchLocation> getUnitRangesAttribute() const {
     return UnitRangeAttribute;
   }
 
   const FunctionIntervals &getFunctionRanges() const { return Ranges; }
 
   const std::vector<PatchLocation> &getRangesAttributes() const {
public:
   /// offset of \p Die in the debug_info section or to the canonical offset of
   /// \p Ctxt if it is non-null.
   void noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
                             DeclContext *Ctxt, PatchLocation Attr);
 
   /// Apply all fixups recored by noteForwardReference().
   void fixupForwardReferences();
 
+  /// Add the low_pc of a label that is relocatad by applying
+  /// offset \p PCOffset.
+  void addLabelLowPc(uint64_t LabelLowPc, int64_t PcOffset);
+
   /// Add a function range [\p LowPC, \p HighPC) that is relocatad by applying
   /// offset \p PCOffset.
   void addFunctionRange(uint64_t LowPC, uint64_t HighPC, int64_t PCOffset);
 
   /// Keep track of a DW_AT_range attribute that we will need to patch up later.
   void noteRangeAttribute(const DIE &Die, PatchLocation Attr);
 
   /// Keep track of a location attribute pointing to a location list in the
private:
 
   FunctionIntervals::Allocator RangeAlloc;
 
   /// The ranges in that interval map are the PC ranges for
   /// functions in this unit, associated with the PC offset to apply
   /// to the addresses to get the linked address.
   FunctionIntervals Ranges;
 
+  /// The DW_AT_low_pc of each DW_TAG_label.
+  SmallDenseMap<uint64_t, uint64_t, 1> Labels;
+
   /// DW_AT_ranges attributes to patch after we have gathered
   /// all the unit's function addresses.
   /// @{
   std::vector<PatchLocation> RangeAttributes;
   Optional<PatchLocation> UnitRangeAttribute;
   /// @}
 
   /// Location attributes that need to be transferred from the
for (const auto &Ref : ForwardDIEReferences) {
     std::tie(RefDie, RefUnit, Ctxt, Attr) = Ref;
     if (Ctxt && Ctxt->getCanonicalDIEOffset())
       Attr.set(Ctxt->getCanonicalDIEOffset());
     else
       Attr.set(RefDie->getOffset() + RefUnit->getStartOffset());
   }
 }
 
+void CompileUnit::addLabelLowPc(uint64_t LabelLowPc, int64_t PcOffset) {
+  Labels.insert({LabelLowPc, PcOffset});
+}
+
 void CompileUnit::addFunctionRange(uint64_t FuncLowPc, uint64_t FuncHighPc,
                                    int64_t PcOffset) {
   Ranges.insert(FuncLowPc, FuncHighPc, PcOffset);
   this->LowPc = std::min(LowPc, FuncLowPc + PcOffset);
   this->HighPc = std::max(HighPc, FuncHighPc + PcOffset);
 }
 
 void CompileUnit::noteRangeAttribute(const DIE &Die, PatchLocation Attr) {
unsigned DwarfLinker::shouldKeepSubprogramDIE(
 
   Flags |= TF_InFunctionScope;
 
   Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
   if (!LowPcIdx)
     return Flags;
 
   uint32_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
-  const DWARFUnit &OrigUnit = Unit.getOrigUnit();
+  DWARFUnit &OrigUnit = Unit.getOrigUnit();
   uint32_t LowPcOffset, LowPcEndOffset;
   std::tie(LowPcOffset, LowPcEndOffset) =
       getAttributeOffsets(Abbrev, *LowPcIdx, Offset, OrigUnit);
 
   auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
   assert(LowPc.hasValue() && "low_pc attribute is not an address.");
   if (!LowPc ||
       !RelocMgr.hasValidRelocation(LowPcOffset, LowPcEndOffset, MyInfo))
     return Flags;
 
   if (Options.Verbose) {
     DIDumpOptions DumpOpts;
     DumpOpts.RecurseDepth = 0;
     DumpOpts.Verbose = Options.Verbose;
     DIE.dump(outs(), 8 /* Indent */, DumpOpts);
   }
 
+  if (DIE.getTag() == dwarf::DW_TAG_label) {
+    if (Unit.hasLabelAt(*LowPc))
+      return Flags;
+    // FIXME: dsymutil-classic compat. dsymutil-classic doesn't consider labels
+    // that don't fall into the CU's aranges. This is wrong IMO. Debug info
+    // generation bugs aside, this is really wrong in the case of labels, where
+    // a label marking the end of a function will have a PC == CU's high_pc.
+    if (dwarf::toAddress(OrigUnit.getUnitDIE().find(dwarf::DW_AT_high_pc))
+          .getValueOr(UINT64_MAX) <= LowPc)
+      return Flags;
+    Unit.addLabelLowPc(*LowPc, MyInfo.AddrAdjust);
+    return Flags | TF_Keep;
+  }
+
   Flags |= TF_Keep;
 
   Optional<uint64_t> HighPc = DIE.getHighPC(*LowPc);
   if (!HighPc) {
     reportWarning("Function without high_pc. Range will be discarded.\n",
                   &DIE);
     return Flags;
   }
unsigned DwarfLinker::shouldKeepDIE(RelocationManager &RelocMgr,
                                     CompileUnit &Unit,
                                     CompileUnit::DIEInfo &MyInfo,
                                     unsigned Flags) {
   switch (DIE.getTag()) {
   case dwarf::DW_TAG_constant:
   case dwarf::DW_TAG_variable:
     return shouldKeepVariableDIE(RelocMgr, DIE, Unit, MyInfo, Flags);
   case dwarf::DW_TAG_subprogram:
+  case dwarf::DW_TAG_label:
     return shouldKeepSubprogramDIE(RelocMgr, DIE, Unit, MyInfo, Flags);
   case dwarf::DW_TAG_imported_module:
   case dwarf::DW_TAG_imported_declaration:
   case dwarf::DW_TAG_imported_unit:
     // We always want to keep these.
     return Flags | TF_Keep;
   default:
     break;