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

[Rui Ueyama]

  - Updated as per Nick's comments.

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2161?vs=5492&id=5560#toc

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

Index: lib/Core/SymbolTable.cpp
===================================================================
--- lib/Core/SymbolTable.cpp
+++ lib/Core/SymbolTable.cpp
@@ -172,28 +172,42 @@
     }
     break;
   case NCR_DupUndef: {
-      const UndefinedAtom* existingUndef =
-        dyn_cast<UndefinedAtom>(existing);
-      const UndefinedAtom* newUndef =
-        dyn_cast<UndefinedAtom>(&newAtom);
-      assert(existingUndef != nullptr);
-      assert(newUndef != nullptr);
-      if (existingUndef->canBeNull() == newUndef->canBeNull()) {
-        useNew = false;
-      } 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());
-      }
+    const UndefinedAtom* existingUndef = dyn_cast<UndefinedAtom>(existing);
+    const UndefinedAtom* newUndef = dyn_cast<UndefinedAtom>(&newAtom);
+    assert(existingUndef != nullptr);
+    assert(newUndef != nullptr);
+
+    bool sameCanBeNull = (existingUndef->canBeNull() == newUndef->canBeNull());
+    if (!sameCanBeNull &&
+        _context.warnIfCoalesableAtomsHaveDifferentCanBeNull()) {
+      llvm::errs() << "lld warning: undefined symbol "
+                   << existingUndef->name()
+                   << " has different weakness in "
+                   << existingUndef->file().path()
+                   << " and in " << newUndef->file().path() << "\n";
+    }
+
+    const UndefinedAtom *existingFallback = existingUndef->fallback();
+    const UndefinedAtom *newFallback = newUndef->fallback();
+    bool hasDifferentFallback =
+        (existingFallback && newFallback &&
+         existingFallback->name() != newFallback->name());
+    if (hasDifferentFallback) {
+      llvm::errs() << "lld warning: undefined symbol "
+                   << existingUndef->name() << " has different fallback: "
+                   << existingFallback->name() << " in "
+                   << existingUndef->file().path() << " and "
+                   << newFallback->name() << " in "
+                   << newUndef->file().path() << "\n";
     }
+
+    bool hasNewFallback = newUndef->fallback();
+    if (sameCanBeNull)
+      useNew = hasNewFallback;
+    else
+      useNew = (newUndef->canBeNull() < existingUndef->canBeNull());
     break;
+  }
   case NCR_DupShLib: {
       const SharedLibraryAtom* curShLib =
         dyn_cast<SharedLibraryAtom>(existing);
Index: test/core/undef-weak-coalesce.objtxt
===================================================================
--- test/core/undef-weak-coalesce.objtxt
+++ test/core/undef-weak-coalesce.objtxt
@@ -1,4 +1,5 @@
-# RUN: lld -core %s | FileCheck %s
+# RUN: lld -core %s 2> %t.err | FileCheck %s
+# RUN: FileCheck -check-prefix=ERROR %s < %t.err
 
 #
 # Test that undefined symbols preserve their attributes and merge properly
@@ -8,86 +9,75 @@
 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
-      
+      fallback:
+        name: baz1
     - name:              bar9
       can-be-null:       at-buildtime
-      
+      fallback:
+        name: baz2
 ---
 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
+      fallback:
+        name: baz3
 ...
 
-# 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:            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:            bar4
+# CHECK-NEXT:  - name:            bar5
+# CHECK-NEXT:    can-be-null:     at-runtime
+# CHECK-NEXT:  - name:            bar7
+# CHECK-NEXT:  - name:            bar6
+# CHECK-NEXT:  - name:            bar8
+# CHECK-NEXT:  - name:            bar9
+# CHECK-NEXT:    can-be-null:     at-runtime
+# CHECK-NEXT:    fallback:
+# CHECK-NEXT:      name:            baz3
+
+# ERROR: undefined symbol bar9 has different fallback: baz2 in and baz3 in
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2161.2.patch
Type: text/x-patch
Size: 6581 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131114/7efbb88a/attachment.bin>