[cfi] Set function attributes for __cfi_* functions.

[eugenis]

Summary

Set target_cpu and target_features attributes on cfi_check_fail and
cfi_check. Make cfi_check use Thumb encoding on ARM target.

[eugenis]

ping

[echristo]

clang/lib/CodeGen/CGExpr.cpp

2917

I don't think we should be force setting thumb anywhere, it should be handled on a function by function basis the way we do all of the target attribute stuff.

2926–2935

All of this should be handled in lib/Basic/Targets/ARM.cpp ARMTargetInfo::initFeatureMap ?

2934–2936

This is odd. Can you explain this a bit more?

[eugenis]

clang/lib/CodeGen/CGExpr.cpp

2917

We want this function to be Thumb even if the rest of the module is ARM. It lets us use 2x less memory in the implementation of __cfi_slowpath here:
https://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#cfi-slowpath

This function does not have source, so it can not have target attributes, too.

Are you suggesting faking a piece of AST (FunctionDecl with attached attributes) and then using the regular attribute setting code?

2934–2936

__cfi_check is called from compiler-rt or libdl.so, so it can not have any fancy calling convention. It must use what's standard on the platform.

OK, __cfi_check has no floating-point parameters, so -mhard-float would not be a problem.

[echristo]

clang/lib/CodeGen/CGExpr.cpp

2917

Aha. Didn't remember that part of the design.

And yeah, that sounds good. And it doesn't really matter if the function has any source code as to whether or not it has attributes. There's no inheritance structure going on here so every function should have them.

2934–2936

Yeah, I think this falls in on the earlier comment. Should probably add the defaults to a bit of AST or something?

[eugenis]

Switched to SetLLVMFunctionAttributes

clang/lib/CodeGen/CGExpr.cpp

void CodeGenFunction::EmitCfiSlowPathCheck(
 
   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);
+
+  // Set default function attributes, but force thumb encoding if applicable.
+  const CGFunctionInfo &FI =
+      CGM.getTypes().arrangeBuiltinFunctionDeclaration(getContext().VoidTy, {});
+  FunctionDecl *FD =
+      FunctionDecl::Create(getContext(), getContext().getTranslationUnitDecl(),
+                           {}, {}, {}, getContext().VoidTy, nullptr, SC_Extern);
+  const auto &Triple = getTarget().getTriple();
+  if (Triple.isARM() || Triple.isThumb()) {
+    FD->addAttr(TargetAttr::CreateImplicit(getContext(), "thumb"));
+  }
+  CGM.SetLLVMFunctionAttributes(FD, FI, F);
 }
 
 // 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
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();
+
+  CGM.SetLLVMFunctionAttributes(nullptr, FI, F);
+
   // 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");
 

clang/test/CodeGen/cfi-check-thumb.c

+// Test that __cfi_check and __cfi_check_fail have common attributes and calling convention.
+// Also __cfi_check is always using Thumb encoding.
+// RUN: %clang_cc1 -triple armv7-linux-android -O0 -fsanitize-cfi-cross-dso \
+// RUN:     -fsanitize=cfi-vcall \
+// RUN:     -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,CHECK-ARM %s
+//
+// RUN: %clang_cc1 -triple thumbv7-linux-android -O0 -fsanitize-cfi-cross-dso \
+// RUN:     -fsanitize=cfi-vcall \
+// RUN:     -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,CHECK-THUMB %s
+//
+// REQUIRES: arm-registered-target
+
+// CHECK-ARM: define weak_odr hidden void @__cfi_check_fail(i8*, i8*) #[[ARM:.*]] {
+// CHECK-THUMB: define weak_odr hidden void @__cfi_check_fail(i8*, i8*) #[[THUMB:.*]] {
+// CHECK: define weak void @__cfi_check(i64, i8*, i8*) #[[THUMB:.*]] {
+
+// CHECK-ARM: attributes #[[ARM]] = {{.*}}"target-features"="{{.*}}-thumb-mode
+// CHECK: attributes #[[THUMB]] = {{.*}}"target-features"="{{.*}}+thumb-mode

llvm/lib/Transforms/IPO/CrossDSOCFI.cpp

Constant *C = M.getOrInsertFunction(
       "__cfi_check", Type::getVoidTy(Ctx), Type::getInt64Ty(Ctx),
       Type::getInt8PtrTy(Ctx), Type::getInt8PtrTy(Ctx));
   Function *F = dyn_cast<Function>(C);
   // Take over the existing function. The frontend emits a weak stub so that the
   // linker knows about the symbol; this pass replaces the function body.
   F->deleteBody();
   F->setAlignment(4096);
 
-  Triple T(M.getTargetTriple());
-  if (T.isARM() || T.isThumb())
-    F->addFnAttr("target-features", "+thumb-mode");
-
   auto args = F->arg_begin();
   Value &CallSiteTypeId = *(args++);
   CallSiteTypeId.setName("CallSiteTypeId");
   Value &Addr = *(args++);
   Addr.setName("Addr");
   Value &CFICheckFailData = *(args++);
   CFICheckFailData.setName("CFICheckFailData");
   assert(args == F->arg_end());

llvm/test/Transforms/CrossDSOCFI/thumb.ll

-; RUN: opt -mtriple=armv7-linux-android -S -cross-dso-cfi < %s | FileCheck --check-prefix=THUMB %s
-; RUN: opt -mtriple=thumbv7-linux-android -S -cross-dso-cfi < %s | FileCheck --check-prefix=THUMB %s
-; RUN: opt -mtriple=i386-linux -S -cross-dso-cfi < %s | FileCheck --check-prefix=NOTHUMB %s
-; RUN: opt -mtriple=x86_64-linux -S -cross-dso-cfi < %s | FileCheck --check-prefix=NOTHUMB %s
-
-target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-
-define signext i8 @f() !type !0 !type !1 {
-entry:
-  ret i8 1
-}
-
-!llvm.module.flags = !{!2}
-
-!0 = !{i64 0, !"_ZTSFcvE"}
-!1 = !{i64 0, i64 111}
-!2 = !{i32 4, !"Cross-DSO CFI", i32 1}
-
-; THUMB: define void @__cfi_check({{.*}} #[[A:.*]] align 4096
-; THUMB: attributes #[[A]] = { {{.*}}"target-features"="+thumb-mode"
-
-; NOTHUMB: define void @__cfi_check({{.*}} align 4096