MaskRay added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1434
+
+ /// Emit KCFI type identifier constants and remove unused identifiers
+ void FinalizeKCFITypes();
----------------
End with a period.
This function can be lower-case as well. There is no strong precedent to keep
it upper-case.
================
Comment at: clang/lib/Driver/SanitizerArgs.cpp:63
SanitizerKind::Unreachable | SanitizerKind::Return;
-static const SanitizerMask AlwaysRecoverable =
- SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress;
+static const SanitizerMask AlwaysRecoverable = SanitizerKind::KernelAddress |
+ SanitizerKind::KernelHWAddress |
----------------
This is incorrect.
If a violation is found, ud2 is executed. ud2 is not followed by normal control
flow so I don't think recovery from the error is supported.
This seems like `Unrecoverable`
================
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)
----------------
MaskRay wrote:
> 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.
If may be useful to have a `-x c++` test.
Add a not-address-taken external linkage function.
================
Comment at: clang/test/CodeGen/kcfi.c:7
+
+// COM: Must emit __kcfi_typeid symbols for address-taken function declarations
+// CHECK: module asm ".weak __kcfi_typeid_f4"
----------------
Optional: `COM:` is fine. But to make non-RUN non-CHECK lines stand out (from
editor highlighting), a simpler `/// ` suffices as well.
================
Comment at: llvm/docs/LangRef.rst:7222
+functions that can be called indirectly. The type data is emitted before the
+function entry. Indirect calls with the :ref:`kcfi operand bundle<ob_kcfi>`
+will emit a check that compares the type identifier to the metadata.
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119296/new/
https://reviews.llvm.org/D119296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
