[lld] r268604 - ELF: Do not use -1 to mark pieces of merge sections as being tail merged.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 21:10:12 PDT 2016


Author: pcc
Date: Wed May  4 23:10:12 2016
New Revision: 268604

URL: http://llvm.org/viewvc/llvm-project?rev=268604&view=rev
Log:
ELF: Do not use -1 to mark pieces of merge sections as being tail merged.

We were previously using an output offset of -1 for both GC'd and tail
merged pieces. We need to distinguish these two cases in order to filter
GC'd symbols from the symbol table -- we were previously asserting when we
asked for the VA of a symbol pointing into a dead piece, which would end
up asking the tail merging string table for an offset even though we hadn't
initialized it properly.

This patch fixes the bug by using an offset of -1 to exclusively mean GC'd
pieces, using 0 for tail merges, and distinguishing the tail merge case from
an offset of 0 by asking the output section whether it is tail merge.

Differential Revision: http://reviews.llvm.org/D19953

Added:
    lld/trunk/test/ELF/string-gc.s
Modified:
    lld/trunk/ELF/InputSection.cpp
    lld/trunk/ELF/InputSection.h
    lld/trunk/ELF/MarkLive.cpp
    lld/trunk/ELF/OutputSections.cpp
    lld/trunk/ELF/OutputSections.h
    lld/trunk/ELF/Writer.cpp

Modified: lld/trunk/ELF/InputSection.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/InputSection.cpp (original)
+++ lld/trunk/ELF/InputSection.cpp Wed May  4 23:10:12 2016
@@ -419,7 +419,8 @@ MergeInputSection<ELFT>::MergeInputSecti
   StringRef Data((const char *)D.data(), D.size());
   std::vector<std::pair<uintX_t, uintX_t>> &Offsets = this->Offsets;
 
-  uintX_t V = Config->GcSections ? -1 : 0;
+  uintX_t V = Config->GcSections ? MergeInputSection<ELFT>::PieceDead
+                                 : MergeInputSection<ELFT>::PieceLive;
   if (Header->sh_flags & SHF_STRINGS) {
     uintX_t Offset = 0;
     while (!Data.empty()) {
@@ -478,15 +479,15 @@ typename ELFT::uint MergeInputSection<EL
   // Compute the Addend and if the Base is cached, return.
   uintX_t Addend = Offset - Start;
   uintX_t &Base = I->second;
-  if (Base != uintX_t(-1))
+  auto *MOS = static_cast<MergeOutputSection<ELFT> *>(this->OutSec);
+  if (!MOS->shouldTailMerge())
     return Base + Addend;
 
   // Map the base to the offset in the output section and cache it.
   ArrayRef<uint8_t> D = this->getSectionData();
   StringRef Data((const char *)D.data(), D.size());
   StringRef Entry = Data.substr(Start, End - Start);
-  Base =
-      static_cast<MergeOutputSection<ELFT> *>(this->OutSec)->getOffset(Entry);
+  Base = MOS->getOffset(Entry);
   return Base + Addend;
 }
 

Modified: lld/trunk/ELF/InputSection.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/InputSection.h (original)
+++ lld/trunk/ELF/InputSection.h Wed May  4 23:10:12 2016
@@ -144,9 +144,22 @@ public:
                     typename InputSectionBase<ELFT>::Kind SectionKind);
 
   // For each piece of data, we maintain the offsets in the input section and
-  // in the output section. The latter may be -1 if it is not assigned yet.
+  // in the output section.
   std::vector<std::pair<uintX_t, uintX_t>> Offsets;
 
+  // Merge input sections may use the following special values as the output
+  // section offset:
+  enum {
+    // The piece is dead.
+    PieceDead = uintX_t(-1),
+    // The piece is live, and an offset has not yet been assigned. After offsets
+    // have been assigned, if the output section uses tail merging, the field
+    // will still have this value and the output section offset is available
+    // from MergeOutputSection<ELFT>::getOffset(). Otherwise, this value has no
+    // special significance, it just means that the offset is 0.
+    PieceLive = uintX_t(0),
+  };
+
   std::pair<std::pair<uintX_t, uintX_t> *, uintX_t>
   getRangeAndSize(uintX_t Offset);
 };

Modified: lld/trunk/ELF/MarkLive.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/MarkLive.cpp?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/MarkLive.cpp (original)
+++ lld/trunk/ELF/MarkLive.cpp Wed May  4 23:10:12 2016
@@ -143,7 +143,7 @@ template <class ELFT> void elf::markLive
     if (auto *MS = dyn_cast<MergeInputSection<ELFT>>(R.Sec)) {
       std::pair<std::pair<uintX_t, uintX_t> *, uintX_t> T =
           MS->getRangeAndSize(R.Offset);
-      T.first->second = 0;
+      T.first->second = MergeInputSection<ELFT>::PieceLive;
     }
     if (R.Sec->Live)
       return;

