From 7f8878bb02eaaae89cbf0dabba15888f8c4add98 Mon Sep 17 00:00:00 2001 From: Nikolai Kholiavin Date: Sat, 30 Mar 2024 01:07:09 +0000 Subject: [PATCH 1/2] [IR] Check for implicit 0 from Min module flag behavior in requirements This fixes combination of Min IR module flag behavior and a module flag requirement to work in the case where the flag is missing in one of the modules. Previously this caused the requirement check to erroneously succeed in IRLinker, but verifier would catch the problem and crash. Issue: https://gitee.com/openharmony/third_party_llvm-project/issues/I9LG51 Signed-off-by: Nikolai Kholiavin --- llvm/lib/Linker/IRMover.cpp | 4 +++- .../Linker/module-flags-min-0-required.ll | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 llvm/test/Linker/module-flags-min-0-required.ll diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp index e31faf6422ed..ab960e021d4d 100644 --- a/llvm/lib/Linker/IRMover.cpp +++ b/llvm/lib/Linker/IRMover.cpp @@ -1487,7 +1487,9 @@ Error IRLinker::linkModuleFlagsMetadata() { Metadata *FlagOps[] = { Op->getOperand(0), ID, ConstantAsMetadata::get(ConstantInt::get(V->getType(), 0))}; - DstModFlags->setOperand(Idx, MDNode::get(DstM.getContext(), FlagOps)); + MDNode *Flag = MDNode::get(DstM.getContext(), FlagOps); + DstModFlags->setOperand(Idx, Flag); + Flags[ID].first = Flag; } } diff --git a/llvm/test/Linker/module-flags-min-0-required.ll b/llvm/test/Linker/module-flags-min-0-required.ll new file mode 100644 index 000000000000..b8c9406be2a7 --- /dev/null +++ b/llvm/test/Linker/module-flags-min-0-required.ll @@ -0,0 +1,20 @@ +; RUN: rm -rf %t && split-file %s %t && cd %t +; RUN: not llvm-link a.ll b.ll -S -o - 2>&1 | FileCheck %s + +; CHECK: error: linking module flags 'required_in_b': does not have the required value + +;--- a.ll +!0 = !{ i32 8, !"foo", i16 2 } +!1 = !{ i32 8, !"bar", i64 4 } +!2 = !{ i32 8, !"only_in_a", i32 4 } + +!llvm.module.flags = !{ !0, !1, !2 } + +;--- b.ll +!0 = !{ i32 8, !"foo", i16 3 } +!1 = !{ i32 8, !"bar", i64 3 } +!2 = !{ i32 8, !"only_in_b", i32 3 } +!3 = !{ i32 8, !"required_in_b", i32 3 } +!4 = !{ i32 3, !"require", !{!"required_in_b", i32 3} } + +!llvm.module.flags = !{ !0, !1, !2, !3, !4 } -- Gitee From c13eb3b50980712bcb374a8fe48bfaaf9c2c18e8 Mon Sep 17 00:00:00 2001 From: Nikolai Kholiavin Date: Sat, 30 Mar 2024 01:11:58 +0000 Subject: [PATCH 2/2] [CFI][clang][Driver][CodeGen] Introduce option to catch mixed CFI code Introduce an option to check that all of the IR modules participating in the IR link have cross-DSO CFI enabled. Option should be specified to the compilation stage, and will fail the link stage if: - all of the CFI-enabled modules were compiled with -fsanitize-cfi-cross-dso-req clang option - at least one of the IR modules was generated with -fsanitize-cfi-cross-dso -fsanitize-cfi-cross-dso-req clang options - at least one of the IR modules was generated without cross-DSO CFI. Note that this doesn't catch CFI/cross-DSO CFI or non-LTO/any CFI code mixes. Issue: https://gitee.com/openharmony/third_party_llvm-project/issues/I9LG51 Signed-off-by: Nikolai Kholiavin --- clang/docs/ClangCommandLineReference.rst | 4 ++ clang/include/clang/Basic/CodeGenOptions.def | 1 + clang/include/clang/Driver/Options.td | 5 ++ clang/include/clang/Driver/SanitizerArgs.h | 1 + clang/lib/CodeGen/CodeGenModule.cpp | 14 +++++- clang/lib/Driver/SanitizerArgs.cpp | 8 ++++ clang/test/CodeGen/cfi-cross-dso-req.c | 13 ++++++ compiler-rt/test/cfi/cross-dso/required.cpp | 48 ++++++++++++++++++++ 8 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 clang/test/CodeGen/cfi-cross-dso-req.c create mode 100644 compiler-rt/test/cfi/cross-dso/required.cpp diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst index 41fb8f7259eb..d82f716fbafe 100644 --- a/clang/docs/ClangCommandLineReference.rst +++ b/clang/docs/ClangCommandLineReference.rst @@ -999,6 +999,10 @@ Make the jump table addresses canonical in the symbol table Enable control flow integrity (CFI) checks for cross-DSO calls. +.. option:: -fsanitize-cfi-cross-dso-req, -fno-sanitize-cfi-cross-dso-req + +Enable the requirement for cross-DSO control flow integrity (CFI) checks for all LTO modules. + .. option:: -fsanitize-cfi-icall-generalize-pointers Generalize pointers in CFI indirect call type signature checks diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 5d4aec842db3..eb136624bb34 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -253,6 +253,7 @@ CODEGENOPT(SanitizeMemoryParamRetval, 1, 0) ///< Enable detection of uninitializ CODEGENOPT(SanitizeMemoryUseAfterDtor, 1, 0) ///< Enable use-after-delete detection ///< in MemorySanitizer CODEGENOPT(SanitizeCfiCrossDso, 1, 0) ///< Enable cross-dso support in CFI. +CODEGENOPT(SanitizeCfiCrossDsoReq, 1, 0) ///< Require cross-dso CFI support in all modules. CODEGENOPT(SanitizeMinimalRuntime, 1, 0) ///< Use "_minimal" sanitizer runtime for ///< diagnostics. CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 97433f169d14..b6a1636aa84a 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1822,6 +1822,11 @@ defm sanitize_cfi_cross_dso : BoolOption<"f", "sanitize-cfi-cross-dso", PosFlag, NegFlag, BothFlags<[], " control flow integrity (CFI) checks for cross-DSO calls.">>, Group; +defm sanitize_cfi_cross_dso_req : BoolOption<"f", "sanitize-cfi-cross-dso-req", + CodeGenOpts<"SanitizeCfiCrossDsoReq">, DefaultFalse, + PosFlag, NegFlag, + BothFlags<[], " the requirement for cross-DSO control flow integrity (CFI) checks for all LTO modules.">>, + Group; def fsanitize_cfi_icall_generalize_pointers : Flag<["-"], "fsanitize-cfi-icall-generalize-pointers">, Group, HelpText<"Generalize pointers in CFI indirect call type signature checks">, diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index 1b29b1151224..18d0a9892e8e 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -35,6 +35,7 @@ class SanitizerArgs { bool MsanUseAfterDtor = true; bool MsanParamRetval = false; bool CfiCrossDso = false; + bool CfiCrossDsoReq = false; bool CfiICallGeneralizePointers = false; bool CfiCanonicalJumpTables = false; int AsanFieldPadding = 0; diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index a6144180f9c9..e9635356e26f 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -741,7 +741,19 @@ void CodeGenModule::Release() { if (CodeGenOpts.SanitizeCfiCrossDso) { // Indicate that we want cross-DSO control flow integrity checks. - getModule().addModuleFlag(llvm::Module::Override, "Cross-DSO CFI", 1); + if (CodeGenOpts.SanitizeCfiCrossDsoReq) { + getModule().addModuleFlag(llvm::Module::Min, "Cross-DSO CFI", 1); + + llvm::Metadata *Ops[2] = { + llvm::MDString::get(VMContext, "Cross-DSO CFI"), + llvm::ConstantAsMetadata::get(llvm::ConstantInt::get( + llvm::Type::getInt32Ty(VMContext), 1))}; + getModule().addModuleFlag(llvm::Module::Require, + "Cross-DSO CFI Requirement", + llvm::MDNode::get(VMContext, Ops)); + } else { + getModule().addModuleFlag(llvm::Module::Override, "Cross-DSO CFI", 1); + } } if (CodeGenOpts.WholeProgramVTables) { diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 20c93c03a4c0..d52afc636445 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -699,6 +699,11 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, // Without PIE, external function address may resolve to a PLT record, which // can not be verified by the target module. NeedPIE |= CfiCrossDso; + + CfiCrossDsoReq = + Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso_req, + options::OPT_fno_sanitize_cfi_cross_dso_req, false); + CfiICallGeneralizePointers = Args.hasArg(options::OPT_fsanitize_cfi_icall_generalize_pointers); @@ -1161,6 +1166,9 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, if (CfiCrossDso) CmdArgs.push_back("-fsanitize-cfi-cross-dso"); + if (CfiCrossDsoReq) + CmdArgs.push_back("-fsanitize-cfi-cross-dso-req"); + if (CfiICallGeneralizePointers) CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers"); diff --git a/clang/test/CodeGen/cfi-cross-dso-req.c b/clang/test/CodeGen/cfi-cross-dso-req.c new file mode 100644 index 000000000000..42be04605765 --- /dev/null +++ b/clang/test/CodeGen/cfi-cross-dso-req.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -O0 -fsanitize-cfi-cross-dso \ +// RUN: -fsanitize=cfi-icall,cfi-nvcall,cfi-vcall,cfi-unrelated-cast,cfi-derived-cast \ +// RUN: -fsanitize-trap=cfi-icall,cfi-nvcall -fsanitize-recover=cfi-vcall,cfi-unrelated-cast \ +// RUN: -fsanitize-cfi-cross-dso-req \ +// RUN: -emit-llvm -o - %s | FileCheck %s + +void caller(void (*f)(void)) { + f(); +} + +// CHECK: ![[#]] = !{i32 8, !"Cross-DSO CFI", i32 1} +// CHECK: ![[#]] = !{i32 3, !"Cross-DSO CFI Requirement", ![[#FLAG:]]} +// CHECK: ![[#FLAG]] = !{!"Cross-DSO CFI", i32 1} diff --git a/compiler-rt/test/cfi/cross-dso/required.cpp b/compiler-rt/test/cfi/cross-dso/required.cpp new file mode 100644 index 000000000000..90bd4c11bcbe --- /dev/null +++ b/compiler-rt/test/cfi/cross-dso/required.cpp @@ -0,0 +1,48 @@ +// RUN: %clangxx_cfi_dso -g -DSRC_A -fPIC -c %s -o %t.a.o -fsplit-lto-unit -fsanitize-cfi-cross-dso-req +// RUN: %clangxx_cfi_dso -g -DSRC_B -fPIC -c %s -o %t.b.nocfi.o -fno-sanitize=cfi -fsplit-lto-unit +// RUN: not %clangxx_cfi_dso %t.a.o %t.b.nocfi.o -shared -o %dynamiclib %ld_flags_rpath_so 2>&1 | FileCheck --check-prefix=FAIL-REQ %s + +// RUN: %clangxx_cfi_dso -g -DSRC_B -fPIC -c %s -o %t.b.o -fsplit-lto-unit -fsanitize-cfi-cross-dso-req +// RUN: %clangxx_cfi_dso %t.a.o %t.b.o -shared -o %dynamiclib %ld_flags_rpath_so +// RUN: %clangxx_cfi_dso %s -o %t %ld_flags_rpath_exe +// RUN: %t 2>&1 | FileCheck --check-prefix=CFI %s + +// FAIL-REQ: linking module flags 'Cross-DSO CFI': does not have the required value + +#include + +struct A { + virtual void f(); +}; + +A *create_B(); + +#ifdef SRC_B + +struct B : A { + void f() override; +}; +void B::f() { + puts("B"); +} + +A *create_B() { + return new B(); +} + +#elif defined SRC_A + +void A::f() { + puts("A"); +} + +#else + +int main() { + A *ptr = create_B(); + ptr->f(); + // CFI: B + return 0; +} + +#endif -- Gitee