[HWASAN] Implement selective instrumentation based on profiling information #83503

kstoimenov · 2024-02-29T23:36:32Z

No description provided.

information.

llvmbot · 2024-02-29T23:37:03Z

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

Author: Kirill Stoimenov (kstoimenov)

Changes

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

1 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+49-2)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 33add6d4cd767b..bf59f10723c2db 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -15,11 +15,14 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/DomTreeUpdater.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/PostDominators.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
@@ -177,6 +180,22 @@ static cl::opt<bool> ClWithTls(
              "platforms that support this"),
     cl::Hidden, cl::init(true));
 
+static cl::opt<bool>
+    CSkipHotCode("hwasan-skip-hot-code",
+                 cl::desc("Do not instument hot functions based on FDO."),
+                 cl::Hidden, cl::init(false));
+
+static cl::opt<int> HotPercentileCutoff("hwasan-percentile-cutoff-hot",
+                                        cl::init(0));
+
+STATISTIC(NumTotalFuncs, "Number of funcs seen by HWASAN");
+STATISTIC(NumHwasanCtors, "Number of HWASAN ctors");
+STATISTIC(NumNoSanitizeFuncs, "Number of no-sanitize HWASAN funcs");
+STATISTIC(NumConsideredFuncs, "Number of funcs considered for HWASAN");
+STATISTIC(NumInstrumentedFuncs, "Number of HWASAN instrumented funcs");
+STATISTIC(NumNoProfileSummaryFuncs, "Number of HWASAN funcs without PS");
+STATISTIC(NumSkippedHotFuncs, "Number of skipped hot HWASAN funcs");
+
 // Mode for selecting how to insert frame record info into the stack ring
 // buffer.
 enum RecordStackHistoryMode {
@@ -1501,11 +1520,39 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
 
 void HWAddressSanitizer::sanitizeFunction(Function &F,
                                           FunctionAnalysisManager &FAM) {
-  if (&F == HwasanCtorFunction)
+  NumTotalFuncs++;
+  if (&F == HwasanCtorFunction) {
+    NumHwasanCtors++;
     return;
+  }
 
-  if (!F.hasFnAttribute(Attribute::SanitizeHWAddress))
+  if (!F.hasFnAttribute(Attribute::SanitizeHWAddress)) {
+    NumNoSanitizeFuncs++;
     return;
+  }
+
+  NumConsideredFuncs++;
+  if (CSkipHotCode) {
+    auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
+    ProfileSummaryInfo *PSI =
+        MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
+    if (PSI != nullptr && PSI->hasProfileSummary()) {
+      auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
+      const bool is_hot =
+          (HotPercentileCutoff.getNumOccurrences() && HotPercentileCutoff >= 0)
+              ? PSI->isFunctionHotInCallGraphNthPercentile(HotPercentileCutoff,
+                                                           &F, BFI)
+              : PSI->isFunctionEntryHot(&F);
+
+      if (is_hot) {
+        ++NumSkippedHotFuncs;
+        return;
+      }
+    } else {
+      ++NumNoProfileSummaryFuncs;
+    }
+  }
+  NumInstrumentedFuncs++;
 
   LLVM_DEBUG(dbgs() << "Function: " << F.getName() << "\n");

llvmbot · 2024-02-29T23:37:04Z

@llvm/pr-subscribers-llvm-transforms

Author: Kirill Stoimenov (kstoimenov)

Changes

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

1 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+49-2)

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 33add6d4cd767b..bf59f10723c2db 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -15,11 +15,14 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/DomTreeUpdater.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/PostDominators.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
@@ -177,6 +180,22 @@ static cl::opt<bool> ClWithTls(
              "platforms that support this"),
     cl::Hidden, cl::init(true));
 
