[lld] r241294 - COFF: Make symbols satisfy weak ordering.

[Rui Ueyama]

Author: ruiu
Date: Thu Jul  2 15:33:48 2015
New Revision: 241294

URL: http://llvm.org/viewvc/llvm-project?rev=241294&view=rev
Log:
COFF: Make symbols satisfy weak ordering.

Previously, SymbolBody::compare(A, B) didn't satisfy weak ordering.
There was a case that A < B and B < A could have been true.
This is because we just pick LHS if A and B are consisdered equivalent.

This patch is to make symbols being weakly ordered. If A and B are
not tie, one of A < B && B > A or A > B && B < A is true.
This is not an improtant property for a single-threaded environment
because everything is deterministic anyways. However, in a multi-
threaded environment, this property becomes important.

If a symbol is defined or lazy, ties are resolved by its file index.
For simple types that we don't really care about their identities,
symbols are compared by their addresses.

Modified:
    lld/trunk/COFF/InputFiles.cpp
    lld/trunk/COFF/InputFiles.h
    lld/trunk/COFF/Symbols.cpp
    lld/trunk/COFF/Symbols.h

Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=241294&r1=241293&r2=241294&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Thu Jul  2 15:33:48 2015
@@ -32,6 +32,8 @@ using llvm::sys::fs::file_magic;
 namespace lld {
 namespace coff {
 
+int InputFile::NextIndex = 0;
+
 // Returns the last element of a path, which is supposed to be a filename.
 static StringRef getBasename(StringRef Path) {
   size_t Pos = Path.find_last_of("\\/");

Modified: lld/trunk/COFF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=241294&r1=241293&r2=241294&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.h (original)
+++ lld/trunk/COFF/InputFiles.h Thu Jul  2 15:33:48 2015
@@ -62,8 +62,15 @@ public:
   // Returns .drectve section contents if exist.
   StringRef getDirectives() { return StringRef(Directives).trim(); }
 
+  // Each file has a unique index. The index number is used to
+  // resolve ties in symbol resolution.
+  int Index;
+  static int NextIndex;
+
 protected:
-  InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
+  InputFile(Kind K, MemoryBufferRef M)
+      : Index(NextIndex++), MB(M), FileKind(K) {}
+
   MemoryBufferRef MB;
   std::string Directives;
 

Modified: lld/trunk/COFF/Symbols.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.cpp?rev=241294&r1=241293&r2=241294&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.cpp (original)
+++ lld/trunk/COFF/Symbols.cpp Thu Jul  2 15:33:48 2015
@@ -47,7 +47,6 @@ int SymbolBody::compare(SymbolBody *Othe
 
   // First handle comparisons between two different kinds.
   if (LK != RK) {
-
     if (RK > LastDefinedKind) {
       if (LK == LazyKind && cast<Undefined>(Other)->WeakAlias)
         return -1;
@@ -94,12 +93,16 @@ int SymbolBody::compare(SymbolBody *Othe
   case DefinedRegularKind: {
     auto *LHS = cast<DefinedRegular>(this);
     auto *RHS = cast<DefinedRegular>(Other);
-    return (LHS->isCOMDAT() && RHS->isCOMDAT()) ? 1 : 0;
+    if (LHS->isCOMDAT() && RHS->isCOMDAT())
+      return LHS->getFileIndex() < RHS->getFileIndex() ? 1 : -1;
+    return 0;
   }
 
   case DefinedCommonKind: {
     auto *LHS = cast<DefinedCommon>(this);
     auto *RHS = cast<DefinedCommon>(Other);
+    if (LHS->getSize() == RHS->getSize())
+      return LHS->getFileIndex() < RHS->getFileIndex() ? 1 : -1;
     return LHS->getSize() > RHS->getSize() ? 1 : -1;
   }
 
@@ -111,17 +114,30 @@ int SymbolBody::compare(SymbolBody *Othe
       return 0;
 
     // Non-replaceable symbols win, but even two replaceable symboles don't
-    // tie.
+    // tie. If both symbols are replaceable, choice is arbitrary.
+    if (RHS->IsReplaceable && LHS->IsReplaceable)
+      return uintptr_t(LHS) < uintptr_t(RHS) ? 1 : -1;
     return LHS->IsReplaceable ? -1 : 1;
   }
 
-  case LazyKind:
-    // Don't tie, just pick the LHS.
-    return 1;
-
-  case UndefinedKind:
-    // Don't tie, just pick the LHS unless the RHS has a weak alias.
-    return cast<Undefined>(Other)->WeakAlias ? -1 : 1;
+  case LazyKind: {
+    // Don't tie, pick the earliest.
+    auto *LHS = cast<Lazy>(this);
+    auto *RHS = cast<Lazy>(Other);
+    return LHS->getFileIndex() < RHS->getFileIndex() ? 1 : -1;
+  }
+
+  case UndefinedKind: {
+    auto *LHS = cast<Undefined>(this);
+    auto *RHS = cast<Undefined>(Other);
+    // Tie if both undefined symbols have different weak aliases.
+    if (LHS->WeakAlias && RHS->WeakAlias) {
+      if (LHS->WeakAlias->repl() != RHS->WeakAlias->repl())
+        return 0;
+      return uintptr_t(LHS) < uintptr_t(RHS) ? 1 : -1;
+    }
+    return LHS->WeakAlias ? 1 : -1;
+  }
 
   case DefinedLocalImportKind:
   case DefinedImportThunkKind:

Modified: lld/trunk/COFF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.h?rev=241294&r1=241293&r2=241294&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.h (original)
+++ lld/trunk/COFF/Symbols.h Thu Jul  2 15:33:48 2015
@@ -139,6 +139,8 @@ public:
     return S->kind() <= LastDefinedCOFFKind;
   }
 
+  int getFileIndex() { return File->Index; }
+
 protected:
   ObjectFile *File;
   const coff_symbol_generic *Sym;
@@ -231,6 +233,8 @@ public:
   // was already returned.
   ErrorOr<std::unique_ptr<InputFile>> getMember();
 
+  int getFileIndex() { return File->Index; }
+
 private:
   ArchiveFile *File;
   const Archive::Symbol Sym;