[ELF] Simplify handling of exportDynamic and isPreemptible

In Writer::includeInDynSym(), exportDynamic is used by a Defined with
protected or default visibility, to record whether it is required to be
exported into .dynsym. It is set when any of the following conditions
hold:

1) There is an interposable symbol from a DSO (Undefined or SharedSymbol with default visibility)
2) If -shared or --export-dynamic is specified, any symbol in an object file/bitcode sets this property, unless suppressed by canBeOmittedFromSymbolTable().
3) --dynamic-list when producing an executable

4) protected symbol from a DSO preempted by copy relocation/canonical PLT when
  --ignore-{data,function}-address-equality is specified
5) ifunc is exported when -z ifunc-noplt is specified

Bullet points 4) and 5) are irrelevant in this patch.

Bullet 3) does not play well with 1) and 2). When -shared is specified,
exportDynamic of most symbols is true. This makes it incapable to record
--dynamic-list marked symbols. We thus have obscure:

    if (!config->shared)
      b->exportDynamic = true;
    else if (b->includeInDynsym())
      b->isPreemptible = true;

This patch adds another bit `Symbol::inDynamicList` to record
3). We can thus simplify handleDynamicList() by unifying the DSO and
  executable cases. It also allows us to simplify isPreemptible - now
the field is only used in finalizeSections() and later stages.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D66091

llvm-svn: 368659

lld/ELF/SymbolTable.cpp

@@ -77,6 +77,7 @@ Symbol *SymbolTable::insert(StringRef name) {
   sym->visibility = STV_DEFAULT;
   sym->isUsedInRegularObj = false;
   sym->exportDynamic = false;
+  sym->inDynamicList = false;
   sym->canInline = true;
   sym->scriptDefined = false;
   sym->partition = 1;
@@ -162,12 +163,8 @@ void SymbolTable::handleDynamicList() {
     else
       syms = findByVersion(ver);
 
-    for (Symbol *b : syms) {
-      if (!config->shared)
-        b->exportDynamic = true;
-      else if (b->includeInDynsym())
-        b->isPreemptible = true;
-    }
+    for (Symbol *sym : syms)
+      sym->inDynamicList = true;
   }
 }
 

lld/ELF/Symbols.cpp

@@ -295,7 +295,7 @@ bool Symbol::includeInDynsym() const {
   if (isUndefWeak() && config->pie && sharedFiles.empty())
     return false;
 
-  return isUndefined() || isShared() || exportDynamic;
+  return isUndefined() || isShared() || exportDynamic || inDynamicList;
 }
 
 // Print out a log message for --trace-symbol.

lld/ELF/Symbols.h

@@ -116,12 +116,21 @@ public:
   // are unreferenced except by other bitcode objects.
   unsigned isUsedInRegularObj : 1;
 
-  // If this flag is true and the symbol has protected or default visibility, it
-  // will appear in .dynsym. This flag is set by interposable DSO symbols in
-  // executables, by most symbols in DSOs and executables built with
-  // --export-dynamic, and by dynamic lists.
+  // Used by a Defined symbol with protected or default visibility, to record
+  // whether it is required to be exported into .dynsym. This is set when any of
+  // the following conditions hold:
+  //
+  // - If there is an interposable symbol from a DSO.
+  // - If -shared or --export-dynamic is specified, any symbol in an object
+  //   file/bitcode sets this property, unless suppressed by LTO
+  //   canBeOmittedFromSymbolTable().
   unsigned exportDynamic : 1;
 
+  // True if the symbol is in the --dynamic-list file. A Defined symbol with
+  // protected or default visibility with this property is required to be
+  // exported into .dynsym.
+  unsigned inDynamicList : 1;
+
   // False if LTO shouldn't inline whatever this symbol points to. If a symbol
   // is overwritten after LTO, LTO shouldn't inline the symbol because it
   // doesn't know the final contents of the symbol.
@@ -233,10 +242,11 @@ protected:
       : file(file), nameData(name.data), nameSize(name.size), binding(binding),
         type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
         isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
-        exportDynamic(isExportDynamic(k, visibility)), canInline(false),
-        referenced(false), traced(false), needsPltAddr(false), isInIplt(false),
-        gotInIgot(false), isPreemptible(false), used(!config->gcSections),
-        needsTocRestore(false), scriptDefined(false) {}
+        exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
+        canInline(false), referenced(false), traced(false), needsPltAddr(false),
+        isInIplt(false), gotInIgot(false), isPreemptible(false),
+        used(!config->gcSections), needsTocRestore(false),
+        scriptDefined(false) {}
 
 public:
   // True the symbol should point to its PLT entry.
@@ -531,6 +541,7 @@ void Symbol::replace(const Symbol &newSym) {
   visibility = old.visibility;
   isUsedInRegularObj = old.isUsedInRegularObj;
   exportDynamic = old.exportDynamic;
+  inDynamicList = old.inDynamicList;
   canInline = old.canInline;
   referenced = old.referenced;
   traced = old.traced;

lld/ELF/Writer.cpp

@@ -1653,13 +1653,13 @@ static bool computeIsPreemptible(const Symbol &b) {
   if (!b.isDefined())
     return true;
 
-  // If we have a dynamic list it specifies which local symbols are preemptible.
-  if (config->hasDynamicList)
-    return false;
-
   if (!config->shared)
     return false;
 
+  // If the dynamic list is present, it specifies preemptable symbols in a DSO.
+  if (config->hasDynamicList)
+    return b.inDynamicList;
+
   // -Bsymbolic means that definitions are not preempted.
   if (config->bsymbolic || (config->bsymbolicFunctions && b.isFunc()))
     return false;
@@ -1728,10 +1728,8 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
   for (Partition &part : partitions)
     finalizeSynthetic(part.ehFrame);
 
-  symtab->forEachSymbol([](Symbol *s) {
-    if (!s->isPreemptible)
-      s->isPreemptible = computeIsPreemptible(*s);
-  });
+  symtab->forEachSymbol(
+      [](Symbol *s) { s->isPreemptible = computeIsPreemptible(*s); });
 
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.