Modified: lld/trunk/ELF/OutputSections.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/OutputSections.cpp (original)
+++ lld/trunk/ELF/OutputSections.cpp Wed May  4 23:10:12 2016
@@ -1271,7 +1271,7 @@ void MergeOutputSection<ELFT>::addSectio
   if (this->Header.sh_flags & SHF_STRINGS) {
     for (unsigned I = 0, N = Offsets.size(); I != N; ++I) {
       auto &P = Offsets[I];
-      if (P.second == (uintX_t)-1)
+      if (P.second == MergeInputSection<ELFT>::PieceDead)
         continue;
 
       uintX_t Start = P.first;
@@ -1279,7 +1279,7 @@ void MergeOutputSection<ELFT>::addSectio
       StringRef Entry = Data.substr(Start, End - Start);
       uintX_t OutputOffset = Builder.add(Entry);
       if (shouldTailMerge())
-        OutputOffset = -1;
+        OutputOffset = MergeInputSection<ELFT>::PieceLive;
       P.second = OutputOffset;
     }
     return;

Modified: lld/trunk/ELF/OutputSections.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/OutputSections.h (original)
+++ lld/trunk/ELF/OutputSections.h Wed May  4 23:10:12 2016
@@ -310,8 +310,6 @@ template <class ELFT>
 class MergeOutputSection final : public OutputSectionBase<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
-  bool shouldTailMerge() const;
-
 public:
   MergeOutputSection(StringRef Name, uint32_t Type, uintX_t Flags,
                      uintX_t Alignment);
@@ -319,6 +317,7 @@ public:
   void writeTo(uint8_t *Buf) override;
   unsigned getOffset(StringRef Val);
   void finalize() override;
+  bool shouldTailMerge() const;
 
 private:
   llvm::StringTableBuilder Builder;

Modified: lld/trunk/ELF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=268604&r1=268603&r2=268604&view=diff
==============================================================================
--- lld/trunk/ELF/Writer.cpp (original)
+++ lld/trunk/ELF/Writer.cpp Wed May  4 23:10:12 2016
@@ -1118,9 +1118,16 @@ template <class ELFT> static bool includ
     return false;
 
   if (auto *D = dyn_cast<DefinedRegular<ELFT>>(&B)) {
+    // Always include absolute symbols.
+    if (!D->Section)
+      return true;
     // Exclude symbols pointing to garbage-collected sections.
-    if (D->Section && !D->Section->Live)
+    if (!D->Section->Live)
       return false;
+    if (auto *S = dyn_cast<MergeInputSection<ELFT>>(D->Section))
+      if (S->getRangeAndSize(D->Value).first->second ==
+          MergeInputSection<ELFT>::PieceDead)
+        return false;
   }
   return true;
 }

Added: lld/trunk/test/ELF/string-gc.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/string-gc.s?rev=268604&view=auto
==============================================================================
--- lld/trunk/test/ELF/string-gc.s (added)
+++ lld/trunk/test/ELF/string-gc.s Wed May  4 23:10:12 2016
@@ -0,0 +1,54 @@
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+// RUN: ld.lld %t.o -o %t --gc-sections
+// RUN: llvm-readobj -symbols %t | FileCheck %s
+
+// CHECK:      Symbols [
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name:  (0)
+// CHECK-NEXT:     Value: 0x0
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Local (0x0)
+// CHECK-NEXT:     Type: None (0x0)
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: Undefined (0x0)
+// CHECK-NEXT:   }
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name: s1 (8)
+// CHECK-NEXT:     Value: 0x10120
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Local (0x0)
+// CHECK-NEXT:     Type: Object (0x1)
+// CHECK-NEXT:     Other [ (0x2)
+// CHECK-NEXT:       STV_HIDDEN (0x2)
+// CHECK-NEXT:     ]
+// CHECK-NEXT:     Section: .rodata (0x1)
+// CHECK-NEXT:   }
+// CHECK-NEXT:   Symbol {
+// CHECK-NEXT:     Name: _start (1)
+// CHECK-NEXT:     Value: 0x11000
+// CHECK-NEXT:     Size: 0
+// CHECK-NEXT:     Binding: Global (0x1)
+// CHECK-NEXT:     Type: Function (0x2)
+// CHECK-NEXT:     Other: 0
+// CHECK-NEXT:     Section: .text (0x2)
+// CHECK-NEXT:   }
+// CHECK-NEXT: ]
+
+.text
+.globl _start
+.type _start, at function
+_start:
+movl $s1, %eax
+
+.hidden s1
+.type s1, at object
+.section .rodata.str1.1,"aMS", at progbits,1
+.globl s1
+s1:
+.asciz "abcd"
+
+.hidden s2
+.type s2, at object
+.globl s2
+s2:
+.asciz "efgh"




More information about the llvm-commits mailing list