[PATCH] D31796: [cfi] Emit __cfi_check stub in the frontend

[Evgeniy Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: lib/CodeGen/CGExpr.cpp:2788
+  auto &Ctx = CGM.getModule().getContext();
+  llvm::Function *F = llvm::Function::Create(
+      llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy}, false),
----------------
pcc wrote:
> 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.
Yes, I've not uploaded the llvm patch yet, but it's a one liner:
  F->deleteBody();
after locating or creating __cfi_check.


================
Comment at: lib/CodeGen/CGExpr.cpp:2790
+      llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy}, false),
+      llvm::GlobalValue::WeakAnyLinkage, "__cfi_check", &CGM.getModule());
+  llvm::BasicBlock *BB = llvm::BasicBlock::Create(Ctx, "entry", F);
----------------
pcc wrote:
> 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`.
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D31796