ELF: Override DSO definitions when creating __start_* and __stop_* symbols.

[pcc]

Summary

Previously we would fail to synthesise a start_ or stop_ symbol if
there existed a definition in a DSO. Instead, we would try to link against
the DSO definition. This became possible after D23552 when linking against
lld-produced DSOs but could in principle also occur when linking against
DSOs produced by other linkers.

Not only does it seem more likely that a user would expect the resolved
definition to be local to the executable, but if a start_ or stop_
symbol was synthesised by the linker, it is effectively impossible to link
against correctly from a non-PIC executable in a read-only section. Neither
a PLT nor a copy relocation would give us the right semantics here. The only
way the link could succeed is if the executable provided its own synthetic
definition of the symbol.

The fix is to also synthesise the definition if the only definition comes
from a DSO. Since this is what the addOptionalSynthetic function does,
switch to using that function.

Fixes PR30680.

[ruiu]

LGTM

[joerg]

You want to allow the main program to get the start/stop of a DSO it links against? Am I understanding you correct?

[krasin]

In D25544#569700, @joerg wrote:

You want to allow the main program to get the start/stop of a DSO it links against? Am I understanding you correct?

Yes, this is correct. One of the major reasons to allow that is that tcmalloc relies on this interface for the malloc hooks:
https://cs.chromium.org/chromium/src/third_party/tcmalloc/vendor/src/base/basictypes.h?q=ATTRIBUTE_SECTION&sq=package:chromium&dr=CSs&l=227

[pcc]

No, the opposite. I want the program to get its own start/stop even if a DSO provides a definition.

[krasin]

In D25544#569703, @pcc wrote:

No, the opposite. I want the program to get its own start/stop even if a DSO provides a definition.

Then count me confused...

[davide]

lgtm

lld/trunk/ELF/Writer.cpp

void PhdrEntry<ELFT>::add(OutputSectionBase<ELFT> *Sec) {
   if (!First)
     First = Sec;
   H.p_align = std::max<typename ELFT::uint>(H.p_align, Sec->getAlignment());
   if (H.p_type == PT_LOAD)
     Sec->FirstInPtLoad = First;
 }
 
 template <class ELFT>
-static Symbol *addOptionalSynthetic(StringRef Name,
-                                    OutputSectionBase<ELFT> *Sec,
-                                    typename ELFT::uint Val) {
+static Symbol *
+addOptionalSynthetic(StringRef Name, OutputSectionBase<ELFT> *Sec,
+                     typename ELFT::uint Val, uint8_t StOther = STV_HIDDEN) {
   SymbolBody *S = Symtab<ELFT>::X->find(Name);
   if (!S)
     return nullptr;
   if (!S->isUndefined() && !S->isShared())
     return S->symbol();
-  return Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, STV_HIDDEN);
+  return Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, StOther);
 }
 
 template <class ELFT>
 static void addSynthetic(StringRef Name, OutputSectionBase<ELFT> *Sec,
                          typename ELFT::uint Val) {
   SymbolBody *S = Symtab<ELFT>::X->find(Name);
   if (!S || S->isUndefined() || S->isShared())
     Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, STV_HIDDEN);

 // respectively. This is not requested by the ELF standard, but GNU ld and
 // gold provide the feature, and used by many programs.
 template <class ELFT>
 void Writer<ELFT>::addStartStopSymbols(OutputSectionBase<ELFT> *Sec) {
   StringRef S = Sec->getName();
   if (!isValidCIdentifier(S))
     return;
   StringSaver Saver(Alloc);
-  StringRef Start = Saver.save("__start_" + S);
-  StringRef Stop = Saver.save("__stop_" + S);
-  if (SymbolBody *B = Symtab<ELFT>::X->find(Start))
-    if (B->isUndefined())
-      Symtab<ELFT>::X->addSynthetic(Start, Sec, 0, B->getVisibility());
-  if (SymbolBody *B = Symtab<ELFT>::X->find(Stop))
-    if (B->isUndefined())
-      Symtab<ELFT>::X->addSynthetic(
-          Stop, Sec, DefinedSynthetic<ELFT>::SectionEnd, B->getVisibility());
+  addOptionalSynthetic(Saver.save("__start_" + S), Sec, 0, STV_DEFAULT);
+  addOptionalSynthetic(Saver.save("__stop_" + S), Sec,
+                       DefinedSynthetic<ELFT>::SectionEnd, STV_DEFAULT);
 }
 
 template <class ELFT>
 OutputSectionBase<ELFT> *Writer<ELFT>::findSection(StringRef Name) {
   for (OutputSectionBase<ELFT> *Sec : OutputSections)
     if (Sec->getName() == Name)
       return Sec;
   return nullptr;

lld/trunk/test/ELF/Inputs/startstop-shared2.s

+.globl __start_foo
+__start_foo:

lld/trunk/test/ELF/startstop-shared2.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/startstop-shared2.s -o %t.o
+// RUN: ld.lld -o %t.so %t.o -shared
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2.o
+// RUN: ld.lld -o %t %t2.o %t.so
+// RUN: llvm-objdump -s -h %t | FileCheck %s
+
+// CHECK: foo           00000000 0000000000011008
+
+// CHECK: Contents of section .text:
+// CHECK-NEXT: 11000 08100100 00000000
+
+.quad __start_foo
+.section foo,"ax"