[PATCH] D135411: Add generic KCFI operand bundle lowering

This revision was automatically updated to reflect the committed changes. Closed by commit rGcacd3e73d7f8: Add generic KCFI operand bundle lowering (authored by samitolvanen). Changed prior to commit: https://reviews.llvm.org/D135411?vs=476615=477315#toc Repository: rG LLVM Github Monorepo

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:975 // done on PreLink stage. -if (!IsThinLTOPostLink) +if (!IsThinLTOPostLink) { addSanitizers(TargetTriple, CodeGenOpts, LangOpts, PB);

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 476615. samitolvanen added a comment. Fixed clang-format warnings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/ https://reviews.llvm.org/D135411 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 476491. samitolvanen added a comment. Dropped the unneeded constructor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/ https://reviews.llvm.org/D135411 Files:

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/include/llvm/Transforms/Instrumentation/KCFI.h:24 +public: + KCFIPass() {} + static bool isRequired() { return true; } remove default constructor? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 476260. samitolvanen added a comment. Dropped the CodeGen dependency. Note that Clang already only adds KCFIPass for targets that don't support back-end lowering, so the check in the pass itself was a bit redundant. The existing CodeGen/kcfi.c test

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:16 +#include "llvm/ADT/Statistic.h" +#include "llvm/CodeGen/TargetLowering.h" +#include "llvm/CodeGen/TargetSubtargetInfo.h" Sorry, I just noticed this include.

[PATCH] D135411: Add generic KCFI operand bundle lowering

This revision was automatically updated to reflect the committed changes. Closed by commit rGeb2a57ebc7aa: Add generic KCFI operand bundle lowering (authored by samitolvanen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 473348. samitolvanen added a comment. Addressed more feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/ https://reviews.llvm.org/D135411 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:42 +namespace { +class KCFIDiagnosticInfo : public DiagnosticInfo { + const Twine If you look at existing DiagnosticInfo subclasses, they

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 473340. samitolvanen marked 4 inline comments as done. samitolvanen added a comment. Added `KCFIDiagnosticInfo` and switched from `report_fatal_error` to `LLVMContext::diagnose`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:61 + if (F.hasFnAttribute("patchable-function-prefix")) +report_fatal_error("-fpatchable-function-entry=N,M, where M>0 is not " + "compatible with -fsanitize=kcfi

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:61 + if (F.hasFnAttribute("patchable-function-prefix")) +report_fatal_error("-fpatchable-function-entry=N,M, where M>0 is not " + "compatible with

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 473321. samitolvanen marked 6 inline comments as done. samitolvanen added a comment. Addressed Fangrui's feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:47 + // Find call instructions with KCFI operand bundles. + SmallVector KCFICalls; + for (Instruction : instructions(F)) { You can omit `, 8` to use the default (also 8).

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:1082 SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero | - SanitizerKind::UnsignedIntegerOverflow | + SanitizerKind::KCFI | SanitizerKind::UnsignedIntegerOverflow |

[PATCH] D135411: Add generic KCFI operand bundle lowering

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:1082 SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero | - SanitizerKind::UnsignedIntegerOverflow | + SanitizerKind::KCFI | SanitizerKind::UnsignedIntegerOverflow |

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 466635. samitolvanen added a comment. Switched to `instructions(F)`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/ https://reviews.llvm.org/D135411 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D135411: Add generic KCFI operand bundle lowering

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:48-49 + SmallVector KCFICalls; + for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) { +if (auto *CI = dyn_cast(&*I)) + if

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:48-49 + SmallVector KCFICalls; + for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) { +if (auto *CI = dyn_cast(&*I)) + if

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen updated this revision to Diff 466612. samitolvanen marked 2 inline comments as done. samitolvanen added a comment. Addressed feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135411/new/ https://reviews.llvm.org/D135411 Files:

[PATCH] D135411: Add generic KCFI operand bundle lowering

kees added a comment. In D135411#3841841 , @samitolvanen wrote: > That being said, I did compile a 64-bit MIPS kernel using this pass, but I > didn't try booting it. I would expect to run into quite a few issues > initially. I've done a test build

[PATCH] D135411: Add generic KCFI operand bundle lowering

nickdesaulniers accepted this revision. nickdesaulniers added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/Transforms/Instrumentation/KCFI.cpp:48-49 + SmallVector KCFICalls; + for (inst_iterator I = inst_begin(F), E = inst_end(F); I !=

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen added a comment. In D135411#3841693 , @kees wrote: > What's the best way to test this in the kernel? I assume add > `ARCH_SUPPORTS_CFI_CLANG` to an arch, and see what blows up? :) Yes, I think that's the best way. > Have you tried this on

[PATCH] D135411: Add generic KCFI operand bundle lowering

kees added a comment. What's the best way to test this in the kernel? I assume add `ARCH_SUPPORTS_CFI_CLANG` to an arch, and see what blows up? :) Have you tried this on any other arch yet? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135411: Add generic KCFI operand bundle lowering

samitolvanen created this revision. Herald added subscribers: Enna1, ormris, pengfei, hiraditya, kristof.beyls. Herald added a project: All. samitolvanen requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. The KCFI