[ELF] - Do not crash when assign common symbol's values in script

[grimar]

Summary

Found that during attempts of linking linux kernel,
previous behavior of inputs from testcase was crash.

[meadori]

IIUC, the crash is b/c LinkerScript<ELFT>::getSymbolSection has no code to handle the common symbol case. The fix is to use SymbolTableSection<ELFT>::getOutputSection, which does have code for handling commons (and various other kinds). If so, then LGTM.

[grimar]

In D28903#650478, @meadori wrote:

IIUC, the crash is b/c LinkerScript<ELFT>::getSymbolSection has no code to handle the common symbol case. The fix is to use SymbolTableSection<ELFT>::getOutputSection, which does have code for handling commons (and various other kinds). If so, then LGTM.

Yes, you're correct. Previously we just partially duplicated code from getSymbolSection().
I probably had to write that all in description. Thank you for looking at this !

lld/trunk/ELF/LinkerScript.cpp

 const OutputSectionBase *LinkerScript<ELFT>::getSymbolSection(StringRef S) {
   SymbolBody *Sym = Symtab<ELFT>::X->find(S);
   if (!Sym) {
     if (OutputSections->empty())
       return nullptr;
     return CurOutSec ? CurOutSec : (*OutputSections)[0];
   }
 
-  if (auto *DR = dyn_cast_or_null<DefinedRegular<ELFT>>(Sym))
-    return DR->Section ? DR->Section->OutSec : nullptr;
-  if (auto *DS = dyn_cast_or_null<DefinedSynthetic>(Sym))
-    return DS->Section;
-
-  return nullptr;
+  return SymbolTableSection<ELFT>::getOutputSection(Sym);
 }
 
 // Returns indices of ELF headers containing specific section, identified
 // by Name. Each index is a zero based number of ELF header listed within
 // PHDRS {} script block.
 template <class ELFT>
 std::vector<size_t> LinkerScript<ELFT>::getPhdrIndices(StringRef SectionName) {
   for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {

lld/trunk/ELF/SyntheticSections.h

public:
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
   void addSymbol(SymbolBody *Body);
   StringTableSection<ELFT> &getStrTabSec() const { return StrTabSec; }
   unsigned getNumSymbols() const { return NumLocals + Symbols.size() + 1; }
 
   ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
 
+  static const OutputSectionBase *getOutputSection(SymbolBody *Sym);
+
   unsigned NumLocals = 0;
   StringTableSection<ELFT> &StrTabSec;
 
 private:
   void writeLocalSymbols(uint8_t *&Buf);
   void writeGlobalSymbols(uint8_t *Buf);
 
-  const OutputSectionBase *getOutputSection(SymbolBody *Sym);
-
   // A vector of symbols and their string table offsets.
   std::vector<SymbolTableEntry> Symbols;
 };
 
 // Outputs GNU Hash section. For detailed explanation see:
 // https://blogs.oracle.com/ali/entry/gnu_hash_elf_sections
 template <class ELFT>
 class GnuHashTableSection final : public SyntheticSection<ELFT> {

lld/trunk/test/ELF/linkerscript/common-assign.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { . = SIZEOF_HEADERS; pfoo = foo; pbar = bar; }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-readobj -symbols %t1 | FileCheck %s
+
+# CHECK:       Symbol {
+# CHECK:         Name: bar
+# CHECK-NEXT:     Value: 0x134
+# CHECK-NEXT:     Size: 4
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Object
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value: 0x138
+# CHECK-NEXT:     Size: 4
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Object
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: pfoo
+# CHECK-NEXT:     Value: 0x138
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: pbar
+# CHECK-NEXT:     Value: 0x134
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.comm foo,4,4
+.comm bar,4,4
+movl  $1, foo(%rip)
+movl  $2, bar(%rip)