[PATCH] D119296: KCFI sanitizer

[Fangrui Song via Phabricator via llvm-commits]

MaskRay requested changes to this revision.
MaskRay added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2280
+void CodeGenModule::setKCFIType(const FunctionDecl *FD, llvm::Function *F) {
+
+  if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
----------------
delete blank line


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2286
+  llvm::MDBuilder MDB(Ctx);
+
+  F->setMetadata(llvm::LLVMContext::MD_kcfi_type,
----------------
delete blank line


================
Comment at: clang/test/CodeGen/kcfi.c:2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck --check-prefixes=CHECK,O0 %s
+// RUN: %clang_cc1 -O2 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck --check-prefixes=CHECK,O2 %s
+#if !__has_feature(kcfi)
----------------
If `-O2` has different behavior, having the test in clang/test/CodeGen is likely a layering violation.
Optimization tests should be placed in llvm/test/, in the relevant pass. Folks working on llvm optimizations generally don't want to test every frontend.


================
Comment at: clang/test/Driver/fsanitize.c:652
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize=cfi -flto -fvisibility=hidden %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-NOCFI
+// CHECK-KCFI-NOCFI: error: invalid argument '-fsanitize=kcfi' not allowed with '-fsanitize=cfi'
----------------
Use modern spelling `--target=`. `-target ` is deprecated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119296/new/

https://reviews.llvm.org/D119296