LTO: Export functions referenced by non-canonical CFI jump tables

[samitolvanen]

Summary

LowerTypeTests pass adds functions with a non-canonical jump table
to cfiFunctionDecls instead of cfiFunctionDefs. As the jump table
is in the regular LTO object, these functions will also need to be
exported. This change fixes the non-canonical jump table case and
adds a test similar to the existing one for canonical jump tables.

[kees]

PoC from Sami:
https://godbolt.org/z/xfsWjhGhq

This change looks sane to me.

[nickdesaulniers]

Is this related to b/158034873 or another reported bug?

[tejohnson]

Adding another possible reviewer since @eugenis added these data structures (D34168). With the caveat that I'm not very familiar at all with this CFI transformation, the patch looks ok to me after looking at how cfiFunctionDecls is used. However it would be great to get a review from either @pcc or @eugenis.

llvm/test/LTO/Resolution/X86/export-jumptable-noncanonical.ll

14

I assume this is part of a non-canonical jump table, which is why we don't internalize with this patch. How can I tell that it is part of a non-canonical rather than canonical jump table?

[samitolvanen]

In D103120#2791546, @nickdesaulniers wrote:

Is this related to b/158034873 or another reported bug?

No, this patch just makes it possible to compile stand-alone programs with non-canonical CFI jump tables.

[samitolvanen]

llvm/test/LTO/Resolution/X86/export-jumptable-noncanonical.ll

14

I assume this is part of a non-canonical jump table, which is why we don't internalize with this patch. How can I tell that it is part of a non-canonical rather than canonical jump table?

Correct. The "CFI Canonical Jump Tables" module flag is set to 0 at the bottom of the file to use non-canonical jump tables.

[pcc]

LGTM

[samitolvanen]

Thanks, Peter. I don't have commit access, so Nick, would you mind committing this for me? The Windows build failure looks unrelated.

llvm/lib/LTO/LTO.cpp

if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
       ExportedGUIDs.insert(GUID);
   }
 
   // Any functions referenced by the jump table in the regular LTO object must
   // be exported.
   for (auto &Def : ThinLTO.CombinedIndex.cfiFunctionDefs())
     ExportedGUIDs.insert(
         GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Def)));
+  for (auto &Decl : ThinLTO.CombinedIndex.cfiFunctionDecls())
+    ExportedGUIDs.insert(
+        GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Decl)));
 
   auto isExported = [&](StringRef ModuleIdentifier, ValueInfo VI) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
     return (ExportList != ExportLists.end() && ExportList->second.count(VI)) ||
            ExportedGUIDs.count(VI.getGUID());
   };
 
   // Update local devirtualized targets that were exported by cross-module

llvm/test/LTO/Resolution/X86/export-jumptable-noncanonical.ll

+; Test that we do not internalize functions that appear in the CFI jump table in
+; the full LTO object file; any such functions will be referenced by the jump
+; table.
+
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t %s
+; RUN: llvm-lto2 run -o %t2 -r %t,f1,p -r %t,f2,p -r %t,_start,px %t -save-temps
+; RUN: llvm-dis %t2.1.2.internalize.bc -o - | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK: define void @f1()
+define void @f1() !type !0 {
+  ret void
+}
+
+; CHECK: define internal void @f2()
+define void @f2() !type !1 {
+  ret void
+}
+
+define i1 @_start(i1 %i) {
+  %1 = select i1 %i, void ()* @f1, void ()* @f2
+  %2 = bitcast void ()* %1 to i8*
+  %3 = call i1 @llvm.type.test(i8* %2, metadata !"typeid1")
+  ret i1 %3
+}
+
+declare i1 @llvm.type.test(i8*, metadata)
+
+!llvm.module.flags = !{!2}
+
+!0 = !{i64 0, !"typeid1"}
+!1 = !{i64 0, !"typeid2"}
+!2 = !{i32 4, !"CFI Canonical Jump Tables", i32 0}