If --dynamic-list is given, only those symbols are preemptible

[• rafael]

Summary

This is PR34053.

The implementation is a bit of a hack, given the precise location where IsPreemtible is set, it cannot be used from SymbolTable::handleAnonymousVersion.

I could add another method to SymbolTable if you think that would be better.

[smeenai]

Thank you!

One issue I have is that this still conflates version scripts and dynamic lists, whereas they're orthogonal concepts in my mind; a version script controls dynamic symbol table population, while a dynamic list (for a shared library) controls preemptibility. The test case below demonstrates this; it passes with ld.bfd and ld.gold.

# REQUIRES: x86

# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
# RUN: echo "{ foo; };" > %t.list
# RUN: echo "{ global: foo; bar; };" > %t.vers
# RUN: ld.lld -fatal-warnings -dynamic-list %t.list -version-script %t.vers -shared %t.o -o %t.so
# RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=RELOCS %s
# RUN: llvm-nm -DU %t.so | FileCheck --check-prefix=DYNSYMS %s

# RELOCS:      Relocations [
# RELOCS-NEXT:   Section ({{.*}}) .rela.plt {
# RELOCS-NEXT:     R_X86_64_JUMP_SLOT foo 0x0
# RELOCS-NEXT:   }
# RELOCS-NEXT: ]

# DYNSYMS:      bar
# DYNSYMS-NEXT: foo

	.globl	foo
foo:
	ret

	.globl	bar
bar:
	ret

	call	foo@PLT
	call	bar@PLT

[grimar]

ELF/Writer.cpp

1188

foo ? :) Looks there is no test for this error.

[ruiu]

ELF/Config.h

87

These members are sorted by type and then by name, so move below.

ELF/Driver.cpp

748

Maybe you want to set this only once by Config->HasDynamicList = Args.hasArg(OPT_dynamic_list)?

ELF/Writer.cpp

1185

This needs a comment.

[• rafael]

Fix iteration with version scripts.

It is now possible to use version scripts to decide which symbols are globals and --dynamic-list to pick which of those are also preemptible.

[smeenai]

This looks great. Thank you!

I'll leave the approval to Rui.

[• rafael]

I am going on vacation. Please commit if OK.

[ruiu]

LGTM

Since Rafael is on vacation, I'll submit this patch with the following fixes.

ELF/ScriptParser.cpp

182–183 ↗

(On Diff #110623)

You want to return here.

185 ↗

(On Diff #110623)

+ next() doesn't seem to make sense, so remove.

[smeenai]

This was committed in r311468 and reverted in r311497 because it broke bots. I'm re-opening now that @rafael is back from vacation.

[• espindola]

312806

ELF/Config.h

 // Most fields are initialized by the driver.
 struct Configuration {
   InputFile *FirstElf = nullptr;
   uint8_t OSABI = 0;
   llvm::CachePruningPolicy ThinLTOCachePolicy;
   llvm::StringMap<uint64_t> SectionStartMap;
   llvm::StringRef Chroot;
   llvm::StringRef DynamicLinker;
+  bool HasDynamicList;
   llvm::StringRef Entry;
   llvm::StringRef Emulation;
   llvm::StringRef Fini;
   llvm::StringRef Init;
   llvm::StringRef LTOAAPipeline;
   llvm::StringRef LTONewPmPasses;
   llvm::StringRef MapFile;
   llvm::StringRef OutputFile;

ELF/Driver.cpp

void LinkerDriver::readConfigs(opt::InputArgList &Args) {
 
   bool HasExportDynamic =
       getArg(Args, OPT_export_dynamic, OPT_no_export_dynamic, false);
 
   // Parses -dynamic-list and -export-dynamic-symbol. They make some
   // symbols private. Note that -export-dynamic takes precedence over them
   // as it says all symbols should be exported.
   if (!HasExportDynamic) {
-    for (auto *Arg : Args.filtered(OPT_dynamic_list))
+    for (auto *Arg : Args.filtered(OPT_dynamic_list)) {
+      Config->HasDynamicList = true;
       if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
         readDynamicList(*Buffer);
+    }
 
     for (auto *Arg : Args.filtered(OPT_export_dynamic_symbol))
       Config->VersionScriptGlobals.push_back(
           {Arg->getValue(), /*IsExternCpp*/ false, /*HasWildcard*/ false});
 
     // Dynamic lists are a simplified linker script that doesn't need the
     // "global:" and implicitly ends with a "local:*". Set the variables
     // needed to simulate that.

ELF/SymbolTable.h

public:
   template <class ELFT> void scanShlibUndefined();
   void scanVersionScript();
 
   SymbolBody *find(StringRef Name);
   SymbolBody *findInCurrentDSO(StringRef Name);
 
   void trace(StringRef Name);
 
-private:
   std::vector<SymbolBody *> findByVersion(SymbolVersion Ver);
+
+private:
   std::vector<SymbolBody *> findAllByVersion(SymbolVersion Ver);
 
   llvm::StringMap<std::vector<SymbolBody *>> &getDemangledSyms();
   void handleAnonymousVersion();
   void assignExactVersion(SymbolVersion Ver, uint16_t VersionId,
                           StringRef VersionName);
   void assignWildcardVersion(SymbolVersion Ver, uint16_t VersionId);
 

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::finalizeSections() {
   addRelIpltSymbols();
 
   // This responsible for splitting up .eh_frame section into
   // pieces. The relocation scan uses those pieces, so this has to be
   // earlier.
   applySynthetic({In<ELFT>::EhFrame},
                  [](SyntheticSection *SS) { SS->finalizeContents(); });
 
+  if (Config->HasDynamicList) {
+    for (SymbolVersion V : Config->VersionScriptGlobals) {
+      if (V.HasWildcard)
+        error("foo");
+      std::vector<SymbolBody *> Syms = Symtab->findByVersion(V);
+      for (SymbolBody *B : Syms)
+        B->IsPreemptible = true;
+    }
+  } else {
     for (Symbol *S : Symtab->getSymbols())
       S->body()->IsPreemptible = S->body()->computeIsPreemptible();
+  }
 
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.
   forEachRelSec(scanRelocations<ELFT>);
 
   if (InX::Plt && !InX::Plt->empty())
     InX::Plt->addSymbols();
   if (InX::Iplt && !InX::Iplt->empty())

test/ELF/dynamic-list-preempt.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "{ foo; };" > %t.list
+# RUN: ld.lld -dynamic-list %t.list -shared %t.o -o %t.so
+# RUN: llvm-readobj -r %t.so | FileCheck %s
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section ({{.*}}) .rela.plt {
+# CHECK-NEXT:     R_X86_64_JUMP_SLOT foo 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+  .globl  foo
+foo:
+  ret
+
+  .globl  bar
+bar:
+  ret
+
+  call  foo@PLT
+  call  bar@PLT