Skip to content

[Driver][CFI] Allow CFI with minimal runtime #137103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Apr 24, 2025

It was introduced with original "minimal runtime"
patch without explanation:
https://reviews.llvm.org/D36810#:~:text=if%20(-,NonTrappingCfi,-)

Note, the same commit contains cfi_check_fail handler,
which can not be reached with -fsanitize-trap=cfi.

This patch makes CFI min runtime behavior consistent with UBSAN:
-fsanitize-trap= ignores -fsanitize-minimal-runtime,
-fno-sanitize-trap= with -fsanitize-minimal-runtime compiles and link
minimal runtime.

Created using spr 1.3.4
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 24, 2025

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-clangir
@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-directx
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

It was introduced with original "minimal runtime"
patch without explanation:
https://reviews.llvm.org/D36810#:~:text=if%20(-,NonTrappingCfi,-)

The same commit contains cfi_check_fail handler,
which can not be reachable with -fsanitize-trap=cfi.


Full diff: https://github.com/llvm/llvm-project/pull/137103.diff

3 Files Affected:

  • (modified) clang/lib/Driver/SanitizerArgs.cpp (-6)
  • (modified) clang/test/Driver/fsanitize.c (+5-6)
  • (modified) clang/test/Driver/sanitizer-ld.c (+3-6)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 94a036172d0b6..2edb200688418 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -849,12 +849,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       D.Diag(clang::diag::err_drv_argument_not_allowed_with)
           << "-fsanitize-minimal-runtime"
           << lastArgumentForMask(D, Args, IncompatibleMask);
-
-    SanitizerMask NonTrappingCfi = Kinds & SanitizerKind::CFI & ~TrappingKinds;
-    if (NonTrappingCfi && DiagnoseErrors)
-      D.Diag(clang::diag::err_drv_argument_only_allowed_with)
-          << "-fsanitize-minimal-runtime"
-          << "-fsanitize-trap=cfi";
   }
 
   for (const auto *Arg : Args.filtered(
diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index af00492314ff7..76de0a94ec591 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -961,19 +961,18 @@
 // CHECK-CFI-ABORT-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-ABORT-MINIMAL: "-fsanitize-minimal-runtime"
 
-// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=cfi -flto -fvisibility=hidden -fsanitize-minimal-runtime -fno-sanitize-trap=cfi -fsanitize-recover=cfi -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-RECOVER-MINIMAL --
-// CHECK-CFI-NOTRAP-RECOVER-MINIMAL: '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=cfi -flto -fvisibility=hidden -fsanitize-minimal-runtime -fno-sanitize-trap=cfi -fsanitize-recover=cfi -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-RECOVER-MINIMAL --
 // CHECK-CFI-NOTRAP-RECOVER-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOTRAP-RECOVER-MINIMAL: "-fsanitize-recover=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOTRAP-RECOVER-MINIMAL: "-fsanitize-minimal-runtime"
 
-// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=cfi -flto -fvisibility=hidden -fsanitize-minimal-runtime -fno-sanitize-trap=cfi -fno-sanitize-recover=cfi -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-ABORT-MINIMAL
-// CHECK-CFI-NOTRAP-ABORT-MINIMAL: '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=cfi -flto -fvisibility=hidden -fsanitize-minimal-runtime -fno-sanitize-trap=cfi -fno-sanitize-recover=cfi -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-ABORT-MINIMAL
 // CHECK-CFI-NOTRAP-ABORT-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOTRAP-ABORT-MINIMAL: "-fsanitize-minimal-runtime"
 
-// RUN: not %clang --target=x86_64-linux-gnu -fsanitize=cfi -fno-sanitize-trap=cfi-icall -flto -fvisibility=hidden -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-MINIMAL
-// CHECK-CFI-NOTRAP-MINIMAL: error: invalid argument '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
+// RUN: %clang --target=x86_64-linux-gnu -fsanitize=cfi -fno-sanitize-trap=cfi-icall -flto -fvisibility=hidden -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-MINIMAL
+// CHECK-CFI-NOTRAP-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+// CHECK-CFI-NOTRAP-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=cfi -fno-sanitize-trap=cfi-icall -fno-sanitize=cfi-icall -flto -fvisibility=hidden -fsanitize-minimal-runtime -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOICALL-MINIMAL
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
diff --git a/clang/test/Driver/sanitizer-ld.c b/clang/test/Driver/sanitizer-ld.c
index a5a4fc2320bd2..befd322d027c9 100644
--- a/clang/test/Driver/sanitizer-ld.c
+++ b/clang/test/Driver/sanitizer-ld.c
@@ -920,7 +920,7 @@
 // CHECK-CFI-MINRT-LINUX: "{{.*}}ld{{(.exe)?}}"
 
 // CFI with diagnostics links the UBSan runtime.
-// RUN: not %clang -fsanitize=cfi -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
+// RUN: %clang -fsanitize=cfi -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
 // RUN:     -flto -fvisibility=hidden \
 // RUN:     -fsanitize-minimal-runtime \
 // RUN:     --target=x86_64-unknown-linux -fuse-ld=ld \
@@ -928,7 +928,6 @@
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:     -### %s 2>&1 \
 // RUN:   | %{filecheck} --check-prefix=CHECK-CFI-MINRT-DIAG-LINUX
-// CHECK-CFI-MINRT-DIAG-LINUX: '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
 // CHECK-CFI-MINRT-DIAG-LINUX: "{{.*}}ld{{(.exe)?}}"
 // CHECK-CFI-MINRT-DIAG-LINUX: "--whole-archive" "{{[^"]*}}libclang_rt.ubsan_minimal.a" "--no-whole-archive"
 
@@ -946,7 +945,7 @@
 // CHECK-CFI-MINRT-CROSS-DSO-LINUX: -export-dynamic
 
 // Cross-DSO CFI with diagnostics links just the CFI runtime.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
 // RUN:     -flto -fvisibility=hidden \
 // RUN:     -fsanitize-minimal-runtime \
 // RUN:     -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
@@ -955,7 +954,6 @@
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:     -### %s 2>&1 \
 // RUN:   | %{filecheck} --check-prefix=CHECK-CFI-MINRT-CROSS-DSO-DIAG-LINUX
-// CHECK-CFI-MINRT-CROSS-DSO-DIAG-LINUX: '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-LINUX: "{{.*}}ld{{(.exe)?}}"
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-LINUX: "--whole-archive" "{{[^"]*}}libclang_rt.cfi_diag.a" "--no-whole-archive"
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-LINUX: -export-dynamic
@@ -972,7 +970,7 @@
 // CHECK-CFI-MINRT-CROSS-DSO-ANDROID: "{{.*}}ld{{(.exe)?}}"
 
 // Cross-DSO CFI with diagnostics on Android links just the UBSAN runtime.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
 // RUN:     -flto -fvisibility=hidden \
 // RUN:     -fsanitize-minimal-runtime \
 // RUN:     -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
