Skip to content

[Sanitizers] prefer fsanitize flag over lower-allow mllvm flag #145786

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Jun 25, 2025

The fsanitize flag in question is -fsanitize-skip-hot-cutoff.

We are planning to deprecate -mllvm -ubsan-guard-checks. Then,
emission of the allow_ubsan_check is only controlled by the fsanitize
flag, at which point it would be very confusing if the cutoff from the
lower-allow mllvm flag applied.

Also, as is, the lower-allow mllvm flag also controls
allow_runtime_check. So now we don't have a way to control the hotness
of this without also overriding the fsanitize flag

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Florian Mayer (fmayer)

Changes

The fsanitize flag in question is -fsanitize-skip-hot-cutoff.

We are planning to deprecate -mllvm -ubsan-guard-checks. Then,
emission of the allow_ubsan_check is only controlled by the fsanitize
flag, at which point it would be very confusing if the cutoff from the
lower-allow mllvm flag applied.

Also, as is, the lower-allow mllvm flag also controls
allow_runtime_check. So now we don't have a way to control the hotness
of this without also overriding the fsanitize flag


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp (+4-6)
diff --git a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
index 10e908ef73ce5..e15fb186352bd 100644
--- a/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
+++ b/llvm/lib/Transforms/Instrumentation/LowerAllowCheckPass.cpp
@@ -31,7 +31,8 @@ using namespace llvm;
 
 static cl::opt<int>
     HotPercentileCutoff("lower-allow-check-percentile-cutoff-hot",
-                        cl::desc("Hot percentile cutoff."));
+                        cl::desc("Hot percentile cutoff."),
+                      cl::init(0));
 
 static cl::opt<float>
     RandomRate("lower-allow-check-random-rate",
@@ -84,15 +85,12 @@ static bool removeUbsanTraps(Function &F, const BlockFrequencyInfo &BFI,
   };
 
   auto GetCutoff = [&](const IntrinsicInst *II) -> unsigned {
-    if (HotPercentileCutoff.getNumOccurrences())
-      return HotPercentileCutoff;
-    else if (II->getIntrinsicID() == Intrinsic::allow_ubsan_check) {
+    if (II->getIntrinsicID() == Intrinsic::allow_ubsan_check) {
       auto *Kind = cast<ConstantInt>(II->getArgOperand(0));
       if (Kind->getZExtValue() < cutoffs.size())
         return cutoffs[Kind->getZExtValue()];
     }
-
-    return 0;
+    return HotPercentileCutoff;
   };
 
   auto ShouldRemoveHot = [&](const BasicBlock &BB, unsigned int cutoff) {

Created using spr 1.3.4
@fmayer fmayer requested a review from thurstond June 25, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants