[HWASAN] Follow up for #83503 implement selective instrumentation #83942

kstoimenov · 2024-03-05T01:49:55Z

Change tests to use IR instead of -stats to avoid depending on Debug mode
Add SkipInstrumentationRandomRate
Remove HWASAN from stat strings

Debug mode, add SkipInstrumentationRandomRate and remove HWASAN from stat strings.

llvmbot · 2024-03-05T01:50:27Z

@llvm/pr-subscribers-llvm-transforms

Author: Kirill Stoimenov (kstoimenov)

Changes

Change tests to use IR instead of -stats to avoid depending on Debug mode
Add SkipInstrumentationRandomRate
Remove HWASAN from stat strings

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

3 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+12-3)
(modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll (+22-10)
(modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll (+14-9)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 4404382a85b7e3..92f8687030d609 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -83,6 +83,8 @@ static const uint64_t kDynamicShadowSentinel =
 
 static const unsigned kShadowBaseAlignment = 32;
 
+static constexpr unsigned MaxRandomRate = 1000;
+
 static cl::opt<std::string>
     ClMemoryAccessCallbackPrefix("hwasan-memory-access-callback-prefix",
                                  cl::desc("Prefix for memory access callbacks"),
@@ -188,9 +190,13 @@ static cl::opt<bool>
 static cl::opt<int> HotPercentileCutoff("hwasan-percentile-cutoff-hot",
                                         cl::init(0));
 
-STATISTIC(NumTotalFuncs, "Number of total funcs HWASAN");
-STATISTIC(NumInstrumentedFuncs, "Number of HWASAN instrumented funcs");
-STATISTIC(NumNoProfileSummaryFuncs, "Number of HWASAN funcs without PS");
+static cl::opt<float> SkipInstRandomRate(
+    "hwasan-skip-inst-random-rate", cl::init(0.0),
+    cl::desc("Probability to skip instrumentation of a function."));
+
+STATISTIC(NumTotalFuncs, "Number of total funcs");
+STATISTIC(NumInstrumentedFuncs, "Number of instrumented funcs");
+STATISTIC(NumNoProfileSummaryFuncs, "Number of funcs without PS");
 
 // Mode for selecting how to insert frame record info into the stack ring
 // buffer.
@@ -1527,6 +1533,9 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
 
   NumTotalFuncs++;
   if (CSkipHotCode) {
+    if ((F.getGUID() % MaxRandomRate) < SkipInstRandomRate) {
+      return;
+    }
     auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
     ProfileSummaryInfo *PSI =
         MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
index 2aa218fa1522d6..c3c2cbe2d5bb92 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
@@ -1,16 +1,28 @@
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S  \
 ; RUN:   -hwasan-skip-hot-code=0 | FileCheck %s --check-prefix=FULL
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S  \
 ; RUN:   -hwasan-skip-hot-code=1 | FileCheck %s --check-prefix=SELSAN
 
-; REQUIRES: asserts
+; FULL: @not_sanitized
+; FULL-NEXT: %x = alloca i8, i64 4
+; FULL: @sanitized_no_ps
+; FULL-SAME: @__hwasan_personality_thunk
 
-; FULL: 1 hwasan - Number of HWASAN instrumented funcs
-; FULL: 1 hwasan - Number of total funcs HWASAN
+; SELSAN: @not_sanitized
+; SELSAN-NEXT: %x = alloca i8, i64 4
+; SELSAN: @sanitized_no_ps
+; SELSAN-SAME: @__hwasan_personality_thunk
 
-; SELSAN: 1 hwasan - Number of HWASAN instrumented funcs
-; SELSAN: 1 hwasan - Number of HWASAN funcs without PS
-; SELSAN: 1 hwasan - Number of total funcs HWASAN
+declare void @use(ptr)
 
-define void @not_sanitized() { ret void }
-define void @sanitized_no_ps() sanitize_hwaddress { ret void }
+define void @not_sanitized() {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+ }
+
+define void @sanitized_no_ps() sanitize_hwaddress {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+ }
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
index 65a5f8c9689678..9cdb927f06760e 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
@@ -1,16 +1,21 @@
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
-; RUN:   -hwasan-skip-hot-code=1 | FileCheck %s --check-prefix=DEFAULT
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
-; RUN:   -hwasan-skip-hot-code=1 -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=PERCENT
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-skip-hot-code=1 \
+; RUN:    | FileCheck %s --check-prefix=DEFAULT
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-skip-hot-code=1 \
+; RUN:    -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=PERCENT
 
-; REQUIRES: asserts
+; DEFAULT: @sanitized
+; DEFAULT-NEXT: %x = alloca i8, i64 4
 
-; DEFAULT: 1 hwasan - Number of total funcs HWASAN
+; PERCENT: @sanitized
+; PERCENT-SAME: @__hwasan_personality_thunk
 
-; PERCENT: 1 hwasan - Number of HWASAN instrumented funcs
-; PERCENT: 1 hwasan - Number of total funcs HWASAN
+declare void @use(ptr)
 
-define void @sanitized() sanitize_hwaddress !prof !36 { ret void }
+define void @sanitized(i32 noundef %0) sanitize_hwaddress !prof !36 {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+}
 
 !llvm.module.flags = !{!6}
 !6 = !{i32 1, !"ProfileSummary", !7}

llvmbot · 2024-03-05T01:50:27Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Kirill Stoimenov (kstoimenov)

Changes

Change tests to use IR instead of -stats to avoid depending on Debug mode
Add SkipInstrumentationRandomRate
Remove HWASAN from stat strings

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

3 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+12-3)
(modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll (+22-10)
(modified) llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll (+14-9)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 4404382a85b7e3..92f8687030d609 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -83,6 +83,8 @@ static const uint64_t kDynamicShadowSentinel =
 
 static const unsigned kShadowBaseAlignment = 32;
 
+static constexpr unsigned MaxRandomRate = 1000;
+
 static cl::opt<std::string>
     ClMemoryAccessCallbackPrefix("hwasan-memory-access-callback-prefix",
                                  cl::desc("Prefix for memory access callbacks"),
@@ -188,9 +190,13 @@ static cl::opt<bool>
 static cl::opt<int> HotPercentileCutoff("hwasan-percentile-cutoff-hot",
                                         cl::init(0));
 
-STATISTIC(NumTotalFuncs, "Number of total funcs HWASAN");
-STATISTIC(NumInstrumentedFuncs, "Number of HWASAN instrumented funcs");
-STATISTIC(NumNoProfileSummaryFuncs, "Number of HWASAN funcs without PS");
+static cl::opt<float> SkipInstRandomRate(
+    "hwasan-skip-inst-random-rate", cl::init(0.0),
+    cl::desc("Probability to skip instrumentation of a function."));
+
+STATISTIC(NumTotalFuncs, "Number of total funcs");
+STATISTIC(NumInstrumentedFuncs, "Number of instrumented funcs");
+STATISTIC(NumNoProfileSummaryFuncs, "Number of funcs without PS");
 
 // Mode for selecting how to insert frame record info into the stack ring
 // buffer.
@@ -1527,6 +1533,9 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
 
   NumTotalFuncs++;
   if (CSkipHotCode) {
+    if ((F.getGUID() % MaxRandomRate) < SkipInstRandomRate) {
+      return;
+    }
     auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
     ProfileSummaryInfo *PSI =
         MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
index 2aa218fa1522d6..c3c2cbe2d5bb92 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
@@ -1,16 +1,28 @@
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S  \
 ; RUN:   -hwasan-skip-hot-code=0 | FileCheck %s --check-prefix=FULL
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S  \
 ; RUN:   -hwasan-skip-hot-code=1 | FileCheck %s --check-prefix=SELSAN
 
-; REQUIRES: asserts
+; FULL: @not_sanitized
+; FULL-NEXT: %x = alloca i8, i64 4
+; FULL: @sanitized_no_ps
+; FULL-SAME: @__hwasan_personality_thunk
 
-; FULL: 1 hwasan - Number of HWASAN instrumented funcs
-; FULL: 1 hwasan - Number of total funcs HWASAN
+; SELSAN: @not_sanitized
+; SELSAN-NEXT: %x = alloca i8, i64 4
+; SELSAN: @sanitized_no_ps
+; SELSAN-SAME: @__hwasan_personality_thunk
 
-; SELSAN: 1 hwasan - Number of HWASAN instrumented funcs
-; SELSAN: 1 hwasan - Number of HWASAN funcs without PS
-; SELSAN: 1 hwasan - Number of total funcs HWASAN
+declare void @use(ptr)
 
-define void @not_sanitized() { ret void }
-define void @sanitized_no_ps() sanitize_hwaddress { ret void }
+define void @not_sanitized() {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+ }
+
+define void @sanitized_no_ps() sanitize_hwaddress {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+ }
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
index 65a5f8c9689678..9cdb927f06760e 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/pgo-opt-out.ll
@@ -1,16 +1,21 @@
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
-; RUN:   -hwasan-skip-hot-code=1 | FileCheck %s --check-prefix=DEFAULT
-; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -stats 2>&1 \
-; RUN:   -hwasan-skip-hot-code=1 -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=PERCENT
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-skip-hot-code=1 \
+; RUN:    | FileCheck %s --check-prefix=DEFAULT
+; RUN: opt < %s -passes='require<profile-summary>,hwasan' -S -hwasan-skip-hot-code=1 \
+; RUN:    -hwasan-percentile-cutoff-hot=700000 | FileCheck %s --check-prefix=PERCENT
 
-; REQUIRES: asserts
+; DEFAULT: @sanitized
+; DEFAULT-NEXT: %x = alloca i8, i64 4
 
-; DEFAULT: 1 hwasan - Number of total funcs HWASAN
+; PERCENT: @sanitized
+; PERCENT-SAME: @__hwasan_personality_thunk
 
-; PERCENT: 1 hwasan - Number of HWASAN instrumented funcs
-; PERCENT: 1 hwasan - Number of total funcs HWASAN
+declare void @use(ptr)
 
-define void @sanitized() sanitize_hwaddress !prof !36 { ret void }
+define void @sanitized(i32 noundef %0) sanitize_hwaddress !prof !36 {
+  %x = alloca i8, i64 4
+  call void @use(ptr %x)
+  ret void
+}
 
 !llvm.module.flags = !{!6}
 !6 = !{i32 1, !"ProfileSummary", !7}

kstoimenov · 2024-03-05T01:58:47Z

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

@@ -1527,6 +1533,9 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,

  NumTotalFuncs++;
  if (CSkipHotCode) {
+    if ((F.getGUID() % MaxRandomRate) < SkipInstRandomRate) {


@vitalybuka probably not correct logic. PTAL.

How is this getGUID value generated?
I wonder if selection will be shuffled after some code changes/compiler updates?
I would assume we want (1) stability with the same code/compiler for reproducibility, (2) shuffle selection on code/compiler updates to get more coverage over time.

I guess we agreed to use createRNG(F.getName()); like in #83471

Module::createRNG docs are unclear:
"The RNG can be seeded via -rng-seed= and is salted with the ModuleID and the provided pass salt."

What is ModuleID? Does it change on code changes?

vitalybuka

I assume you are going to switch to random

dvyukov · 2024-03-06T17:20:10Z

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

-STATISTIC(NumInstrumentedFuncs, "Number of HWASAN instrumented funcs");
-STATISTIC(NumNoProfileSummaryFuncs, "Number of HWASAN funcs without PS");
+static cl::opt<float> SkipInstRandomRate(
+    "hwasan-skip-inst-random-rate", cl::init(0.0),


Do we want to bound it to [0, 1000]? Please also add the expected range of values to the descirption.

For consistently, by comment for UBSAN:

+static cl::opt<float> + RandomRate("remove-traps-random-rate", cl::init(0.0), + cl::desc("Probability value in the range [0.0, 1.0] of " + "unconditional pseudo-random checks removal."));

Debug mode, add SkipInstrumentationRandomRate and remove HWASAN from stat strings.

github-actions · 2024-03-06T20:45:58Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

vitalybuka

Don't create Rng without RandomSkipRate.getNumOccurrences())?

[HWASAN] Change tests to use IR instead of -stats to avoid depnding on

7b69b72

Debug mode, add SkipInstrumentationRandomRate and remove HWASAN from stat strings.

llvmbot added compiler-rt:sanitizer llvm:transforms labels Mar 5, 2024

kstoimenov requested a review from vitalybuka March 5, 2024 01:58

kstoimenov commented Mar 5, 2024

View reviewed changes

vitalybuka requested a review from dvyukov March 6, 2024 17:13

vitalybuka requested changes Mar 6, 2024

View reviewed changes

dvyukov reviewed Mar 6, 2024

View reviewed changes

[HWASAN] Change tests to use IR instead of -stats to avoid depnding on

8acaf61

Debug mode, add SkipInstrumentationRandomRate and remove HWASAN from stat strings.

vitalybuka reviewed Mar 6, 2024

View reviewed changes

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp Outdated Show resolved Hide resolved

vitalybuka reviewed Mar 6, 2024

View reviewed changes

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp Show resolved Hide resolved

kstoimenov added 3 commits March 6, 2024 21:06

Fixing code.

d402744

clang format

a8879e2

Addressed comments.

af9bbdd

vitalybuka self-requested a review March 6, 2024 23:26

vitalybuka approved these changes Mar 6, 2024

View reviewed changes

kstoimenov added 3 commits March 6, 2024 23:27

Changed flag descriptions.

afa8c92

Made Rng optional

ea30dfc

More clang-format

680c51c

kstoimenov merged commit 4258b0e into llvm:main Mar 7, 2024
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HWASAN] Follow up for #83503 implement selective instrumentation #83942

[HWASAN] Follow up for #83503 implement selective instrumentation #83942

kstoimenov commented Mar 5, 2024 •

edited

llvmbot commented Mar 5, 2024

llvmbot commented Mar 5, 2024

kstoimenov Mar 5, 2024

dvyukov Mar 6, 2024

vitalybuka Mar 6, 2024

dvyukov Mar 7, 2024

vitalybuka left a comment

dvyukov Mar 6, 2024

vitalybuka Mar 6, 2024 •

edited

github-actions bot commented Mar 6, 2024 •

edited

vitalybuka left a comment •

edited

[HWASAN] Follow up for #83503 implement selective instrumentation #83942

[HWASAN] Follow up for #83503 implement selective instrumentation #83942

Conversation

kstoimenov commented Mar 5, 2024 • edited

llvmbot commented Mar 5, 2024

llvmbot commented Mar 5, 2024

kstoimenov Mar 5, 2024

Choose a reason for hiding this comment

dvyukov Mar 6, 2024

Choose a reason for hiding this comment

vitalybuka Mar 6, 2024

Choose a reason for hiding this comment

dvyukov Mar 7, 2024

Choose a reason for hiding this comment

vitalybuka left a comment

Choose a reason for hiding this comment

dvyukov Mar 6, 2024

Choose a reason for hiding this comment

vitalybuka Mar 6, 2024 • edited

Choose a reason for hiding this comment

github-actions bot commented Mar 6, 2024 • edited

vitalybuka left a comment • edited

Choose a reason for hiding this comment

kstoimenov commented Mar 5, 2024 •

edited

vitalybuka Mar 6, 2024 •

edited

github-actions bot commented Mar 6, 2024 •

edited

vitalybuka left a comment •

edited