From 65813edfe2421ae3810f54e65a551464aa6fe0cb Mon Sep 17 00:00:00 2001
From: Rui Ueyama <ruiu@google.com>
Date: Thu, 2 Jul 2015 20:33:48 +0000
Subject: [PATCH] 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.

llvm-svn: 241294
---
 lld/COFF/InputFiles.cpp |  2 ++
 lld/COFF/InputFiles.h   |  9 ++++++++-
 lld/COFF/Symbols.cpp    | 34 +++++++++++++++++++++++++---------
 lld/COFF/Symbols.h      |  4 ++++
 4 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 20cc25f66922..395d51067c79 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -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("\\/");
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index d4f56aa8642b..40df27aef79e 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -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;
 
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index 58eb1908043c..baf60e97932d 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -47,7 +47,6 @@ int SymbolBody::compare(SymbolBody *Other) {
 
   // 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 *Other) {
   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 *Other) {
       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 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:
-    // Don't tie, just pick the LHS unless the RHS has a weak alias.
-    return cast<Undefined>(Other)->WeakAlias ? -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:
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index 7eac93dfc68e..459d04c47348 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -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;