[ELF] Make binding (weak or non-weak) logic consistent for Undefined and SharedSymbol

This is a case missed by D64136. If %t1.o has a weak reference on foo,
and %t2.so has a non-weak reference on foo:

```
0. ld.lld %t1.o %t2.so          # ok; STB_WEAK; accepted since D64136
1. ld.lld %t2.so %t1.o          # undefined symbol: foo; STB_GLOBAL
2. gold %t1.o %t2.so            # ok; STB_WEAK
3. gold %t2.so %t1.o            # undefined reference to 'foo'; STB_GLOBAL
4. ld.bfd %t1.o %t2.so          # undefined reference to `foo'; STB_WEAK
5. ld.bfd %t2.so %t1.o          # undefined reference to `foo'; STB_WEAK
```

It can be argued that in both cases, the binding of the undefined foo
should be set to STB_WEAK, because the binding should not be affected by
referenced from shared objects.

--allow-shlib-undefined doesn't suppress errors (3,4,5), but -shared or
--noinhibit-exec allows ld.bfd/gold to produce a binary:

```
3. gold -shared %t2.so %t1.o    # ok; STB_GLOBAL
4. ld.bfd -shared %t2.so %t1.o  # ok; STB_WEAK
5. ld.bfd -shared %t1.o %t1.o   # ok; STB_WEAK
```

If %t2.so has DT_NEEDED entries, ld.bfd will load them (lld/gold don't
have the behavior). If one of the DSO defines foo and it is in the
link-time search path (e.g. DT_NEEDED entry is an absolute path, via
-rpath=, via -rpath-link=, etc),
`ld.bfd %t1.o %t2.so` and `ld.bfd %t1.o %t2.so` will not error.

In this patch, we make Undefined and SharedSymbol share the same binding
computing logic. Case 1 will be allowed:

```
0. ld.lld %t1.o %t2.so          # ok; STB_WEAK; accepted since D64136
1. ld.lld %t2.so %t1.o          # ok; STB_WEAK; changed by this patch
```

In the future, we can explore the option that turns both (0,1) into
errors if --no-allow-shlib-undefined (default when linking an
executable) is in action.

Reviewed By: ruiu

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

llvm-svn: 368038

lld/ELF/InputFiles.cpp

@@ -1098,6 +1098,7 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
     // Handle global undefined symbols.
     if (eSym.st_shndx == SHN_UNDEF) {
       this->symbols[i]->resolve(Undefined{this, name, binding, stOther, type});
+      this->symbols[i]->referenced = true;
       continue;
     }
 

lld/ELF/Symbols.cpp

@@ -492,17 +492,13 @@ void Symbol::resolveUndefined(const Undefined &other) {
   if (dyn_cast_or_null<SharedFile>(other.file))
     return;
 
-  if (isUndefined()) {
-    // The binding may "upgrade" from weak to non-weak.
-    if (other.binding != STB_WEAK)
+  if (isUndefined() || isShared()) {
+    // The binding will be weak if there is at least one reference and all are
+    // weak. The binding has one opportunity to change to weak: if the first
+    // reference is weak.
+    if (other.binding != STB_WEAK || !referenced)
       binding = other.binding;
-  } else if (auto *s = dyn_cast<SharedSymbol>(this)) {
-    // The binding of a SharedSymbol will be weak if there is at least one
-    // reference and all are weak. The binding has one opportunity to change to
-    // weak: if the first reference is weak.
-    if (other.binding != STB_WEAK || !s->referenced)
-      binding = other.binding;
-    s->referenced = true;
+    referenced = true;
   }
 }
 
@@ -658,6 +654,6 @@ void Symbol::resolveShared(const SharedSymbol &other) {
     uint8_t bind = binding;
     replace(other);
     binding = bind;
-    cast<SharedSymbol>(this)->referenced = true;
+    referenced = true;
   }
 }

lld/ELF/Symbols.h

@@ -127,6 +127,11 @@ public:
   // doesn't know the final contents of the symbol.
   unsigned canInline : 1;
 
+  // Used by Undefined and SharedSymbol to track if there has been at least one
+  // undefined reference to the symbol. The binding may change to STB_WEAK if
+  // the first undefined reference from a non-shared object is weak.
+  unsigned referenced : 1;
+
   // True if this symbol is specified by --trace-symbol option.
   unsigned traced : 1;
 
@@ -229,9 +234,9 @@ protected:
         type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
         isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
         exportDynamic(isExportDynamic(k, visibility)), canInline(false),
-        traced(false), needsPltAddr(false), isInIplt(false), gotInIgot(false),
-        isPreemptible(false), used(!config->gcSections), needsTocRestore(false),
-        scriptDefined(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.
@@ -367,11 +372,6 @@ public:
   uint64_t value; // st_value
   uint64_t size;  // st_size
   uint32_t alignment;
-
-  // This is true if there has been at least one undefined reference to the
-  // symbol. The binding may change to STB_WEAK if the first undefined reference
-  // is weak.
-  bool referenced = false;
 };
 
 // LazyArchive and LazyObject represent a symbols that is not yet in the link,
@@ -535,6 +535,7 @@ void Symbol::replace(const Symbol &New) {
   isUsedInRegularObj = old.isUsedInRegularObj;
   exportDynamic = old.exportDynamic;
   canInline = old.canInline;
+  referenced = old.referenced;
   traced = old.traced;
   isPreemptible = old.isPreemptible;
   scriptDefined = old.scriptDefined;

lld/test/ELF/weak-undef-shared.s

@@ -30,6 +30,9 @@
 # RUN: ld.lld %t1.o %t2.so -o %t
 # RUN: llvm-readelf --dyn-syms %t | FileCheck --check-prefix=WEAK %s
 
+# RUN: ld.lld %t2.so %t1.o -o %t
+# RUN: llvm-readelf --dyn-syms %t | FileCheck --check-prefix=WEAK %s
+
 # WEAK:   NOTYPE WEAK   DEFAULT UND foo
 # GLOBAL: NOTYPE GLOBAL DEFAULT UND foo