Fix thinlto cache key computation for cfi-icall.

Summary:
Fixed PR33966.

CFI code generation for users (not just callers) of a function depends
on whether this function has a jumptable entry or not. This
information needs to be encoded in of thinlto cache key.

We filter the jumptable list against functions that are actually
referenced in the current module.

Subscribers: mehdi_amini, inglorion, eraman, hiraditya

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

llvm-svn: 310536

lib/LTO/LTO.cpp

@@ -65,7 +65,9 @@ static void computeCacheKey(
     const FunctionImporter::ExportSetTy &ExportList,
     const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
     const GVSummaryMapTy &DefinedGlobals,
-    const TypeIdSummariesByGuidTy &TypeIdSummariesByGuid) {
+    const TypeIdSummariesByGuidTy &TypeIdSummariesByGuid,
+    const std::set<GlobalValue::GUID> &CfiFunctionDefs,
+    const std::set<GlobalValue::GUID> &CfiFunctionDecls) {
   // Compute the unique hash for this entry.
   // This is based on the current compiler version, the module itself, the
   // export list, the hash for every single module in the import list, the
@@ -158,22 +160,39 @@ static void computeCacheKey(
                                     sizeof(GlobalValue::LinkageTypes)));
   }
 
+  // Members of CfiFunctionDefs and CfiFunctionDecls that are referenced or
+  // defined in this module.
+  std::set<GlobalValue::GUID> UsedCfiDefs;
+  std::set<GlobalValue::GUID> UsedCfiDecls;
+
+  // Typeids used in this module.
   std::set<GlobalValue::GUID> UsedTypeIds;
 
-  auto AddUsedTypeIds = [&](GlobalValueSummary *GS) {
-    auto *FS = dyn_cast_or_null<FunctionSummary>(GS);
-    if (!FS)
-      return;
-    for (auto &TT : FS->type_tests())
-      UsedTypeIds.insert(TT);
-    for (auto &TT : FS->type_test_assume_vcalls())
-      UsedTypeIds.insert(TT.GUID);
-    for (auto &TT : FS->type_checked_load_vcalls())
-      UsedTypeIds.insert(TT.GUID);
-    for (auto &TT : FS->type_test_assume_const_vcalls())
-      UsedTypeIds.insert(TT.VFunc.GUID);
-    for (auto &TT : FS->type_checked_load_const_vcalls())
-      UsedTypeIds.insert(TT.VFunc.GUID);
+  auto AddUsedCfiGlobal = [&](GlobalValue::GUID ValueGUID) {
+    if (CfiFunctionDefs.count(ValueGUID))
+      UsedCfiDefs.insert(ValueGUID);
+    if (CfiFunctionDecls.count(ValueGUID))
+      UsedCfiDecls.insert(ValueGUID);
+  };
+
+  auto AddUsedThings = [&](GlobalValueSummary *GS) {
+    if (!GS) return;
+    for (const ValueInfo &VI : GS->refs())
+      AddUsedCfiGlobal(VI.getGUID());
+    if (auto *FS = dyn_cast<FunctionSummary>(GS)) {
+      for (auto &TT : FS->type_tests())
+        UsedTypeIds.insert(TT);
+      for (auto &TT : FS->type_test_assume_vcalls())
+        UsedTypeIds.insert(TT.GUID);
+      for (auto &TT : FS->type_checked_load_vcalls())
+        UsedTypeIds.insert(TT.GUID);
+      for (auto &TT : FS->type_test_assume_const_vcalls())
+        UsedTypeIds.insert(TT.VFunc.GUID);
+      for (auto &TT : FS->type_checked_load_const_vcalls())
+        UsedTypeIds.insert(TT.VFunc.GUID);
+      for (auto &ET : FS->calls())
+        AddUsedCfiGlobal(ET.first.getGUID());
+    }
   };
 
   // Include the hash for the linkage type to reflect internalization and weak
@@ -182,14 +201,15 @@ static void computeCacheKey(
     GlobalValue::LinkageTypes Linkage = GS.second->linkage();
     Hasher.update(
         ArrayRef<uint8_t>((const uint8_t *)&Linkage, sizeof(Linkage)));
-    AddUsedTypeIds(GS.second);
+    AddUsedCfiGlobal(GS.first);
+    AddUsedThings(GS.second);
   }
 
   // Imported functions may introduce new uses of type identifier resolutions,
   // so we need to collect their used resolutions as well.
   for (auto &ImpM : ImportList)
     for (auto &ImpF : ImpM.second)
-      AddUsedTypeIds(Index.findSummaryInModule(ImpF.first, ImpM.first()));
+      AddUsedThings(Index.findSummaryInModule(ImpF.first, ImpM.first()));
 
   auto AddTypeIdSummary = [&](StringRef TId, const TypeIdSummary &S) {
     AddString(TId);
@@ -222,6 +242,14 @@ static void computeCacheKey(
         AddTypeIdSummary(Summary->first, Summary->second);
   }
 
+  AddUnsigned(UsedCfiDefs.size());
+  for (auto &V : UsedCfiDefs)
+    AddUint64(V);
+
+  AddUnsigned(UsedCfiDecls.size());
+  for (auto &V : UsedCfiDecls)
+    AddUint64(V);
+
   if (!Conf.SampleProfile.empty()) {
     auto FileOrErr = MemoryBuffer::getFile(Conf.SampleProfile);
     if (FileOrErr)
@@ -815,6 +843,8 @@ class InProcessThinBackend : public ThinBackendProc {
   AddStreamFn AddStream;
   NativeObjectCache Cache;
   TypeIdSummariesByGuidTy TypeIdSummariesByGuid;
+  std::set<GlobalValue::GUID> CfiFunctionDefs;
+  std::set<GlobalValue::GUID> CfiFunctionDecls;
 
   Optional<Error> Err;
   std::mutex ErrMu;
@@ -834,6 +864,12 @@ public:
     // each function without needing to compute GUIDs in each backend.
     for (auto &TId : CombinedIndex.typeIds())
       TypeIdSummariesByGuid[GlobalValue::getGUID(TId.first)].push_back(&TId);
+    for (auto &Name : CombinedIndex.cfiFunctionDefs())
+      CfiFunctionDefs.insert(
+          GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Name)));
+    for (auto &Name : CombinedIndex.cfiFunctionDecls())
+      CfiFunctionDecls.insert(
+          GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Name)));
   }
 
   Error runThinLTOBackendThread(
@@ -867,7 +903,8 @@ public:
     SmallString<40> Key;
     // The module may be cached, this helps handling it.
     computeCacheKey(Key, Conf, CombinedIndex, ModuleID, ImportList, ExportList,
-                    ResolvedODR, DefinedGlobals, TypeIdSummariesByGuid);
+                    ResolvedODR, DefinedGlobals, TypeIdSummariesByGuid,
+                    CfiFunctionDefs, CfiFunctionDecls);
     if (AddStreamFn CacheAddStream = Cache(Task, Key))
       return RunThinBackend(CacheAddStream);
 

