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–271

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

tools/dsymutil/DwarfLinker.cpp

2145–2146

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–271

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

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.
   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;
+  if (auto HighPcAddr = getHighPC(*LowPcAddr)) {
+    LowPC = *LowPcAddr;
+    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,

unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

 
 TEST(DWARFDebugInfo, TestDWARF32Version4Addr8References) {
   // Test that we can decode all forms for DWARF32, version 4, with 8 byte
   // addresses.
   typedef uint64_t AddrType;
   TestReferences<4, AddrType>();
 }
 
+template <uint16_t Version, class AddrType> void TestAddresses() {
+  // Test DWARF API that grab the DW_AT_high_pc
+  const uint8_t AddrSize = sizeof(AddrType);
+  const bool SupportsHighPCAsOffset = Version >= 4;
+  initLLVMIfNeeded();
+  Triple Triple = getHostTripleForAddrSize(AddrSize);
+  auto ExpectedDG = dwarfgen::Generator::create(Triple, Version);
+  if (HandleExpectedError(ExpectedDG))
+    return;
+  dwarfgen::Generator *DG = ExpectedDG.get().get();
+  dwarfgen::CompileUnit &CU = DG->addCompileUnit();
+  dwarfgen::DIE CUDie = CU.getUnitDIE();
+  
+  CUDie.addAttribute(DW_AT_name, DW_FORM_strp, "/tmp/main.c");
+  CUDie.addAttribute(DW_AT_language, DW_FORM_data2, DW_LANG_C);
+  
+  // Create a subprogram DIE with no low or high PC.
+  dwarfgen::DIE SubprogramNoPC = CUDie.addChild(DW_TAG_subprogram);
+  SubprogramNoPC.addAttribute(DW_AT_name, DW_FORM_strp, "no_pc");
+
+  // Create a subprogram DIE with a low PC only.
+  dwarfgen::DIE SubprogramLowPC = CUDie.addChild(DW_TAG_subprogram);
+  SubprogramLowPC.addAttribute(DW_AT_name, DW_FORM_strp, "low_pc");
+  const uint64_t ActualLowPC = 0x1000;
+  const uint64_t ActualHighPC = 0x2000;
+  const uint64_t ActualHighPCOffset = ActualHighPC - ActualLowPC;
+  SubprogramLowPC.addAttribute(DW_AT_low_pc, DW_FORM_addr, ActualLowPC);
+
+  // Create a subprogram DIE with a low and high PC.
+  dwarfgen::DIE SubprogramLowHighPC = CUDie.addChild(DW_TAG_subprogram);
+  SubprogramLowHighPC.addAttribute(DW_AT_name, DW_FORM_strp, "low_high_pc");
+  SubprogramLowHighPC.addAttribute(DW_AT_low_pc, DW_FORM_addr, ActualLowPC);
+  // Encode the high PC as an offset from the low PC if supported.
+  if (SupportsHighPCAsOffset)
+    SubprogramLowHighPC.addAttribute(DW_AT_high_pc, DW_FORM_data4,
+                                     ActualHighPCOffset);
+  else
+    SubprogramLowHighPC.addAttribute(DW_AT_high_pc, DW_FORM_addr, ActualHighPC);
+  
+  StringRef FileBytes = DG->generate();
+  MemoryBufferRef FileBuffer(FileBytes, "dwarf");
+  auto Obj = object::ObjectFile::createObjectFile(FileBuffer);
+  EXPECT_TRUE((bool)Obj);
+  DWARFContextInMemory DwarfContext(*Obj.get());
+  
+  // Verify the number of compile units is correct.
+  uint32_t NumCUs = DwarfContext.getNumCompileUnits();
+  EXPECT_EQ(NumCUs, 1u);
+  DWARFCompileUnit *U = DwarfContext.getCompileUnitAtIndex(0);
+  
+  // Get the compile unit DIE is valid.
+  auto DieDG = U->getUnitDIE(false);
+  EXPECT_TRUE(DieDG.isValid());
+  // DieDG.dump(llvm::outs(), U, UINT32_MAX);
+  
+  uint64_t LowPC, HighPC;
+  Optional<uint64_t> OptU64;
+  // Verify the that our subprogram with no PC value fails appropriately when
+  // asked for any PC values.
+  auto SubprogramDieNoPC = DieDG.getFirstChild();
+  EXPECT_TRUE(SubprogramDieNoPC.isValid());
+  EXPECT_EQ(SubprogramDieNoPC.getTag(), DW_TAG_subprogram);
+  OptU64 = SubprogramDieNoPC.getAttributeValueAsAddress(DW_AT_low_pc);
+  EXPECT_FALSE((bool)OptU64);
+  OptU64 = SubprogramDieNoPC.getAttributeValueAsAddress(DW_AT_high_pc);
+  EXPECT_FALSE((bool)OptU64);
+  EXPECT_FALSE(SubprogramDieNoPC.getLowAndHighPC(LowPC, HighPC));
+  OptU64 = SubprogramDieNoPC.getAttributeValueAsAddress(DW_AT_high_pc);
+  EXPECT_FALSE((bool)OptU64);
+  OptU64 = SubprogramDieNoPC.getAttributeValueAsUnsignedConstant(DW_AT_high_pc);
+  EXPECT_FALSE((bool)OptU64);
+  OptU64 = SubprogramDieNoPC.getHighPC(ActualLowPC);
+  EXPECT_FALSE((bool)OptU64);
+  EXPECT_FALSE(SubprogramDieNoPC.getLowAndHighPC(LowPC, HighPC));
+  
+  
+  // Verify the that our subprogram with only a low PC value succeeds when
+  // we ask for the Low PC, but fails appropriately when asked for the high PC
+  // or both low and high PC values.
+  auto SubprogramDieLowPC = SubprogramDieNoPC.getSibling();
+  EXPECT_TRUE(SubprogramDieLowPC.isValid());
+  EXPECT_EQ(SubprogramDieLowPC.getTag(), DW_TAG_subprogram);
+  OptU64 = SubprogramDieLowPC.getAttributeValueAsAddress(DW_AT_low_pc);
+  EXPECT_TRUE((bool)OptU64);
+  EXPECT_EQ(OptU64.getValue(), ActualLowPC);
+  OptU64 = SubprogramDieLowPC.getAttributeValueAsAddress(DW_AT_high_pc);
+  EXPECT_FALSE((bool)OptU64);
+  OptU64 = SubprogramDieLowPC.getAttributeValueAsUnsignedConstant(DW_AT_high_pc);
+  EXPECT_FALSE((bool)OptU64);
+  OptU64 = SubprogramDieLowPC.getHighPC(ActualLowPC);
+  EXPECT_FALSE((bool)OptU64);
+  EXPECT_FALSE(SubprogramDieLowPC.getLowAndHighPC(LowPC, HighPC));
+
+  
+  // Verify the that our subprogram with only a low PC value succeeds when
+  // we ask for the Low PC, but fails appropriately when asked for the high PC
+  // or both low and high PC values.
+  auto SubprogramDieLowHighPC = SubprogramDieLowPC.getSibling();
+  EXPECT_TRUE(SubprogramDieLowHighPC.isValid());
+  EXPECT_EQ(SubprogramDieLowHighPC.getTag(), DW_TAG_subprogram);
+  OptU64 = SubprogramDieLowHighPC.getAttributeValueAsAddress(DW_AT_low_pc);
+  EXPECT_TRUE((bool)OptU64);
+  EXPECT_EQ(OptU64.getValue(), ActualLowPC);
+  // Get the high PC as an address. This should succeed if the high PC was
+  // encoded as an address and fail if the high PC was encoded as an offset.
+  OptU64 = SubprogramDieLowHighPC.getAttributeValueAsAddress(DW_AT_high_pc);
+  if (SupportsHighPCAsOffset) {
+    EXPECT_FALSE((bool)OptU64);
+  } else {
+    EXPECT_TRUE((bool)OptU64);
+    EXPECT_EQ(OptU64.getValue(), ActualHighPC);
+  }
+  // Get the high PC as an unsigned constant. This should succeed if the high PC
+  // was encoded as an offset and fail if the high PC was encoded as an address.
+  OptU64 = SubprogramDieLowHighPC.getAttributeValueAsUnsignedConstant(
+      DW_AT_high_pc);
+  if (SupportsHighPCAsOffset) {
+    EXPECT_TRUE((bool)OptU64);
+    EXPECT_EQ(OptU64.getValue(), ActualHighPCOffset);
+  } else {
+    EXPECT_FALSE((bool)OptU64);
+  }
+
+  OptU64 = SubprogramDieLowHighPC.getHighPC(ActualLowPC);
+  EXPECT_TRUE((bool)OptU64);
+  EXPECT_EQ(OptU64.getValue(), ActualHighPC);
+
+  EXPECT_TRUE(SubprogramDieLowHighPC.getLowAndHighPC(LowPC, HighPC));
+  EXPECT_EQ(LowPC, ActualLowPC);
+  EXPECT_EQ(HighPC, ActualHighPC);
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version2Addr4Addresses) {
+  // Test that we can decode address values in DWARF32, version 2, with 4 byte
+  // addresses.
+  typedef uint32_t AddrType;
+  TestAddresses<2, AddrType>();
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version2Addr8Addresses) {
+  // Test that we can decode address values in DWARF32, version 2, with 8 byte
+  // addresses.
+  typedef uint64_t AddrType;
+  TestAddresses<2, AddrType>();
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version3Addr4Addresses) {
+  // Test that we can decode address values in DWARF32, version 3, with 4 byte
+  // addresses.
+  typedef uint32_t AddrType;
+  TestAddresses<3, AddrType>();
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version3Addr8Addresses) {
+  // Test that we can decode address values in DWARF32, version 3, with 8 byte
+  // addresses.
+  typedef uint64_t AddrType;
+  TestAddresses<3, AddrType>();
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version4Addr4Addresses) {
+  // Test that we can decode address values in DWARF32, version 4, with 4 byte
+  // addresses.
+  typedef uint32_t AddrType;
+  TestAddresses<4, AddrType>();
+}
+
+TEST(DWARFDebugInfo, TestDWARF32Version4Addr8Addresses) {
+  // Test that we can decode address values in DWARF32, version 4, with 8 byte
+  // addresses.
+  typedef uint64_t AddrType;
+  TestAddresses<4, AddrType>();
+}
+
+
 } // end anonymous namespace