ELF: Do not use -1 to mark pieces of merge sections as being tail mer…

…ged.

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

llvm-svn: 268604

lld/ELF/InputSection.cpp

@@ -419,7 +419,8 @@ MergeInputSection<ELFT>::MergeInputSection(elf::ObjectFile<ELFT> *F,
   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<ELFT>::getOffset(uintX_t Offset) {
   // 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;
 }
 

lld/ELF/InputSection.h

@@ -144,9 +144,22 @@ template <class ELFT> class SplitInputSection : public InputSectionBase<ELFT> {
                     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);
 };

lld/ELF/MarkLive.cpp

@@ -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;

lld/ELF/OutputSections.cpp

@@ -1271,15 +1271,15 @@ void MergeOutputSection<ELFT>::addSection(InputSectionBase<ELFT> *C) {
   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;
       uintX_t End = (I == N - 1) ? Data.size() : Offsets[I + 1].first;
       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;

lld/ELF/OutputSections.h

@@ -310,15 +310,14 @@ 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);
   void addSection(InputSectionBase<ELFT> *S) override;
   void writeTo(uint8_t *Buf) override;
   unsigned getOffset(StringRef Val);
   void finalize() override;
+  bool shouldTailMerge() const;
 
 private:
   llvm::StringTableBuilder Builder;

lld/ELF/Writer.cpp

@@ -1118,9 +1118,16 @@ template <class ELFT> static bool includeInSymtab(const SymbolBody &B) {
     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;
 }

lld/test/ELF/string-gc.s

@@ -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,@function
+_start:
+movl $s1, %eax
+
+.hidden s1
+.type s1,@object
+.section .rodata.str1.1,"aMS",@progbits,1
+.globl s1
+s1:
+.asciz "abcd"
+
+.hidden s2
+.type s2,@object
+.globl s2
+s2:
+.asciz "efgh"