Fix __cfi_check not aligned to 4k on relocatable files with no executable code

[kongyi]

Summary

CrossDSOCFIPass is supposed to replace this stub function to a properly aligned function. However the pass is not ran if the file has no executable code, thus producing incorrectly aligned __cfi_check.

Fixes https://github.com/llvm/llvm-project/issues/45638.

[eugenis]

Hmm, this is not 100% correct. CFI by design respects caller's settings for -fsanitize-recover and -fsanitize-trap, communicated through the DiagData argument to __cfi_check. A better default implementation would call __cfi_check_fail instead of trap unconditionally.

I do not see straight away why CrossDSOCFI pass is not run on no-code modules. Do you know the reason? If that's hard to change, we could modify clang codegen to call __cfi_check_fail from the stub __cfi_check. In that case, we also would not need to add __cfi_check_fail to the used globals list.

[kongyi]

In D155736#4519795, @eugenis wrote:

Hmm, this is not 100% correct. CFI by design respects caller's settings for -fsanitize-recover and -fsanitize-trap, communicated through the DiagData argument to __cfi_check. A better default implementation would call __cfi_check_fail instead of trap unconditionally.

I do not see straight away why CrossDSOCFI pass is not run on no-code modules. Do you know the reason? If that's hard to change, we could modify clang codegen to call __cfi_check_fail from the stub __cfi_check. In that case, we also would not need to add __cfi_check_fail to the used globals list.

Seems quite difficult to make CrossDSOCFI run with no code (a lot of interactions with other passes). I implemented your alternative suggestion, ptal.

[eugenis]

clang/lib/CodeGen/CGExpr.cpp

3452

I think the order is actually reversed? Pls compare codegen with a module that has some code (so that the pass runs) but no valid cfi targets.

clang/test/CodeGen/cfi-cross-dso-align.c

7

Please check the generated code in the test.

[eugenis]

LGTM, thank you.

[ro]

It seems this patch broke the Solaris/amd64 buildbot:

FAIL: Clang::cfi-check-fail.c

[thakis]

Looks like this breaks check-clang: http://45.33.8.238/linux/114361/step_7.txt

Please take a look and revert for now if it takes a while to fix.

clang/lib/CodeGen/CGExpr.cpp

 // 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);
+  F->setAlignment(llvm::Align(4096));
   CGM.setDSOLocal(F);
   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);
+  // CrossDSOCFI pass is not executed if there is no executable code.
+  SmallVector<llvm::Value*> Args{F->getArg(2), F->getArg(1)};
+  llvm::CallInst::Create(M->getFunction("__cfi_check_fail"), Args, "", 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
for (auto CheckKindMaskPair : CheckKinds) {
     if (CGM.getLangOpts().Sanitize.has(Mask))
       EmitCheck(std::make_pair(Cond, Mask), SanitizerHandler::CFICheckFail, {},
                 {Data, Addr, ValidVtable});
     else
       EmitTrapCheck(Cond, SanitizerHandler::CFICheckFail);
   }
 
   FinishFunction();
-  // The only reference to this function will be created during LTO link.
-  // Make sure it survives until then.
-  CGM.addUsedGlobal(F);
 }
 
 void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
   if (SanOpts.has(SanitizerKind::Unreachable)) {
     SanitizerScope SanScope(this);
     EmitCheck(std::make_pair(static_cast<llvm::Value *>(Builder.getFalse()),
                              SanitizerKind::Unreachable),
               SanitizerHandler::BuiltinUnreachable,

clang/test/CodeGen/cfi-cross-dso-align.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux -O0 -fsanitize-cfi-cross-dso \
+// RUN:     -emit-llvm -o - %s | FileCheck %s
+
+int a;
+
+// CHECK: define weak void @__cfi_check(i64 %[[TYPE:.*]], ptr %[[ADDR:.*]], ptr %[[DATA:.*]]) align 4096
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @__cfi_check_fail(ptr %[[DATA]], ptr %[[ADDR]])