Skip to content
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

[ASan] AddressSanitizerPass constructor should honor the AsanCtorKind argument #72330

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

usama54321
Copy link
Member

Currently, the ConstructorKind member variable in AddressSanitizerPass gets overriden by the ClConstructorKind whether the option is passed from the command line or not. This override should only happen if the ClConstructorKind argument is passed from the command line. Otherwise, the constructor should honor the argument passed to it. This patch makes this fix.

rdar://118423755

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 15, 2023

@llvm/pr-subscribers-llvm-transforms

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

Author: Usama Hameed (usama54321)

Changes

Currently, the ConstructorKind member variable in AddressSanitizerPass gets overriden by the ClConstructorKind whether the option is passed from the command line or not. This override should only happen if the ClConstructorKind argument is passed from the command line. Otherwise, the constructor should honor the argument passed to it. This patch makes this fix.

rdar://118423755


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h (+2-1)
  • (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+5-2)
diff --git a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
index d697b72cde05e95..48da3c99d645edc 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h
@@ -22,7 +22,8 @@ enum class AsanDtorKind {
 /// Types of ASan module constructors supported
 enum class AsanCtorKind {
   None,
-  Global
+  Global,
+  Invalid
 };
 
 /// Mode of ASan detect stack use after return
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 5c2763850ac6540..0e1fa6a2bc2c20d 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -355,7 +355,7 @@ static cl::opt<AsanCtorKind> ClConstructorKind(
     cl::values(clEnumValN(AsanCtorKind::None, "none", "No constructors"),
                clEnumValN(AsanCtorKind::Global, "global",
                           "Use global constructors")),
-    cl::init(AsanCtorKind::Global), cl::Hidden);
+    cl::init(AsanCtorKind::Invalid), cl::Hidden);
 // These flags allow to change the shadow mapping.
 // The shadow mapping looks like
 //    Shadow = (Mem >> scale) + offset
@@ -813,6 +813,9 @@ class ModuleAddressSanitizer {
     if (ClOverrideDestructorKind != AsanDtorKind::Invalid)
       this->DestructorKind = ClOverrideDestructorKind;
     assert(this->DestructorKind != AsanDtorKind::Invalid);
+
+    if (ClConstructorKind != AsanCtorKind::Invalid)
+      this->ConstructorKind = ClConstructorKind;
   }
 
   bool instrumentModule(Module &);
@@ -1149,7 +1152,7 @@ AddressSanitizerPass::AddressSanitizerPass(
     AsanCtorKind ConstructorKind)
     : Options(Options), UseGlobalGC(UseGlobalGC),
       UseOdrIndicator(UseOdrIndicator), DestructorKind(DestructorKind),
-      ConstructorKind(ClConstructorKind) {}
+      ConstructorKind(ConstructorKind) {}
 
 PreservedAnalyses AddressSanitizerPass::run(Module &M,
                                             ModuleAnalysisManager &MAM) {

Copy link

github-actions bot commented Nov 15, 2023

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

argument.

Currently, the ConstructorKind member variable in AddressSanitizerPass
gets overriden by the ClConstructorKind whether the option is passed
from the command line or not.  This override should only happen if the
ClConstructorKind argument is passed from the command line.  Otherwise,
the constructor should honor the argument passed to it. This patch makes
this fix.

rdar://118423755
@vitalybuka
Copy link
Collaborator

To properly test this, we need to move DestructorKind and ConstructorKind (and the rest) into AddressSanitizerOptions
and extend parseASanPassOptions. It will make this patch unnecessary complicated. But follow up patches are welcomed.

@usama54321
Copy link
Member Author

Thanks @vitalybuka. I will merge this for now and try to follow this up with an AddressSanitizerOptions patch

@usama54321 usama54321 merged commit 4fe29d0 into llvm:main Nov 17, 2023
3 checks passed
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
… argument (llvm#72330)

Currently, the ConstructorKind member variable in AddressSanitizerPass
gets overriden by the ClConstructorKind whether the option is passed
from the command line or not. This override should only happen if the
ClConstructorKind argument is passed from the command line. Otherwise,
the constructor should honor the argument passed to it. This patch makes
this fix.

rdar://118423755
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
… argument (llvm#72330)

Currently, the ConstructorKind member variable in AddressSanitizerPass
gets overriden by the ClConstructorKind whether the option is passed
from the command line or not. This override should only happen if the
ClConstructorKind argument is passed from the command line. Otherwise,
the constructor should honor the argument passed to it. This patch makes
this fix.

rdar://118423755
usama54321 added a commit to usama54321/apple-llvm-project that referenced this pull request Nov 29, 2023
… argument (llvm#72330)

Currently, the ConstructorKind member variable in AddressSanitizerPass
gets overriden by the ClConstructorKind whether the option is passed
from the command line or not. This override should only happen if the
ClConstructorKind argument is passed from the command line. Otherwise,
the constructor should honor the argument passed to it. This patch makes
this fix.

rdar://118423755
usama54321 added a commit to apple/llvm-project that referenced this pull request Nov 30, 2023
… argument (llvm#72330)

Currently, the ConstructorKind member variable in AddressSanitizerPass
gets overriden by the ClConstructorKind whether the option is passed
from the command line or not. This override should only happen if the
ClConstructorKind argument is passed from the command line. Otherwise,
the constructor should honor the argument passed to it. This patch makes
this fix.

rdar://118423755
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.

None yet

4 participants