@@ -981,7 +979,6 @@
 // RUN:     --sysroot=%S/Inputs/basic_android_tree \
 // RUN:     -### %s 2>&1 \
 // RUN:   | %{filecheck} --check-prefix=CHECK-CFI-MINRT-CROSS-DSO-DIAG-ANDROID
-// CHECK-CFI-MINRT-CROSS-DSO-DIAG-ANDROID: '-fsanitize-minimal-runtime' only allowed with '-fsanitize-trap=cfi'
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-ANDROID: "{{.*}}ld{{(.exe)?}}"
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-ANDROID: "{{[^"]*}}libclang_rt.ubsan_minimal.so"
 // CHECK-CFI-MINRT-CROSS-DSO-DIAG-ANDROID: "--export-dynamic-symbol=__cfi_check"

@vitalybuka vitalybuka requested review from fmayer and pcc April 24, 2025 01:17
@vitalybuka
Copy link
Collaborator Author

TODO: In a separate PRs I will add Codegen and runtime tests.

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.drivercfi-allow-cfi-with-minimal-runtime to main April 24, 2025 04:13
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka
Copy link
Collaborator Author

TODO: In a separate PRs I will add Codegen and runtime tests.

Runtime #137245
Codegen #137225

@llvmbot llvmbot added cmake Build system in general and CMake in particular backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang:codegen IR generation bugs: mangling, exceptions, etc. labels Apr 25, 2025
@llvmbot llvmbot added backend:SPIR-V llvm:ir ClangIR Anything related to the ClangIR project labels Apr 25, 2025
@vitalybuka vitalybuka marked this pull request as draft April 25, 2025 06:34
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka vitalybuka removed cmake Build system in general and CMake in particular backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang:codegen IR generation bugs: mangling, exceptions, etc. libc BOLT backend:DirectX HLSL HLSL Language Support backend:SPIR-V llvm:ir ClangIR Anything related to the ClangIR project labels Apr 25, 2025
Created using spr 1.3.4
@vitalybuka vitalybuka marked this pull request as ready for review April 25, 2025 07:54
@vitalybuka vitalybuka merged commit 7aa8ece into main Apr 25, 2025
6 of 8 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/drivercfi-allow-cfi-with-minimal-runtime branch April 25, 2025 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants