[llvm] 5e13e0c - [NFC] Move ValidTextRanges out of DwarfTransformer and into GsymCreator and unify address is not in GSYM errors so all strings match.

Greg Clayton via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 15 16:48:36 PST 2020


Author: Greg Clayton
Date: 2020-02-15T16:48:23-08:00
New Revision: 5e13e0ce4cd695a71225183ce7d3590ccc9d4559

URL: https://github.com/llvm/llvm-project/commit/5e13e0ce4cd695a71225183ce7d3590ccc9d4559
DIFF: https://github.com/llvm/llvm-project/commit/5e13e0ce4cd695a71225183ce7d3590ccc9d4559.diff

LOG: [NFC] Move ValidTextRanges out of DwarfTransformer and into GsymCreator and unify address is not in GSYM errors so all strings match.

Added: 
    

Modified: 
    llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
    llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
    llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
    llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
    llvm/lib/DebugInfo/GSYM/GsymReader.cpp
    llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h b/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
index 7a1553f78183..a5d07fbeda92 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/DwarfTransformer.h
@@ -53,23 +53,6 @@ class DwarfTransformer {
 
   llvm::Error verify(StringRef GsymPath);
 
-  /// Set valid .text address ranges that all functions be contained in.
-  ///
-  /// Any functions whose addresses do not exist within these function bounds
-  /// will not be converted into the final GSYM. This allows the object file
-  /// to figure out the valid file address ranges of all the code sections
-  /// and ensure we don't add invalid functions to the final output. Many
-  /// linkers have issues when dead stripping functions where they set the
-  /// DW_AT_low_pc to zero, but newer DWARF has the DW_AT_high_pc as an offset
-  /// from the DW_AT_low_pc and these size attributes have no relocations that
-  /// can be applied. This results in DWARF where many functions have an
-  /// DW_AT_low_pc of zero and a valid offset size for DW_AT_high_pc. If we
-  /// extract all valid ranges from an object file that are marked with
-  /// executable permissions, we can properly ensure that these functions are
-  /// removed.
-  void SetValidTextRanges(AddressRanges &TextRanges) {
-    ValidTextRanges = TextRanges;
-  }
 
 private:
 
@@ -98,7 +81,6 @@ class DwarfTransformer {
   DWARFContext &DICtx;
   raw_ostream &Log;
   GsymCreator &Gsym;
-  Optional<AddressRanges> ValidTextRanges;
 
   friend class DwarfTransformerTest;
 };

diff  --git a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
index 1d684ce45f96..fc98516e5731 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h
@@ -140,6 +140,7 @@ class GsymCreator {
   DenseMap<llvm::gsym::FileEntry, uint32_t> FileEntryToIndex;
   std::vector<llvm::gsym::FileEntry> Files;
   std::vector<uint8_t> UUID;
+  Optional<AddressRanges> ValidTextRanges;
   bool Finalized = false;
 
 public:
@@ -229,6 +230,38 @@ class GsymCreator {
   /// Get the current number of FunctionInfo objects contained in this
   /// object.
   size_t getNumFunctionInfos() const;
+
+  /// Set valid .text address ranges that all functions must be contained in.
+  void SetValidTextRanges(AddressRanges &TextRanges) {
+    ValidTextRanges = TextRanges;
+  }
+
+  /// Get the valid text ranges.
+  const Optional<AddressRanges> GetValidTextRanges() const {
+    return ValidTextRanges;
+  }
+
+  /// Check if an address is a valid code address.
+  ///
+  /// Any functions whose addresses do not exist within these function bounds
+  /// will not be converted into the final GSYM. This allows the object file
+  /// to figure out the valid file address ranges of all the code sections
+  /// and ensure we don't add invalid functions to the final output. Many
+  /// linkers have issues when dead stripping functions from DWARF debug info
+  /// where they set the DW_AT_low_pc to zero, but newer DWARF has the
+  /// DW_AT_high_pc as an offset from the DW_AT_low_pc and these size
+  /// attributes have no relocations that can be applied. This results in DWARF
+  /// where many functions have an DW_AT_low_pc of zero and a valid offset size
+  /// for DW_AT_high_pc. If we extract all valid ranges from an object file
+  /// that are marked with executable permissions, we can properly ensure that
+  /// these functions are removed.
+  ///
+  /// \param Addr An address to check.
+  ///
+  /// \returns True if the address is in the valid text ranges or if no valid
+  ///          text ranges have been set, false otherwise.
+  bool IsValidTextAddress(uint64_t Addr) const;
+
 };
 
 } // namespace gsym

diff  --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index c3bf71f21cda..1e527ab3916e 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -384,7 +384,7 @@ void DwarfTransformer::handleDie(raw_ostream &OS, CUInfo &CUI, DWARFDie Die) {
       // high PC can be an offset from the low PC in more recent DWARF versions
       // we need to watch for a zero'ed low pc which we do using
       // ValidTextRanges below.
-      if (ValidTextRanges && !ValidTextRanges->contains(Range.LowPC)) {
+      if (!Gsym.IsValidTextAddress(Range.LowPC)) {
         // We expect zero and -1 to be invalid addresses in DWARF depending
         // on the linker of the DWARF. This indicates a function was stripped
         // and the debug info wasn't able to be stripped from the DWARF. If
@@ -392,8 +392,8 @@ void DwarfTransformer::handleDie(raw_ostream &OS, CUInfo &CUI, DWARFDie Die) {
         if (Range.LowPC != 0) {
           // Unexpected invalid address, emit an error
           Log << "warning: DIE has an address range whose start address is "
-              "not in any executable sections (" << *ValidTextRanges <<
-              ") and will not be processed:\n";
+              "not in any executable sections (" <<
+              *Gsym.GetValidTextRanges() << ") and will not be processed:\n";
           Die.dump(Log, 0, DIDumpOptions::getForSingleDIE());
         }
         break;

diff  --git a/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp b/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
index b4cf199f8e07..a8b03afd69da 100644
--- a/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
+++ b/llvm/lib/DebugInfo/GSYM/GsymCreator.cpp
@@ -289,3 +289,8 @@ size_t GsymCreator::getNumFunctionInfos() const{
   return Funcs.size();
 }
 
+bool GsymCreator::IsValidTextAddress(uint64_t Addr) const {
+  if (ValidTextRanges)
+    return ValidTextRanges->contains(Addr);
+  return true; // No valid text ranges has been set, so accept all ranges.
+}

diff  --git a/llvm/lib/DebugInfo/GSYM/GsymReader.cpp b/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
index bc5f14f80d8a..39e811da4e91 100644
--- a/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
+++ b/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
@@ -227,7 +227,7 @@ Expected<uint64_t>
 GsymReader::getAddressIndex(const uint64_t Addr) const {
   if (Addr < Hdr->BaseAddress)
     return createStringError(std::errc::invalid_argument,
-                             "address 0x%" PRIx64 " not in GSYM", Addr);
+                             "address 0x%" PRIx64 " is not in GSYM", Addr);
   const uint64_t AddrOffset = Addr - Hdr->BaseAddress;
   switch (Hdr->AddrOffSize) {
   case 1: return getAddressOffsetIndex<uint8_t>(AddrOffset);
@@ -255,7 +255,7 @@ llvm::Expected<FunctionInfo> GsymReader::getFunctionInfo(uint64_t Addr) const {
       if (ExpectedFI->Range.contains(Addr) || ExpectedFI->Range.size() == 0)
         return ExpectedFI;
       return createStringError(std::errc::invalid_argument,
-                                "address 0x%" PRIx64 " not in GSYM", Addr);
+                                "address 0x%" PRIx64 " is not in GSYM", Addr);
     }
   }
   return createStringError(std::errc::invalid_argument,
@@ -322,10 +322,10 @@ void GsymReader::dump(raw_ostream &OS) {
     dump(OS, getFile(I));
     OS << "\n";
   }
-  OS << "\n" << StrTab;
+  OS << "\n" << StrTab << "\n";
 
   for (uint32_t I = 0; I < Header.NumAddresses; ++I) {
-    OS << "\nFunctionInfo @ " << HEX32(AddrInfoOffsets[I]) << ": ";
+    OS << "FunctionInfo @ " << HEX32(AddrInfoOffsets[I]) << ": ";
     if (auto FI = getFunctionInfo(*getAddress(I)))
       dump(OS, *FI);
     else

diff  --git a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
index 5c567b012f1b..d818edd72b4f 100644
--- a/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
+++ b/llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp
@@ -1288,21 +1288,21 @@ TEST(GSYMTest, TestGsymReader) {
   ASSERT_FALSE((bool)Err);
   if (auto ExpectedGR = GsymReader::copyBuffer(OutStrm.str())) {
     const GsymReader &GR = ExpectedGR.get();
-    VerifyFunctionInfoError(GR, Func1Addr-1, "address 0xfff not in GSYM");
+    VerifyFunctionInfoError(GR, Func1Addr-1, "address 0xfff is not in GSYM");
 
     FunctionInfo Func1(Func1Addr, FuncSize, Func1Name);
     VerifyFunctionInfo(GR, Func1Addr, Func1);
     VerifyFunctionInfo(GR, Func1Addr+1, Func1);
     VerifyFunctionInfo(GR, Func1Addr+FuncSize-1, Func1);
     VerifyFunctionInfoError(GR, Func1Addr+FuncSize,
-                            "address 0x1010 not in GSYM");
-    VerifyFunctionInfoError(GR, Func2Addr-1, "address 0x101f not in GSYM");
+                            "address 0x1010 is not in GSYM");
+    VerifyFunctionInfoError(GR, Func2Addr-1, "address 0x101f is not in GSYM");
     FunctionInfo Func2(Func2Addr, FuncSize, Func2Name);
     VerifyFunctionInfo(GR, Func2Addr, Func2);
     VerifyFunctionInfo(GR, Func2Addr+1, Func2);
     VerifyFunctionInfo(GR, Func2Addr+FuncSize-1, Func2);
     VerifyFunctionInfoError(GR, Func2Addr+FuncSize,
-                            "address 0x1030 not in GSYM");
+                            "address 0x1030 is not in GSYM");
   }
 }
 
@@ -1765,7 +1765,7 @@ TEST(GSYMTest, TestDWARFTextRanges) {
   // GSYM.
   AddressRanges TextRanges;
   TextRanges.insert(AddressRange(0x1000, 0x2000));
-  DT.SetValidTextRanges(TextRanges);
+  GC.SetValidTextRanges(TextRanges);
   const uint32_t ThreadCount = 1;
   ASSERT_THAT_ERROR(DT.convert(ThreadCount), Succeeded());
   ASSERT_THAT_ERROR(GC.finalize(OS), Succeeded());


        


More information about the llvm-commits mailing list