[PATCH] Select new undefined atom rather than old one if other conditions are the same.

[Rui Ueyama]

We can add multiple undefined atoms having the same name to the symbol table.
If such atoms are added, the symbol table compares their canBeNull attributes,
and select one having a stronger constraint. If their canBeNulls are the same,
the choice is arbitrary. Currently it choose the existing one.

This patch changes the preference, so that the symbol table choose the new one
rather than the existing one if canBeNulls are the same. This shouldn't change
the behavior except the case described below.

A new undefined atom may have a new fallback atom attribute. By choosing the new
atom, we can update the fallback atom during Core Linking. PE/COFF actually need
that. For example, _lseek is an alias for __lseek on Windows. One of an object
file in OLDNAMES.LIB has an undefined atom for _lseek with the fallback to
__lseek. When the linker tries to resolve _read, it supposed to read the file
from OLDNAMES.LIB and use the new fallback from the file. Currently LLD cannot
handle such case because duplicate undefined atoms with the same attributes are
ignored.

http://llvm-reviews.chandlerc.com/D2161

Files:
  lib/Core/SymbolTable.cpp
  test/core/undef-weak-coalesce.objtxt

Index: lib/Core/SymbolTable.cpp
===================================================================
--- lib/Core/SymbolTable.cpp
+++ lib/Core/SymbolTable.cpp
@@ -178,20 +178,34 @@
             dyn_cast<UndefinedAtom>(&newAtom);
           assert(existingUndef != nullptr);
           assert(newUndef != nullptr);
-          if ( existingUndef->canBeNull() == newUndef->canBeNull() ) {
-            useNew = false;
+          if (existingUndef->canBeNull() != newUndef->canBeNull() &&
+              _context.warnIfCoalesableAtomsHaveDifferentCanBeNull()) {
+            // FIXME: need diagonstics interface for writing warning messages
+            llvm::errs() << "lld warning: undefined symbol "
+                         << existingUndef->name()
+                         << " has different weakness in "
+                         << existingUndef->file().path()
+                         << " and in "
+                         << newUndef->file().path();
           }
-          else {
-            if (_context.warnIfCoalesableAtomsHaveDifferentCanBeNull()) {
-              // FIXME: need diagonstics interface for writing warning messages
-              llvm::errs() << "lld warning: undefined symbol "
-                           << existingUndef->name()
-                           << " has different weakness in "
-                           << existingUndef->file().path()
-                           << " and in "
-                           << newUndef->file().path();
-            }
-            useNew = (newUndef->canBeNull() < existingUndef->canBeNull());
+          useNew = (newUndef->canBeNull() <= existingUndef->canBeNull());
+
+          // Warn if the fallback atom is being overriden.
+          const UndefinedAtom *existingFallback = existingUndef->fallback();
+          if (useNew && existingFallback) {
+            const UndefinedAtom *newFallback = newUndef->fallback();
+            if (!newFallback) {
+              llvm::errs() << "lld warning: new undefined atom removes the "
+                           << "existing fallback atom "
+                           << existingUndef->name() << " ("
+                           << existingFallback->name() << ")";
+            } else if (existingFallback->name() != newFallback->name())
+              llvm::errs() << "lld warning: new undefined atom "
+                           << newUndef->name() << " ("
+                           << newFallback->name() << ")"
+                           << " shadows the existing fallback atom "
+                           << existingUndef->name() << " ("
+                           << existingFallback->name() << ")";
           }
         }
         break;
Index: test/core/undef-weak-coalesce.objtxt
===================================================================
--- test/core/undef-weak-coalesce.objtxt
+++ test/core/undef-weak-coalesce.objtxt
@@ -8,86 +8,66 @@
 undefined-atoms:
     - name:              regular_func
       can-be-null:       never
-
     - name:              weak_import_func
       can-be-null:       at-runtime
-      
     - name:              weak_func
       can-be-null:       at-buildtime
-      
     - name:              bar1
       can-be-null:       never
-      
     - name:              bar2
       can-be-null:       at-runtime
-      
     - name:              bar3
       can-be-null:       at-buildtime
-      
     - name:              bar4
       can-be-null:       never
-      
     - name:              bar5
       can-be-null:       at-runtime
-      
     - name:              bar6
       can-be-null:       at-buildtime
-      
     - name:              bar7
       can-be-null:       never
-      
     - name:              bar8
       can-be-null:       at-runtime
-      
     - name:              bar9
       can-be-null:       at-buildtime
-      
 ---
 undefined-atoms:
     - name:              bar1
       can-be-null:       never
-      
     - name:              bar2
       can-be-null:       at-runtime
-      
     - name:              bar3
       can-be-null:       at-buildtime
-      
     - name:              bar4
       can-be-null:       at-runtime
-      
     - name:              bar5
       can-be-null:       at-buildtime
-      
     - name:              bar6
       can-be-null:       never
-      
     - name:              bar7
       can-be-null:       at-buildtime
-      
     - name:              bar8
       can-be-null:       never
-      
     - name:              bar9
       can-be-null:       at-runtime
 ...
 
-# CHECK:   - name:       regular_func
-# CHECK:   - name:       weak_import_func
-# CHECK:     can-be-null:  at-runtime
-# CHECK:   - name:       weak_func
-# CHECK:     can-be-null:  at-buildtime
-# CHECK:   - name:       bar1
-# CHECK:   - name:       bar2
-# CHECK:     can-be-null:  at-runtime
-# CHECK:   - name:       bar3
-# CHECK:     can-be-null:  at-buildtime
-# CHECK:   - name:       bar4
-# CHECK:   - name:       bar5
-# CHECK:     can-be-null:  at-runtime
-# CHECK:   - name:       bar7
-# CHECK:   - name:       bar6
-# CHECK:   - name:       bar8
-# CHECK:   - name:       bar9
-# CHECK:     can-be-null:  at-runtime
-# CHECK:       ...
+# CHECK:        - name:       regular_func
+# CHECK-NEXT:   - name:       weak_import_func
+# CHECK-NEXT:     can-be-null:  at-runtime
+# CHECK-NEXT:   - name:       weak_func
+# CHECK-NEXT:     can-be-null:  at-buildtime
+# CHECK-NEXT:   - name:       bar4
+# CHECK-NEXT:   - name:       bar5
+# CHECK-NEXT:     can-be-null:  at-runtime
+# CHECK-NEXT:   - name:       bar7
+# CHECK-NEXT:   - name:       bar1
+# CHECK-NEXT:   - name:       bar2
+# CHECK-NEXT:     can-be-null:  at-runtime
+# CHECK-NEXT:   - name:       bar3
+# CHECK-NEXT:     can-be-null:  at-buildtime
+# CHECK-NEXT:   - name:       bar6
+# CHECK-NEXT:   - name:       bar8
+# CHECK-NEXT:   - name:       bar9
+# CHECK-NEXT:     can-be-null:  at-runtime
+# CHECK-NEXT:       ...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2161.1.patch
Type: text/x-patch
Size: 5936 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131112/78748efd/attachment.bin>