Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM modulo some wordsmithing in the documentation. Comment at: docs/ControlFlowIntegrity.rst:31 @@ +30,3 @@ +enabled, and are statically linked into the program. This may preclude

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42914. eugenis marked 10 inline comments as done. Repository: rL LLVM http://reviews.llvm.org/D15367 Files: docs/ControlFlowIntegrity.rst docs/ControlFlowIntegrityDesign.rst docs/UsersManual.rst include/clang/Driver/Options.td

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis added inline comments. Comment at: docs/ControlFlowIntegrityDesign.rst:389 @@ +388,3 @@ + - Calls between different instrumented DSOs are also protected with + performance penalty compared to the monolithic CFI. + - Calls from instrumented DSO to an uninstrumented

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

pcc added a comment. LGTM Comment at: docs/ControlFlowIntegrityDesign.rst:389 @@ +388,3 @@ + - Calls between different instrumented DSOs are also protected, with + performance penalty (in addition to the monolithic CFI overhead). + - Calls from an instrumented DSO to an

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis marked an inline comment as done. eugenis added a comment. Repository: rL LLVM http://reviews.llvm.org/D15367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42918. Repository: rL LLVM http://reviews.llvm.org/D15367 Files: docs/ControlFlowIntegrity.rst docs/ControlFlowIntegrityDesign.rst docs/UsersManual.rst include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis closed this revision. eugenis added a comment. r255694 Repository: rL LLVM http://reviews.llvm.org/D15367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42902. eugenis added a comment. added the new flag to UserManual Repository: rL LLVM http://reviews.llvm.org/D15367 Files: docs/ControlFlowIntegrity.rst docs/ControlFlowIntegrityDesign.rst docs/UsersManual.rst include/clang/Driver/Options.td

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis added a comment. added some docs Comment at: lib/CodeGen/CodeGenModule.cpp:1041 @@ -988,3 +1040,3 @@ void CodeGenModule::addUsedGlobal(llvm::GlobalValue *GV) { assert(!GV->isDeclaration() && "Only globals with definition can force usage.");

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42806. eugenis marked 2 inline comments as done. Repository: rL LLVM http://reviews.llvm.org/D15367 Files: docs/ControlFlowIntegrity.rst docs/ControlFlowIntegrityDesign.rst include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42782. eugenis marked 3 inline comments as done. Repository: rL LLVM http://reviews.llvm.org/D15367 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGClass.cpp

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:3871 @@ +3870,3 @@ +isa(MD) && dyn_cast(MD)->isDistinct(); +if (CGM.getCodeGenOpts().SanitizeCfiCrossDso && !hasLocalScope) { + EmitCfiSlowPathCheck(BitSetTest, MD, CastedCallee);

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

pcc added a comment. Please add documentation. At the very least please document flags in `docs/ControlFlowIntegrity.rst` and `docs/UsersManual.rst`. We should also document the design in `docs/ControlFlowIntegrityDesign.rst`. Comment at: lib/CodeGen/CGExpr.cpp:3868 @@

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis updated this revision to Diff 42607. eugenis marked an inline comment as done. Repository: rL LLVM http://reviews.llvm.org/D15367 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGClass.cpp

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2558 @@ +2557,3 @@ + false)); + llvm::MDString *MDS = dyn_cast(MD); + llvm::Constant *TypeId = pcc wrote: > What happens if `MD` is not an `MDString`? assert + check on the caller side

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

pcc added inline comments. Comment at: lib/CodeGen/CGClass.cpp:2564 @@ -2563,16 +2563,3 @@ - SanitizerMask M; - switch (TCK) { - case CFITCK_VCall: -M = SanitizerKind::CFIVCall; -break; - case CFITCK_NVCall: -M = SanitizerKind::CFINVCall; -break; - case

Re: [PATCH] D15367: Cross-DSO control flow integrity (Clang part)

pcc added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2558 @@ +2557,3 @@ + false)); + llvm::MDString *MDS = dyn_cast(MD); + llvm::Constant *TypeId = What happens if `MD` is not an `MDString`? Comment at:

[PATCH] D15367: Cross-DSO control flow integrity (Clang part)

eugenis created this revision. eugenis added reviewers: kcc, pcc. eugenis added a subscriber: cfe-commits. eugenis set the repository for this revision to rL LLVM. Clang-side cross-DSO CFI. * Enabled with -fsanitize-cfi-cross-dso * uses a runtime library, unlike "plain" CFI * does not yet support