[ASan] Allow for passing AddressSanitizer command line options through the AddressSanitizerOptions struct. #72439

usama54321 · 2023-11-15T21:08:35Z

This patch adds the ability to pass values for the command line options of -max-inline-poisoning-size, -instrumentation-with-calls-threshold and -asan-guard-against-version-mismatch through the AddressSanitizerOptions struct. The motivation is to use these new options when using the pass in Swift.

rdar://118470958

llvmbot · 2023-11-15T21:09:04Z

@llvm/pr-subscribers-llvm-transforms

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

Author: Usama Hameed (usama54321)

Changes

This patch adds the ability to pass values for the command line options of -max-inline-poisoning-size and -instrumentation-with-calls-threshold through the AddressSanitizerOptions struct. The motivation is to use these new options when using the pass in Swift.

rdar://118470958

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

2 Files Affected:

(modified) llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h (+6)
(modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+30-18)

diff --git a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
index ca54387306664c9..0f45240bf28f5f1 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
@@ -20,12 +20,18 @@ namespace llvm {
 class Module;
 class raw_ostream;
 
+static const int InstrumentationWithCallsThreshold = 7000;
+static const uint32_t MaxInlinePoisoningSize = 64;
+
 struct AddressSanitizerOptions {
   bool CompileKernel = false;
   bool Recover = false;
   bool UseAfterScope = false;
   AsanDetectStackUseAfterReturnMode UseAfterReturn =
       AsanDetectStackUseAfterReturnMode::Runtime;
+  int InstrumentationWithCallsThreshold =
+      llvm::InstrumentationWithCallsThreshold;
+  uint32_t MaxInlinePoisoningSize = llvm::MaxInlinePoisoningSize;
 };
 
 /// Public interface to the address sanitizer module pass for instrumenting code
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 80c044b6bee8da2..391d54dbe232442 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -266,7 +266,7 @@ static cl::opt<uint32_t> ClMaxInlinePoisoningSize(
     "asan-max-inline-poisoning-size",
     cl::desc(
         "Inline shadow poisoning for blocks up to the given size in bytes."),
-    cl::Hidden, cl::init(64));
+    cl::Hidden, cl::init(MaxInlinePoisoningSize));
 
 static cl::opt<AsanDetectStackUseAfterReturnMode> ClUseAfterReturn(
     "asan-use-after-return",
@@ -323,11 +323,10 @@ static cl::opt<unsigned> ClRealignStack(
 
 static cl::opt<int> ClInstrumentationWithCallsThreshold(
     "asan-instrumentation-with-call-threshold",
-    cl::desc(
-        "If the function being instrumented contains more than "
-        "this number of memory accesses, use callbacks instead of "
-        "inline checks (-1 means never use callbacks)."),
-    cl::Hidden, cl::init(7000));
+    cl::desc("If the function being instrumented contains more than "
+             "this number of memory accesses, use callbacks instead of "
+             "inline checks (-1 means never use callbacks)."),
+    cl::Hidden, cl::init(InstrumentationWithCallsThreshold));
 
 static cl::opt<std::string> ClMemoryAccessCallbackPrefix(
     "asan-memory-access-callback-prefix",
@@ -644,18 +643,28 @@ namespace {
 
 /// AddressSanitizer: instrument the code in module to find memory bugs.
 struct AddressSanitizer {
-  AddressSanitizer(Module &M, const StackSafetyGlobalInfo *SSGI,
-                   bool CompileKernel = false, bool Recover = false,
-                   bool UseAfterScope = false,
-                   AsanDetectStackUseAfterReturnMode UseAfterReturn =
-                       AsanDetectStackUseAfterReturnMode::Runtime)
+  AddressSanitizer(
+      Module &M, const StackSafetyGlobalInfo *SSGI, bool CompileKernel = false,
+      bool Recover = false, bool UseAfterScope = false,
+      AsanDetectStackUseAfterReturnMode UseAfterReturn =
+          AsanDetectStackUseAfterReturnMode::Runtime,
+      int InstrumentationWithCallsThreshold =
+          llvm::InstrumentationWithCallsThreshold,
+      uint32_t MaxInlinePoisoningSize = llvm::MaxInlinePoisoningSize)
       : CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
                                                             : CompileKernel),
         Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
         UseAfterScope(UseAfterScope || ClUseAfterScope),
         UseAfterReturn(ClUseAfterReturn.getNumOccurrences() ? ClUseAfterReturn
                                                             : UseAfterReturn),
-        SSGI(SSGI) {
+        SSGI(SSGI),
+        InstrumentationWithCallsThreshold(
+            ClInstrumentationWithCallsThreshold.getNumOccurrences() > 0
+                ? ClInstrumentationWithCallsThreshold
+                : InstrumentationWithCallsThreshold),
+        MaxInlinePoisoningSize(ClMaxInlinePoisoningSize.getNumOccurrences() > 0
+                                   ? ClMaxInlinePoisoningSize
+                                   : MaxInlinePoisoningSize) {
     C = &(M.getContext());
     DL = &M.getDataLayout();
     LongSize = M.getDataLayout().getPointerSizeInBits();
@@ -774,6 +783,8 @@ struct AddressSanitizer {
 
   FunctionCallee AMDGPUAddressShared;
   FunctionCallee AMDGPUAddressPrivate;
+  int InstrumentationWithCallsThreshold;
+  uint32_t MaxInlinePoisoningSize;
 };
 
 class ModuleAddressSanitizer {
@@ -1164,9 +1175,10 @@ PreservedAnalyses AddressSanitizerPass::run(Module &M,
   const StackSafetyGlobalInfo *const SSGI =
       ClUseStackSafety ? &MAM.getResult<StackSafetyGlobalAnalysis>(M) : nullptr;
   for (Function &F : M) {
-    AddressSanitizer FunctionSanitizer(M, SSGI, Options.CompileKernel,
-                                       Options.Recover, Options.UseAfterScope,
-                                       Options.UseAfterReturn);
+    AddressSanitizer FunctionSanitizer(
+        M, SSGI, Options.CompileKernel, Options.Recover, Options.UseAfterScope,
+        Options.UseAfterReturn, Options.InstrumentationWithCallsThreshold,
+        Options.MaxInlinePoisoningSize);
     const TargetLibraryInfo &TLI = FAM.getResult<TargetLibraryAnalysis>(F);
     Modified |= FunctionSanitizer.instrumentFunction(F, &TLI);
   }
@@ -2890,9 +2902,9 @@ bool AddressSanitizer::instrumentFunction(Function &F,
     }
   }
 
-  bool UseCalls = (ClInstrumentationWithCallsThreshold >= 0 &&
+  bool UseCalls = (InstrumentationWithCallsThreshold >= 0 &&
                    OperandsToInstrument.size() + IntrinToInstrument.size() >
-                       (unsigned)ClInstrumentationWithCallsThreshold);
+                       (unsigned)InstrumentationWithCallsThreshold);
   const DataLayout &DL = F.getParent()->getDataLayout();
   ObjectSizeOpts ObjSizeOpts;
   ObjSizeOpts.RoundToAlign = true;
@@ -3066,7 +3078,7 @@ void FunctionStackPoisoner::copyToShadow(ArrayRef<uint8_t> ShadowMask,
     for (; j < End && ShadowMask[j] && Val == ShadowBytes[j]; ++j) {
     }
 
-    if (j - i >= ClMaxInlinePoisoningSize) {
+    if (j - i >= ASan.MaxInlinePoisoningSize) {
       copyToShadowInline(ShadowMask, ShadowBytes, Done, i, IRB, ShadowBase);
       IRB.CreateCall(AsanSetShadowFunc[Val],
                      {IRB.CreateAdd(ShadowBase, ConstantInt::get(IntptrTy, i)),

github-actions · 2023-11-15T23:03:06Z

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

wrotki · 2023-11-16T01:18:38Z

LGTM. I assume that existing tests like llvm/test/Instrumentation/AddressSanitizer/instrumentation-with-call-threshold.ll should cover this slightly changed functionality.

usama54321 · 2023-11-17T00:31:04Z

lint fixes

vitalybuka · 2023-11-21T18:47:42Z

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h

@@ -20,12 +20,20 @@ namespace llvm {
 class Module;
 class raw_ostream;

+static const int InstrumentationWithCallsThreshold = 7000;


Please move consts into

AddressSanitizerOptions

they do not deserve to be in llvm:: top namespace

I want to use the same default values in both AddressSanitizerOptions and the cl::init() calls. If I put these in the struct directly, I can't reference them in the cl::init calls which creates the chance of a mismatch between these two places which could lead to weird bugs.

Do you think adding a new namespace here makes sense? Or maybe the mismatch is not a big concern.

vitalybuka · 2023-11-21T18:51:53Z

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

-                   bool UseAfterScope = false,
-                   AsanDetectStackUseAfterReturnMode UseAfterReturn =
-                       AsanDetectStackUseAfterReturnMode::Runtime)
+  AddressSanitizer(


May I ask you to remove default arguments from these constructor, including existing ones. In a separate PR is possible.

This is internal class and it's constructed just onces.
It will make code cleaner

Ok I will do this in a separate PR

vitalybuka · 2023-11-21T18:52:55Z

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

@@ -782,9 +793,13 @@ class ModuleAddressSanitizer {
                         bool Recover = false, bool UseGlobalsGC = true,


same about default args for ModuleAddressSanitizer

the AddressSanitizerOptions struct. This patch adds the ability to pass values for the command line options of -max-inline-poisoning-size, -instrumentation-with-calls-threshold and -asan-guard-against-version-mismatch through the AddressSanitizerOptions struct. The motivation is to use these new options when using the pass in Swift. rdar://118470958

…h the AddressSanitizerOptions struct. (llvm#72439) This patch adds the ability to pass values for the command line options of -max-inline-poisoning-size, -instrumentation-with-calls-threshold and -asan-guard-against-version-mismatch through the AddressSanitizerOptions struct. The motivation is to use these new options when using the pass in Swift. rdar://118470958

usama54321 requested review from kubamracek, vitalybuka, devincoughlin, rsundahl, wrotki and aeubanks November 15, 2023 21:08

llvmbot added compiler-rt:sanitizer llvm:transforms labels Nov 15, 2023

usama54321 requested a review from MaskRay November 15, 2023 21:47

usama54321 force-pushed the eng/118470958 branch from 7abb4fb to 782af05 Compare November 15, 2023 23:00

wrotki approved these changes Nov 16, 2023

View reviewed changes

usama54321 force-pushed the eng/118470958 branch from 782af05 to 977a72c Compare November 17, 2023 00:30

usama54321 mentioned this pull request Nov 17, 2023

[Sanitizers] Add new sanitize-stable-abi flag for libsanitizers. apple/swift#69943

Closed

vitalybuka reviewed Nov 21, 2023

View reviewed changes

vitalybuka approved these changes Nov 21, 2023

View reviewed changes

usama54321 force-pushed the eng/118470958 branch from 977a72c to ca365c2 Compare November 27, 2023 22:15

usama54321 merged commit e88a1ce into llvm:main Nov 28, 2023
3 checks passed

usama54321 mentioned this pull request Dec 4, 2023

[Cherrpick][ASan] Allow for passing AddressSanitizer command line options throug… apple/llvm-project#7844

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ASan] Allow for passing AddressSanitizer command line options through the AddressSanitizerOptions struct. #72439

[ASan] Allow for passing AddressSanitizer command line options through the AddressSanitizerOptions struct. #72439

usama54321 commented Nov 15, 2023 •

edited

llvmbot commented Nov 15, 2023 •

edited

github-actions bot commented Nov 15, 2023 •

edited

wrotki commented Nov 16, 2023

usama54321 commented Nov 17, 2023

vitalybuka Nov 21, 2023 •

edited

usama54321 Nov 27, 2023

vitalybuka Nov 21, 2023

usama54321 Nov 27, 2023

vitalybuka Nov 21, 2023

		@@ -782,9 +793,13 @@ class ModuleAddressSanitizer {
		bool Recover = false, bool UseGlobalsGC = true,

[ASan] Allow for passing AddressSanitizer command line options through the AddressSanitizerOptions struct. #72439

[ASan] Allow for passing AddressSanitizer command line options through the AddressSanitizerOptions struct. #72439

Conversation

usama54321 commented Nov 15, 2023 • edited

llvmbot commented Nov 15, 2023 • edited

github-actions bot commented Nov 15, 2023 • edited

wrotki commented Nov 16, 2023

usama54321 commented Nov 17, 2023

vitalybuka Nov 21, 2023 • edited

Choose a reason for hiding this comment

usama54321 Nov 27, 2023

Choose a reason for hiding this comment

vitalybuka Nov 21, 2023

Choose a reason for hiding this comment

usama54321 Nov 27, 2023

Choose a reason for hiding this comment

vitalybuka Nov 21, 2023

Choose a reason for hiding this comment

usama54321 commented Nov 15, 2023 •

edited

llvmbot commented Nov 15, 2023 •

edited

github-actions bot commented Nov 15, 2023 •

edited

vitalybuka Nov 21, 2023 •

edited