[cfi] Emit __cfi_check stub in the frontend

[eugenis]

Summary

Previously cfi_check was created in LTO optimization pipeline, which
means LLD has no way of knowing about the existence of this symbol
without rescanning the LTO output object. As a result, LLD fails to
export cfi_check, even when given --export-dynamic-symbol flag.

[pcc]

lib/CodeGen/CGExpr.cpp

2786

Please add a comment explaining why we are creating this function.

2788

What will happen in the CrossDSOCFI pass if it is given a module with this function definition? It looks like it will append its basic blocks to the end of the definition. Because the entry block you are creating here returns immediately, all of the blocks it adds will be unreachable.

Probably the easiest thing to do is to change CrossDSOCFI to replace any __cfi_check definition it sees with its own definition.

2790

Although this linkage may prevent inlining for now, we may in the future change the LTO implementation to promote weak functions to strong if the linker knows that a particular definition of a symbol is the final definition for the symbol. That will mean that this dummy definition may be inlined into callers.

This is a largely theoretical problem because we never create a direct call to the __cfi_check function. But if we wanted to fix this I can imagine creating an intrinsic that the CrossDSOCFI pass expands to the definition of __cfi_check.

2885

remove this line

[eugenis]

lib/CodeGen/CGExpr.cpp

2788

Yes, I've not uploaded the llvm patch yet, but it's a one liner:

F->deleteBody();

after locating or creating __cfi_check.

2790

Do you mean "expands to a call to cfi_check"?
We do need to have a definition of cfi_check before CrossDSOCFI for the linker.

[pcc]

lib/CodeGen/CGExpr.cpp

2788

I suppose that would work for now, but what the CrossDSOCFI pass is doing seems a little hackish because it will crash if the type of the existing definition (or declaration) does not exactly match the expected type.

2790

I mean basically that Clang would create a definition like this:

define void @__cfi_check(i64 %0, i8* %1, i8* %2) {
  call void @llvm.cfi_check(i64 %0, i8* %1, i8* %2)
  ret void
}

and CrossDSOCFI would find all calls to @llvm.cfi_check and replace them with the code that the pass currently generates as the body of __cfi_check.

[eugenis]

lib/CodeGen/CGExpr.cpp

2790

Ah, that definitely makes sense. Let's mention this in a comment for now.

[pcc]

lib/CodeGen/CGExpr.cpp

2790

Works for me.

[pcc]

LGTM

lib/CodeGen/CGExpr.cpp

2798

I would terminate this definition in an llvm.trap so that the program will crash if this definition were to survive LTO for some reason.

[eugenis]

r299806
Thanks for the review!

lib/CodeGen/CGExpr.cpp

if (WithDiag) {
     CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);
 }
 
+// Emit a stub for __cfi_check function so that the linker knows about this
+// symbol in LTO mode.
+void CodeGenFunction::EmitCfiCheckStub() {
+  llvm::Module *M = &CGM.getModule();
+  auto &Ctx = M->getContext();
+  llvm::Function *F = llvm::Function::Create(
+      llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy}, false),
+      llvm::GlobalValue::WeakAnyLinkage, "__cfi_check", M);
+  llvm::BasicBlock *BB = llvm::BasicBlock::Create(Ctx, "entry", F);
+  // FIXME: consider emitting an intrinsic call like
+  // call void @llvm.cfi_check(i64 %0, i8* %1, i8* %2)
+  // which can be lowered in CrossDSOCFI pass to the actual contents of
+  // __cfi_check. This would allow inlining of __cfi_check calls.
+  llvm::CallInst::Create(
+      llvm::Intrinsic::getDeclaration(M, llvm::Intrinsic::trap), "", BB);
+  llvm::ReturnInst::Create(Ctx, nullptr, BB);
+}
+
 // This function is basically a switch over the CFI failure kind, which is
 // extracted from CFICheckFailData (1st function argument). Each case is either
 // llvm.trap or a call to one of the two runtime handlers, based on
 // -fsanitize-trap and -fsanitize-recover settings.  Default case (invalid
 // failure kind) traps, but this should really never happen.  CFICheckFailData
 // can be nullptr if the calling module has -fsanitize-trap behavior for this
 // check kind; in this case __cfi_check_fail traps as well.
 void CodeGenFunction::EmitCfiCheckFail() {
for (auto CheckKindMaskPair : CheckKinds) {
     if (CGM.getLangOpts().Sanitize.has(Mask))
       EmitCheck(std::make_pair(Cond, Mask), SanitizerHandler::CFICheckFail, {},
                 {Data, Addr, ValidVtable});
     else
       EmitTrapCheck(Cond);
   }
 
   FinishFunction();
   // The only reference to this function will be created during LTO link.
   // Make sure it survives until then.
   CGM.addUsedGlobal(F);
 }
 
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per

lib/CodeGen/CodeGenFunction.h

public:
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
   /// \brief Emit a call to trap or debugtrap and attach function attribute
   /// "trap-func-name" if specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+  /// \brief Emit a stub for the cross-DSO CFI check function.
+  void EmitCfiCheckStub();
+
   /// \brief Emit a cross-DSO CFI failure handling function.
   void EmitCfiCheckFail();
 
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
                            AbstractCallee AC, unsigned ParmNum);
 

lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::Release() {
   }
   EmitCtorList(GlobalCtors, "llvm.global_ctors");
   EmitCtorList(GlobalDtors, "llvm.global_dtors");
   EmitGlobalAnnotations();
   EmitStaticExternCAliases();
   EmitDeferredUnusedCoverageMappings();
   if (CoverageMapping)
     CoverageMapping->emit();
-  if (CodeGenOpts.SanitizeCfiCrossDso)
+  if (CodeGenOpts.SanitizeCfiCrossDso) {
     CodeGenFunction(*this).EmitCfiCheckFail();
+    CodeGenFunction(*this).EmitCfiCheckStub();
+  }
   emitAtAvailableLinkGuard();
   emitLLVMUsed();
   if (SanStats)
     SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
       (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
     EmitModuleLinkOptions();

test/CodeGen/cfi-check-fail.c

 // CHECK:   br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !nosanitize
 
 // CHECK: [[HANDLE4]]:
 // CHECK-NEXT:   call void @llvm.trap()
 // CHECK-NEXT:   unreachable
 
 // CHECK: [[CONT5]]:
 // CHECK:   ret void
+
+// CHECK: define weak void @__cfi_check(i64, i8*, i8*)
+// CHECK-NOT: }
+// CHECK: call void @llvm.trap()
+// CHECK-NEXT: ret void