+static cl::opt<bool>
+    CSkipHotCode("hwasan-skip-hot-code",
+                 cl::desc("Do not instument hot functions based on FDO."),
+                 cl::Hidden, cl::init(false));
+
+static cl::opt<int> HotPercentileCutoff("hwasan-percentile-cutoff-hot",
+                                        cl::init(0));
+
+STATISTIC(NumTotalFuncs, "Number of funcs seen by HWASAN");
+STATISTIC(NumHwasanCtors, "Number of HWASAN ctors");
+STATISTIC(NumNoSanitizeFuncs, "Number of no-sanitize HWASAN funcs");
+STATISTIC(NumConsideredFuncs, "Number of funcs considered for HWASAN");
+STATISTIC(NumInstrumentedFuncs, "Number of HWASAN instrumented funcs");
+STATISTIC(NumNoProfileSummaryFuncs, "Number of HWASAN funcs without PS");
+STATISTIC(NumSkippedHotFuncs, "Number of skipped hot HWASAN funcs");
+
 // Mode for selecting how to insert frame record info into the stack ring
 // buffer.
 enum RecordStackHistoryMode {
@@ -1501,11 +1520,39 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
 
 void HWAddressSanitizer::sanitizeFunction(Function &F,
                                           FunctionAnalysisManager &FAM) {
-  if (&F == HwasanCtorFunction)
+  NumTotalFuncs++;
+  if (&F == HwasanCtorFunction) {
+    NumHwasanCtors++;
     return;
+  }
 
-  if (!F.hasFnAttribute(Attribute::SanitizeHWAddress))
+  if (!F.hasFnAttribute(Attribute::SanitizeHWAddress)) {
+    NumNoSanitizeFuncs++;
     return;
+  }
+
+  NumConsideredFuncs++;
+  if (CSkipHotCode) {
+    auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
+    ProfileSummaryInfo *PSI =
+        MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
+    if (PSI != nullptr && PSI->hasProfileSummary()) {
+      auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
+      const bool is_hot =
+          (HotPercentileCutoff.getNumOccurrences() && HotPercentileCutoff >= 0)
+              ? PSI->isFunctionHotInCallGraphNthPercentile(HotPercentileCutoff,
+                                                           &F, BFI)
+              : PSI->isFunctionEntryHot(&F);
+
+      if (is_hot) {
+        ++NumSkippedHotFuncs;
+        return;
+      }
+    } else {
+      ++NumNoProfileSummaryFuncs;
+    }
+  }
+  NumInstrumentedFuncs++;
 
   LLVM_DEBUG(dbgs() << "Function: " << F.getName() << "\n");

github-actions · 2024-02-29T23:39:13Z

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

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

llvm/test/Instrumentation/HWAddressSanitizer/selsan-no-ps.ll

Enna1 · 2024-03-01T03:11:17Z

Hi @kstoimenov @vitalybuka

I have some high level questions:

What is the motivation of profile information based selective instrumentation? is it about code size?
Will the profile information based selective instrumentation be implemented for other sanitizers(asan, msan, tsan)?
Will the profile information based selective instrumentation cause some false negatives or false positives?
IIUR, lets say the compilation unit where the heap-buffer-overflow happens is not instrumented, then there is no hwasan check inserted, so false negative may happen?

Thanks!

edited:
Just noticed this ubsan patch: #83471

RKSimon · 2024-03-01T10:15:28Z

@kstoimenov @vitalybuka This appears to be breaking some bots:

https://lab.llvm.org/buildbot/#/builders/124/builds/9846
Failed Tests (5):
LLVM :: CodeGen/AArch64/shufflevector.ll
LLVM :: CodeGen/Hexagon/loop_align_count.ll
LLVM :: CodeGen/Hexagon/loop_align_count.mir
LLVM :: Instrumentation/HWAddressSanitizer/pgo-opt-out-no-ps.ll
LLVM :: Instrumentation/HWAddressSanitizer/pgo-opt-out.ll

…ng information (#83503)" Broke a build bot: https://lab.llvm.org/buildbot/#/builders/124/builds/9846 This reverts commit e7c3cd2.

kstoimenov · 2024-03-01T17:19:10Z

I reverted it with f6f79d4.

…ing information (#83503)" Added REQUIRES: asserts, which should fix the build problem. This reverts commit f6f79d4.

kstoimenov · 2024-03-04T18:17:37Z

Reapplied in 5675447.

vitalybuka · 2024-03-05T01:43:15Z

Hi @kstoimenov @vitalybuka

I have some high level questions:

What is the motivation of profile information based selective instrumentation? is it about code size?

There are use cases when we can't run 100% of instrumentation because of unacceptable performance overhead. However if we opt out hot code we can still keep most of coverage with minimal loss.

Will the profile information based selective instrumentation be implemented for other sanitizers(asan, msan, tsan)?

ubsan, hwasan only.

To avoid false positives msan, tsan even with no_sanitize attribute require some instrumentation, so perf overhead will be smaller, but still significant. So I think it's infeasible here.

asan for good detection requires allocator quarantine and fake stack. So it may benefit, but still expensive. We don't have plans on that, but anyone is welcome to try to port this patch to Asan.

Will the profile information based selective instrumentation cause some false negatives or false positives?
IIUR, lets say the compilation unit where the heap-buffer-overflow happens is not instrumented, then there is no hwasan check inserted, so false negative may happen?

non-instrumented code with hwasan, like with asan, will not cause false positive.
Some false negative are expected by definition, if code involved into the bug is hot and instrumentation is omitted.

Thanks!

edited: Just noticed this ubsan patch: #83471

…3942) 1. Change tests to use IR instead of -stats to avoid depending on Debug mode 2. Add SkipInstrumentationRandomRate 3. Remove HWASAN from stat strings

kstoimenov added 3 commits February 23, 2024 17:38

[HWASAN] Implement selective instumentation based on profiling

6fff463

information.

Removed cold code.

fc36ab7

Added tests.

269c6c8

llvmbot added compiler-rt:sanitizer llvm:transforms labels Feb 29, 2024

kstoimenov requested a review from vitalybuka February 29, 2024 23:36

For realz added tests.

587cb5d

vitalybuka approved these changes Feb 29, 2024

View reviewed changes

kstoimenov added 2 commits March 1, 2024 00:28

Addressed comments.

addfab9

clang-format

40f330e

kstoimenov merged commit e7c3cd2 into llvm:main Mar 1, 2024

kstoimenov added a commit that referenced this pull request Mar 1, 2024

Revert "[HWASAN] Implement selective instrumentation based on profili…

f6f79d4

…ng information (#83503)" Broke a build bot: https://lab.llvm.org/buildbot/#/builders/124/builds/9846 This reverts commit e7c3cd2.

kstoimenov added a commit that referenced this pull request Mar 4, 2024

Reapply "[HWASAN] Implement selective instrumentation based on profil…

5675447

…ing information (#83503)" Added REQUIRES: asserts, which should fix the build problem. This reverts commit f6f79d4.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[HWASAN] Implement selective instrumentation based on profiling information #83503

[HWASAN] Implement selective instrumentation based on profiling information #83503

Uh oh!

kstoimenov commented Feb 29, 2024

Uh oh!

llvmbot commented Feb 29, 2024

Uh oh!

llvmbot commented Feb 29, 2024

Uh oh!

github-actions bot commented Feb 29, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Enna1 commented Mar 1, 2024 •

edited

Loading

Uh oh!

RKSimon commented Mar 1, 2024

Uh oh!

kstoimenov commented Mar 1, 2024

Uh oh!

kstoimenov commented Mar 4, 2024

Uh oh!

vitalybuka commented Mar 5, 2024

Uh oh!

Uh oh!

[HWASAN] Implement selective instrumentation based on profiling information #83503

[HWASAN] Implement selective instrumentation based on profiling information #83503

Uh oh!

Conversation

kstoimenov commented Feb 29, 2024

Uh oh!

llvmbot commented Feb 29, 2024

Uh oh!

llvmbot commented Feb 29, 2024

Uh oh!

github-actions bot commented Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Enna1 commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RKSimon commented Mar 1, 2024

Uh oh!

kstoimenov commented Mar 1, 2024

Uh oh!

kstoimenov commented Mar 4, 2024

Uh oh!

vitalybuka commented Mar 5, 2024

Uh oh!

Uh oh!

github-actions bot commented Feb 29, 2024 •

edited

Loading

Enna1 commented Mar 1, 2024 •

edited

Loading