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 ↗

(On Diff #110318)

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

[ruiu]

ELF/Config.h

87 ↗

(On Diff #110318)

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

ELF/Driver.cpp

748 ↗

(On Diff #110318)

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

ELF/Writer.cpp

1185 ↗

(On Diff #110318)

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

lld/trunk/ELF/Config.h

struct Configuration {
   std::string Rpath;
   std::vector<VersionDefinition> VersionDefinitions;
   std::vector<llvm::StringRef> Argv;
   std::vector<llvm::StringRef> AuxiliaryList;
   std::vector<llvm::StringRef> FilterList;
   std::vector<llvm::StringRef> SearchPaths;
   std::vector<llvm::StringRef> SymbolOrderingFile;
   std::vector<llvm::StringRef> Undefined;
+  std::vector<SymbolVersion> DynamicList;
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<SymbolVersion> VersionScriptLocals;
   std::vector<uint8_t> BuildIdVector;
   llvm::MapVector<Symbol *, RenamedSymbol> RenamedSymbols;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;

lld/trunk/ELF/ScriptParser.cpp

 static ExprValue bitOr(ExprValue A, ExprValue B) {
   moveAbsRight(A, B);
   return {A.Sec, A.ForceAbsolute,
           (A.getValue() | B.getValue()) - A.getSecAddr(), A.Loc};
 }
 
 void ScriptParser::readDynamicList() {
   expect("{");
-  readAnonymousDeclaration();
-  if (!atEOF())
+  std::vector<SymbolVersion> Locals;
+  std::vector<SymbolVersion> Globals;
+  std::tie(Locals, Globals) = readSymbols();
+  expect(";");
+
+  if (!atEOF()) {
     setError("EOF expected, but got " + next());
+    return;
+  }
+  if (!Locals.empty()) {
+    setError("\"local:\" scope not supported in --dynamic-list");
+    return;
+  }
+
+  for (SymbolVersion V : Globals)
+    Config->DynamicList.push_back(V);
 }
 
 void ScriptParser::readVersionScript() {
   readVersionScriptCommand();
   if (!atEOF())
     setError("EOF expected, but got " + next());
 }
 

lld/trunk/ELF/SymbolTable.h

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

lld/trunk/ELF/SymbolTable.cpp

void SymbolTable::handleAnonymousVersion() {
   for (SymbolVersion &Ver : Config->VersionScriptGlobals)
     assignWildcardVersion(Ver, VER_NDX_GLOBAL);
   for (SymbolVersion &Ver : Config->VersionScriptLocals)
     assignExactVersion(Ver, VER_NDX_LOCAL, "local");
   for (SymbolVersion &Ver : Config->VersionScriptLocals)
     assignWildcardVersion(Ver, VER_NDX_LOCAL);
 }
 
+// Handles -dynamic-list.
+void SymbolTable::handleDynamicList() {
+  for (SymbolVersion &Ver : Config->DynamicList) {
+    std::vector<SymbolBody *> Syms;
+    if (Ver.HasWildcard)
+      Syms = findByVersion(Ver);
+    else
+      Syms = findAllByVersion(Ver);
+
+    for (SymbolBody *B : Syms) {
+      if (!Config->Shared)
+        B->symbol()->VersionId = VER_NDX_GLOBAL;
+      else if (B->symbol()->includeInDynsym())
+        B->IsPreemptible = true;
+    }
+  }
+}
+
 // Set symbol versions to symbols. This function handles patterns
 // containing no wildcard characters.
 void SymbolTable::assignExactVersion(SymbolVersion Ver, uint16_t VersionId,
                                      StringRef VersionName) {
   if (Ver.HasWildcard)
     return;
 
   // Get a list of symbols which we need to assign the version to.
if (B->symbol()->VersionId == Config->DefaultSymbolVersion)
       B->symbol()->VersionId = VersionId;
 }
 
 // This function processes version scripts by updating VersionId
 // member of symbols.
 void SymbolTable::scanVersionScript() {
   // Handle edge cases first.
   handleAnonymousVersion();
+  handleDynamicList();
 
   // Now we have version definitions, so we need to set version ids to symbols.
   // Each version definition has a glob pattern, and all symbols that match
   // with the pattern get that version.
 
   // First, we assign versions to exact matching symbols,
   // i.e. version definitions not containing any glob meta-characters.
   for (VersionDefinition &V : Config->VersionDefinitions)

lld/trunk/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->DynamicList.empty())
     for (Symbol *S : Symtab->getSymbols())
       S->body()->IsPreemptible = computeIsPreemptible(*S->body());
 
   // 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())

lld/trunk/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; zed; };" > %t.list
+# RUN: echo "{ global: foo; bar; local: *; };" > %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-readobj -dyn-symbols  %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:      DynamicSymbols [
+# DYNSYMS-NEXT:   Symbol {
+# DYNSYMS-NEXT:     Name: @ (0)
+# DYNSYMS-NEXT:     Value: 0x0
+# DYNSYMS-NEXT:     Size: 0
+# DYNSYMS-NEXT:     Binding: Local
+# DYNSYMS-NEXT:     Type: None
+# DYNSYMS-NEXT:     Other: 0
+# DYNSYMS-NEXT:     Section: Undefined
+# DYNSYMS-NEXT:   }
+# DYNSYMS-NEXT:   Symbol {
+# DYNSYMS-NEXT:     Name: bar@
+# DYNSYMS-NEXT:     Value:
+# DYNSYMS-NEXT:     Size:
+# DYNSYMS-NEXT:     Binding: Global
+# DYNSYMS-NEXT:     Type:
+# DYNSYMS-NEXT:     Other:
+# DYNSYMS-NEXT:     Section:
+# DYNSYMS-NEXT:   }
+# DYNSYMS-NEXT:   Symbol {
+# DYNSYMS-NEXT:     Name: foo@
+# DYNSYMS-NEXT:     Value:
+# DYNSYMS-NEXT:     Size:
+# DYNSYMS-NEXT:     Binding: Global
+# DYNSYMS-NEXT:     Type:
+# DYNSYMS-NEXT:     Other:
+# DYNSYMS-NEXT:     Section:
+# DYNSYMS-NEXT:   }
+# DYNSYMS-NEXT: ]
+
+        .globl foo
+foo:
+        ret
+
+        .globl bar
+bar:
+        ret
+
+        .globl baz
+baz:
+        ret
+
+        .globl zed
+zed:
+        ret
+
+        call   foo@PLT
+        call   bar@PLT
+        call   baz@PLT
+        call   zed@PLT