[PATCH] D37656: [cfi] Set function attributes for __cfi_* functions.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:2917
+                                CodeGenFunction &CGF, llvm::Function *F,
+                                bool ForceThumb) {
+  StringRef TargetCPU = CGF.getTarget().getTargetOpts().CPU;
----------------
echristo wrote:
> 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.
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?




================
Comment at: clang/lib/CodeGen/CGExpr.cpp:2961-2963
+  // attributes as SetLLVMFunctionAttributes sets. In particular, __cfi_check
+  // must use the default calling convention for the platform. ABI-changing
+  // flags like -mhard-float should not affect __cfi_check.
----------------
echristo wrote:
> This is odd. Can you explain this a bit more?
__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. 


https://reviews.llvm.org/D37656