test/ThinLTO/X86/Inputs/cache-icall.ll

@@ -0,0 +1,9 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f() !type !0 {
+entry:
+  ret void
+}
+
+!0 = !{i64 0, !"_ZTSFvvE"}

test/ThinLTO/X86/cache-icall.ll

@@ -0,0 +1,55 @@
+; Test that the list of CFI jumptable entries is part of ThinLTO cache key.
+
+; Linking Inputs/cache-icall.ll results in f() being added to CFI jumptable; otherwise it is not.
+; This affects code generated for any users of f(). Make sure that we don't pull a stale object
+; file for %t.o from the cache.
+
+; RUN: opt -module-hash -module-summary -thinlto-bc %s -o %t.bc
+; RUN: opt -module-hash -module-summary -thinlto-bc %p/Inputs/cache-icall.ll -o %t2.bc
+
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+
+; RUN: llvm-lto2 run -o %t-no.o %t.bc -cache-dir %t.cache \
+; RUN:   -r=%t.bc,_start,px \
+; RUN:   -r=%t.bc,f,
+
+; RUN: llvm-readelf -symbols %t-no.o.* | FileCheck %s --check-prefix=SYMBOLS-NO
+
+; RUN: llvm-lto2 run -o %t-yes.o %t.bc %t2.bc -cache-dir %t.cache \
+; RUN:   -r=%t.bc,_start,px \
+; RUN:   -r=%t.bc,f, \
+; RUN:   -r=%t2.bc,f,p
+
+; RUN: llvm-readelf -symbols %t-yes.o.* | FileCheck %s --check-prefix=SYMBOLS-YES
+
+; SYMBOLS-NO-DAG: {{FUNC .* f.cfi_jt$}}
+; SYMBOLS-NO-DAG: {{NOTYPE .* UND f.cfi_jt$}}
+
+; SYMBOLS-YES-NOT: f.cfi_jt
+; SYMBOLS-YES-DAG: {{FUNC .* f.cfi$}}
+; SYMBOLS-YES-DAG: {{NOTYPE .* UND f.cfi$}}
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8* @_start(void ()* %p) !type !0 {
+entry:
+  %0 = bitcast void ()* %p to i8*
+  %1 = tail call i1 @llvm.type.test(i8* %0, metadata !"_ZTSFvvE")
+  br i1 %1, label %cont, label %trap
+
+trap:                                             ; preds = %entry
+  tail call void @llvm.trap()
+  unreachable
+
+cont:                                             ; preds = %entry
+  tail call void %p()
+  ret i8* bitcast (void ()* @f to i8*)
+}
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.trap()
+declare !type !1 void @f()
+
+!0 = !{i64 0, !"_ZTSFPvPFvvEE"}
+!1 = !{i64 0, !"_ZTSFvvE"}