From baa45eb64cc395d453a65cca78880743e188a97f Mon Sep 17 00:00:00 2001
From: Peter Collingbourne <peter@pcc.me.uk>
Date: Fri, 3 Mar 2017 20:25:30 +0000
Subject: [PATCH] LTO: Hash the set of imported symbols for each module.

This set may affect code generation and is sensitive to link order (and
possibly in the future to the linker's choice of prevailing symbol), so we
need to include it.

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

llvm-svn: 296907
---
 lib/LTO/LTO.cpp                               | 20 +++++++++++++++-
 .../ThinLTO/X86/Inputs/cache-import-lists1.ll | 11 +++++++++
 .../ThinLTO/X86/Inputs/cache-import-lists2.ll | 11 +++++++++
 test/ThinLTO/X86/cache-import-lists.ll        | 24 +++++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 test/ThinLTO/X86/Inputs/cache-import-lists1.ll
 create mode 100644 test/ThinLTO/X86/Inputs/cache-import-lists2.ll
 create mode 100644 test/ThinLTO/X86/cache-import-lists.ll

diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp
index fe74c5f7d4b..a41ee607415 100644
--- a/lib/LTO/LTO.cpp
+++ b/lib/LTO/LTO.cpp
@@ -84,6 +84,18 @@ static void computeCacheKey(
     Data[3] = I >> 24;
     Hasher.update(ArrayRef<uint8_t>{Data, 4});
   };
+  auto AddUint64 = [&](uint64_t I) {
+    uint8_t Data[8];
+    Data[0] = I;
+    Data[1] = I >> 8;
+    Data[2] = I >> 16;
+    Data[3] = I >> 24;
+    Data[4] = I >> 32;
+    Data[5] = I >> 40;
+    Data[6] = I >> 48;
+    Data[7] = I >> 56;
+    Hasher.update(ArrayRef<uint8_t>{Data, 8});
+  };
   AddString(Conf.CPU);
   // FIXME: Hash more of Options. For now all clients initialize Options from
   // command-line flags (which is unsupported in production), but may set
@@ -112,10 +124,16 @@ static void computeCacheKey(
     // The export list can impact the internalization, be conservative here
     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&F, sizeof(F)));
 
-  // Include the hash for every module we import functions from
+  // Include the hash for every module we import functions from. The set of
+  // imported symbols for each module may affect code generation and is
+  // sensitive to link order, so include that as well.
   for (auto &Entry : ImportList) {
     auto ModHash = Index.getModuleHash(Entry.first());
     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
+
+    AddUint64(Entry.second.size());
+    for (auto &Fn : Entry.second)
+      AddUint64(Fn.first);
   }
 
   // Include the hash for the resolved ODR.
diff --git a/test/ThinLTO/X86/Inputs/cache-import-lists1.ll b/test/ThinLTO/X86/Inputs/cache-import-lists1.ll
new file mode 100644
index 00000000000..58bfb39f9ee
--- /dev/null
+++ b/test/ThinLTO/X86/Inputs/cache-import-lists1.ll
@@ -0,0 +1,11 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f1() {
+  call void @linkonce_odr()
+  ret void
+}
+
+define linkonce_odr void @linkonce_odr() {
+  ret void
+}
diff --git a/test/ThinLTO/X86/Inputs/cache-import-lists2.ll b/test/ThinLTO/X86/Inputs/cache-import-lists2.ll
new file mode 100644
index 00000000000..899bbaea13d
--- /dev/null
+++ b/test/ThinLTO/X86/Inputs/cache-import-lists2.ll
@@ -0,0 +1,11 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f2() {
+  call void @linkonce_odr()
+  ret void
+}
+
+define linkonce_odr void @linkonce_odr() {
+  ret void
+}
diff --git a/test/ThinLTO/X86/cache-import-lists.ll b/test/ThinLTO/X86/cache-import-lists.ll
new file mode 100644
index 00000000000..6ec5dba1089
--- /dev/null
+++ b/test/ThinLTO/X86/cache-import-lists.ll
@@ -0,0 +1,24 @@
+; RUN: opt -module-hash -module-summary %s -o %t.bc
+; RUN: opt -module-hash -module-summary %S/Inputs/cache-import-lists1.ll -o %t1.bc
+; RUN: opt -module-hash -module-summary %S/Inputs/cache-import-lists2.ll -o %t2.bc
+
+; Tests that the hash for t is sensitive to the set of imported functions
+; for each module, which in this case depends on the link order (the function
+; linkonce_odr will be imported from either t1 or t2, whichever comes first).
+
+; RUN: rm -rf %t.cache
+; RUN: llvm-lto2 -cache-dir %t.cache -o %t.o %t.bc %t1.bc %t2.bc -r=%t.bc,main,plx -r=%t.bc,f1,lx -r=%t.bc,f2,lx -r=%t1.bc,f1,plx -r=%t1.bc,linkonce_odr,plx -r=%t2.bc,f2,plx -r=%t2.bc,linkonce_odr,lx
+; RUN: llvm-lto2 -cache-dir %t.cache -o %t.o %t.bc %t2.bc %t1.bc -r=%t.bc,main,plx -r=%t.bc,f1,lx -r=%t.bc,f2,lx -r=%t2.bc,f2,plx -r=%t2.bc,linkonce_odr,plx -r=%t1.bc,f1,plx -r=%t1.bc,linkonce_odr,lx
+; RUN: ls %t.cache | count 6
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @main() {
+  call void @f1()
+  call void @f2()
+  ret void
+}
+
+declare void @f1()
+declare void @f2()