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

llvm-svn: 268604

7 files changed, 85 insertions, 11 deletions

diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 6737c2d8367..41703fb73b3 100644
--- a/lld/ELF/InputSection.cpp
+++ b/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;
 }
 
diff --git a/lld/ELF/InputSection.h b/lld/ELF/InputSection.h
index 908375f1a63..ea8fd438cdf 100644
--- a/lld/ELF/InputSection.h
+++ b/lld/ELF/InputSection.h
@@ -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);
 };
diff --git a/lld/ELF/MarkLive.cpp b/lld/ELF/MarkLive.cpp
index 5b20ccca047..f8000007913 100644
--- a/lld/ELF/MarkLive.cpp
+++ b/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;
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index 50108de6845..c3c9672e708 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -1271,7 +1271,7 @@ 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;
@@ -1279,7 +1279,7 @@ void MergeOutputSection<ELFT>::addSection(InputSectionBase<ELFT> *C) {
       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;
diff --git a/lld/ELF/OutputSections.h b/lld/ELF/OutputSections.h
index 1a305fbf1b0..d8c0e5b7484 100644
--- a/lld/ELF/OutputSections.h
+++ b/lld/ELF/OutputSections.h
@@ -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;
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 63ed12a64e1..a590827c2e0 100644
--- a/lld/ELF/Writer.cpp
+++ b/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;
 }
diff --git a/lld/test/ELF/string-gc.s b/lld/test/ELF/string-gc.s
new file mode 100644
index 00000000000..b6b1e868f5d
--- /dev/null
+++ b/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"