Make a function to correctly extract the DW_AT_high_pc given the low pc value.

[clayborg]

Summary

DWARF 4 and later supports encoding the PC as an address or as as offset from the low PC. Clients using DWARFDie should be insulated from how to extract the high PC value. This function takes care of extracting the form value and looking for the correct form.

[davide]

is there a way to test this?

include/llvm/DebugInfo/DWARF/DWARFDie.h

285

The typo fix may be committed separately, maybe?

[aprantl]

lib/DebugInfo/DWARF/DWARFDie.cpp

266

should the update be atomically (ie. only update if highpc is available, too)?

tools/dsymutil/DwarfLinker.cpp

2145

I think MSVC now also understands
Ranges[*LowPc] = { *HighPc, MyInfo.AddrAdjust };

[clayborg]

I will remove the typo fix and move the assignment of the low PC into the if.

include/llvm/DebugInfo/DWARF/DWARFDie.h

285

Yeah, I can remove this.

lib/DebugInfo/DWARF/DWARFDie.cpp

266

I can move the assignment into the if statement below.

[clayborg]

I can add a test as well.

[clayborg]

Addressed all comments and added a full suite of low/high PC unit tests.

[probinson]

include/llvm/DebugInfo/DWARF/DWARFDie.h

288

Comment should end with a period.

unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

796 ↗

(On Diff #81976)

Comment should end with a period. Also 'grabs' not 'grab'.

[aprantl]

LGTM after addressing all outstanding issues. Thanks!

include/llvm/DebugInfo/DWARF/DWARFDie.h

public:
   /// DWARFDie object if it doesn't.
   DWARFDie getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const;
 
   /// Extract the range base attribute from this DIE as absolute section offset.
   ///
   /// This is a utility function that checks for either the DW_AT_rnglists_base
   /// or DW_AT_GNU_ranges_base attribute.
   ///
-  /// \returns anm optional absolute section offset value for the attribute.
+  /// \returns an optional absolute section offset value for the attribute.
   Optional<uint64_t> getRangesBaseAttribute() const;
   
+  /// Get the DW_AT_high_pc attribute value as an address
+  ///
+  /// In DWARF version 4 and later the high PC can be encoded as an offset from
+  /// the DW_AT_low_pc. This function takes care of extracting the value as an
+  /// address or offset and adds it to the low PC if needed and returns the
+  /// value as an optional in case the DIE doesn't have a DW_AT_high_pc
+  /// attribute.
+  ///
+  /// \param LowPC the low PC that might be needed to calculate the high PC.
+  /// \returns an optional address value for the attribute.
+  Optional<uint64_t> getHighPC(uint64_t LowPC) const;
+
   /// Retrieves DW_AT_low_pc and DW_AT_high_pc from CU.
   /// Returns true if both attributes are present.
   bool getLowAndHighPC(uint64_t &LowPC, uint64_t &HighPC) const;
   
   /// Get the address ranges for this DIE.
   ///
   /// Get the hi/low PC range if both attributes are available or exrtracts the
   /// non-contiguous address ranges from the DW_AT_ranges attribute.

lib/DebugInfo/DWARF/DWARFDie.cpp

 Optional<uint64_t>
 DWARFDie::getRangesBaseAttribute() const {
   auto Result = getAttributeValueAsSectionOffset(DW_AT_rnglists_base);
   if (Result)
     return Result;
   return getAttributeValueAsSectionOffset(DW_AT_GNU_ranges_base);
 }
 
-bool DWARFDie::getLowAndHighPC(uint64_t &LowPC, uint64_t &HighPC) const {
-  if (auto LowPCVal = getAttributeValueAsAddress(DW_AT_low_pc))
-    LowPC = *LowPCVal;
-  else
-    return false;
-
-  if (auto HighPCVal = getAttributeValueAsAddress(DW_AT_high_pc)) {
+Optional<uint64_t> DWARFDie::getHighPC(uint64_t LowPC) const {
+  if (auto FormValue = getAttributeValue(DW_AT_high_pc)) {
+    if (auto Address = FormValue->getAsAddress()) {
       // High PC is an address.
-    HighPC = *HighPCVal;
-  } else if (auto Offset = getAttributeValueAsUnsignedConstant(DW_AT_high_pc)) {
+      return Address;
+    }
+    if (auto Offset = FormValue->getAsUnsignedConstant()) {
       // High PC is an offset from LowPC.
-    HighPC = LowPC + *Offset;
-  } else
+      return LowPC + *Offset;
+    }
+  }
+  return None;
+}
+
+bool DWARFDie::getLowAndHighPC(uint64_t &LowPC, uint64_t &HighPC) const {
+  auto LowPcAddr = getAttributeValueAsAddress(DW_AT_low_pc);
+  if (!LowPcAddr)
     return false;
+  LowPC = *LowPcAddr;
+  if (auto HighPcAddr = getHighPC(LowPC)) {
+    HighPC = *HighPcAddr;
     return true;
   }
+  return false;
+}
 
 DWARFAddressRangesVector
 DWARFDie::getAddressRanges() const {
   if (isNULL())
     return DWARFAddressRangesVector();
   // Single range specified by low/high PC.
   uint64_t LowPC, HighPC;
   if (getLowAndHighPC(LowPC, HighPC)) {

tools/dsymutil/DwarfLinker.cpp

if (!LowPc ||
       !RelocMgr.hasValidRelocation(LowPcOffset, LowPcEndOffset, MyInfo))
     return Flags;
 
   if (Options.Verbose)
     DIE.dump(outs(), 0, 8 /* Indent */);
 
   Flags |= TF_Keep;
 
-  Optional<DWARFFormValue> HighPcValue;
-  if (!(HighPcValue = DIE.getAttributeValue(dwarf::DW_AT_high_pc))) {
+  Optional<uint64_t> HighPc = DIE.getHighPC(*LowPc);
+  if (!HighPc) {
     reportWarning("Function without high_pc. Range will be discarded.\n",
                   &DIE);
     return Flags;
   }
 
-  uint64_t HighPc;
-  if (HighPcValue->isFormClass(DWARFFormValue::FC_Address)) {
-    HighPc = *HighPcValue->getAsAddress();
-  } else {
-    assert(HighPcValue->isFormClass(DWARFFormValue::FC_Constant));
-    HighPc = *LowPc + *HighPcValue->getAsUnsignedConstant();
-  }
-
   // Replace the debug map range with a more accurate one.
-  Ranges[*LowPc] = std::make_pair(HighPc, MyInfo.AddrAdjust);
-  Unit.addFunctionRange(*LowPc, HighPc, MyInfo.AddrAdjust);
+  Ranges[*LowPc] = std::make_pair(*HighPc, MyInfo.AddrAdjust);
+  Unit.addFunctionRange(*LowPc, *HighPc, MyInfo.AddrAdjust);
   return Flags;
 }
 
 /// \brief Check if a DIE should be kept.
 /// \returns updated TraversalFlags.
 unsigned DwarfLinker::shouldKeepDIE(RelocationManager &RelocMgr,
                                     const DWARFDie &DIE,
                                     CompileUnit &Unit,