Skip to content

Conversation

@vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Apr 15, 2025

Show why we don't need regular CFI runtime, when CFI diag runtime is linked.

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 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 15, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

Show why we don't need regular CFI runtime, if CFI diag is linked.


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

1 Files Affected:

  • (modified) clang/lib/Driver/SanitizerArgs.cpp (+1-2)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 3c7cd562a14e3..1db9da1b60939 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -371,8 +371,7 @@ bool SanitizerArgs::needsUbsanCXXRt() const {
 }
 
 bool SanitizerArgs::needsCfiRt() const {
-  return !(Sanitizers.Mask & SanitizerKind::CFI & ~TrapSanitizers.Mask) &&
-         CfiCrossDso && !ImplicitCfiRuntime;
+  return !needsCfiDiagRt() && CfiCrossDso && !ImplicitCfiRuntime;
 }
 
 bool SanitizerArgs::needsCfiDiagRt() const {

@vitalybuka vitalybuka requested a review from Copilot April 15, 2025 23:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

clang/lib/Driver/SanitizerArgs.cpp:374

  • The refactored boolean logic now relies on needsCfiDiagRt() to encapsulate the previous explicit bitmask evaluation. Please confirm that needsCfiDiagRt() accurately represents the same conditions as the original expression to avoid unintended behavior.
return !needsCfiDiagRt() && CfiCrossDso && !ImplicitCfiRuntime;

@vitalybuka vitalybuka requested review from fmayer and thurstond April 15, 2025 23:57
Copy link
Contributor

@fmayer fmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Driver

nit2: I don't think this is really reordering.

@vitalybuka vitalybuka changed the title [NFC][Drive] Reorder boolean expression [NFC][Driver][CFI] Update boolean expression Apr 16, 2025
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 2e9ab7c into main Apr 16, 2025
9 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcdrive-reorder-boolean-expression branch April 16, 2025 15:09
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants