[llvm-branch-commits] [llvm] eea0313 - [AddressSanitizer] Split out memory intrinsic handling

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: eea0313cd52e43cc48665ba989b6d861c6e35232

URL: 
https://github.com/llvm/llvm-project/commit/eea0313cd52e43cc48665ba989b6d861c6e35232
DIFF: 
https://github.com/llvm/llvm-project/commit/eea0313cd52e43cc48665ba989b6d861c6e35232.diff

LOG: [AddressSanitizer] Split out memory intrinsic handling

Summary:
In both AddressSanitizer and HWAddressSanitizer, we first collect
instructions whose operands should be instrumented and memory intrinsics,
then instrument them. Both during collection and when inserting
instrumentation, they are handled separately.

Collect them separately and instrument them separately. This is a bit
more straightforward, and prepares for collecting operands instead of
instructions in a future patch.

This is patch 2/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77617

Added: 


Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index d6dedc6f76ab..fcb2b17a7cf5 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2652,6 +2652,7 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   // are calls between uses).
   SmallPtrSet TempsToInstrument;
   SmallVector ToInstrument;
+  SmallVector IntrinToInstrument;
   SmallVector NoReturnCalls;
   SmallVector AllBlocks;
   SmallVector PointerComparisonsOrSubtracts;
@@ -2688,8 +2689,11 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   isInterestingPointerSubtraction(&Inst))) {
 PointerComparisonsOrSubtracts.push_back(&Inst);
 continue;
-  } else if (isa(Inst)) {
+  } else if (MemIntrinsic *MI = dyn_cast(&Inst)) {
 // ok, take it.
+IntrinToInstrument.push_back(MI);
+NumInsnsPerBB++;
+continue;
   } else {
 if (isa(Inst)) NumAllocas++;
 if (auto *CB = dyn_cast(&Inst)) {
@@ -2708,9 +2712,9 @@ bool AddressSanitizer::instrumentFunction(Function &F,
 }
   }
 
-  bool UseCalls =
-  (ClInstrumentationWithCallsThreshold >= 0 &&
-   ToInstrument.size() > (unsigned)ClInstrumentationWithCallsThreshold);
+  bool UseCalls = (ClInstrumentationWithCallsThreshold >= 0 &&
+   ToInstrument.size() + IntrinToInstrument.size() >
+   (unsigned)ClInstrumentationWithCallsThreshold);
   const DataLayout &DL = F.getParent()->getDataLayout();
   ObjectSizeOpts ObjSizeOpts;
   ObjSizeOpts.RoundToAlign = true;
@@ -2723,9 +2727,11 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   if (isInterestingMemoryAccess(Inst, &IsWrite, &TypeSize, &Alignment))
 instrumentMop(ObjSizeVis, Inst, UseCalls,
   F.getParent()->getDataLayout());
-  else
-instrumentMemIntrinsic(cast(Inst));
 }
+  }
+  for (auto Inst : IntrinToInstrument) {
+if (!suppressInstrumentationSiteForDebug(NumInstrumented))
+  instrumentMemIntrinsic(Inst);
 FunctionModified = true;
   }
 

diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 9470cb2cfb28..8c9bc428e7f4 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -720,11 +720,6 @@ bool HWAddressSanitizer::instrumentMemAccess(Instruction 
*I) {
   uint64_t TypeSize = 0;
   Value *MaybeMask = nullptr;
 
-  if (ClInstrumentMemIntrinsics && isa(I)) {
-instrumentMemIntrinsic(cast(I));
-return true;
-  }
-
   Value *Addr =
   isInterestingMemoryAccess(I, &IsWrite, &TypeSize, &Alignment, 
&MaybeMask);
 
@@ -1090,6 +1085,7 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
   LLVM_DEBUG(dbgs() << "Function: " << F.getName() << "\n");
 
   SmallVector ToInstrument;
+  SmallVector IntrinToInstrument;
   SmallVector AllocasToInstrument;
   SmallVector RetVec;
   SmallVector LandingPadVec;
@@ -1121,8 +1117,11 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
   uint64_t TypeSize;
   Value *Addr = isInterestingMemoryAccess(&Inst, &IsWrite, &TypeSize,

[llvm-branch-commits] [llvm] 84d341f - [AddressSanitizer] Instrument byval call arguments

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:49+02:00
New Revision: 84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae

URL: 
https://github.com/llvm/llvm-project/commit/84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae
DIFF: 
https://github.com/llvm/llvm-project/commit/84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae.diff

LOG: [AddressSanitizer] Instrument byval call arguments

Summary:
In the LLVM IR, "call" instructions read memory for each byval operand.
For example:

```
$ cat blah.c
struct foo { void *a, *b, *c; };
struct bar { struct foo foo; };
void func1(const struct foo);
void func2(struct bar *bar) { func1(bar->foo); }
$ [...]/bin/clang -S -flto -c blah.c -O2 ; cat blah.s
[...]
define dso_local void @func2(%struct.bar* %bar) local_unnamed_addr #0 {
entry:
  %foo = getelementptr inbounds %struct.bar, %struct.bar* %bar, i64 0, i32 0
  tail call void @func1(%struct.foo* byval(%struct.foo) align 8 %foo) #2
  ret void
}
[...]
$ [...]/bin/clang -S -c blah.c -O2 ; cat blah.s
[...]
func2:  # @func2
[...]
subq$24, %rsp
[...]
movq16(%rdi), %rax
movq%rax, 16(%rsp)
movups  (%rdi), %xmm0
movups  %xmm0, (%rsp)
callq   func1
addq$24, %rsp
[...]
retq
```

Let ASAN instrument these hidden memory accesses.

This is patch 4/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, dexonsmith, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77619

Added: 
llvm/test/Instrumentation/AddressSanitizer/byval-args.ll

Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 6226bcd35381..93326c8fd13a 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -213,6 +213,11 @@ static cl::opt ClInstrumentAtomics(
 cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
 cl::init(true));
 
+static cl::opt
+ClInstrumentByval("asan-instrument-byval",
+  cl::desc("instrument byval call arguments"), cl::Hidden,
+  cl::init(true));
+
 static cl::opt ClAlwaysSlowPath(
 "asan-always-slow-path",
 cl::desc("use instrumentation with slow path for all accesses"), 
cl::Hidden,
@@ -1414,6 +1419,14 @@ void AddressSanitizer::getInterestingMemoryOperands(
 Alignment = (unsigned)AlignmentConstant->getZExtValue();
   Value *Mask = CI->getOperand(2 + OpOffset);
   Interesting.emplace_back(I, OpOffset, IsWrite, Ty, Alignment, Mask);
+} else {
+  for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+ignoreAccess(CI->getArgOperand(ArgNo)))
+  continue;
+Type *Ty = CI->getParamByValType(ArgNo);
+Interesting.emplace_back(I, ArgNo, false, Ty, 1);
+  }
 }
   }
 }

diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 982c1d3516cc..0b9856b5126a 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -97,6 +97,10 @@ static cl::opt ClInstrumentAtomics(
 cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
 cl::init(true));
 
+static cl::opt ClInstrumentByval("hwasan-instrument-byval",
+   cl::desc("instrument byval arguments"),
+   cl::Hidden, cl::init(true));
+
 static cl::opt ClRecover(
 "hwasan-recover",
 cl::desc("Enable recovery mode (continue-after-error)."),
@@ -549,6 +553,14 @@ void HWAddressSanitizer::getInterestingMemoryOperands(
   return;
 Interesting.emplace_back(I, XCHG->getPointerOperandIndex(), true,
  XCHG->getCompareOperand()->getType(), 0);
+  } else if (auto CI = dyn_cast(I)) {
+for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+  if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+  ignoreAccess(CI->getArgOperand(ArgNo)))
+continue;
+  Type *Ty = CI->getParamByValType(ArgNo);
+  Interesting.emplace_back(I, ArgNo

[llvm-branch-commits] [llvm] c9cba16 - [AddressSanitizer] Refactor ClDebug{Min, Max} handling

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: c9cba1600de842f803bb3d55cc4d97deba55f5c8

URL: 
https://github.com/llvm/llvm-project/commit/c9cba1600de842f803bb3d55cc4d97deba55f5c8
DIFF: 
https://github.com/llvm/llvm-project/commit/c9cba1600de842f803bb3d55cc4d97deba55f5c8.diff

LOG: [AddressSanitizer] Refactor ClDebug{Min,Max} handling

Summary:
A following commit will split the loop over ToInstrument into two.
To avoid having to duplicate the condition for suppressing instrumentation
sites based on ClDebug{Min,Max}, refactor it out into a new function.

While we're at it, we can also avoid the indirection through
NumInstrumented for setting FunctionModified.

This is patch 1/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77616

Added: 


Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 4556e3275a10..d6dedc6f76ab 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -638,6 +638,7 @@ struct AddressSanitizer {
  Value *SizeArgument, uint32_t Exp);
   void instrumentMemIntrinsic(MemIntrinsic *MI);
   Value *memToShadow(Value *Shadow, IRBuilder<> &IRB);
+  bool suppressInstrumentationSiteForDebug(int &Instrumented);
   bool instrumentFunction(Function &F, const TargetLibraryInfo *TLI);
   bool maybeInsertAsanInitAtFunctionEntry(Function &F);
   void maybeInsertDynamicShadowAtFunctionEntry(Function &F);
@@ -2610,6 +2611,14 @@ void AddressSanitizer::markEscapedLocalAllocas(Function 
&F) {
   }
 }
 
+bool AddressSanitizer::suppressInstrumentationSiteForDebug(int &Instrumented) {
+  bool ShouldInstrument =
+  ClDebugMin < 0 || ClDebugMax < 0 ||
+  (Instrumented >= ClDebugMin && Instrumented <= ClDebugMax);
+  Instrumented++;
+  return !ShouldInstrument;
+}
+
 bool AddressSanitizer::instrumentFunction(Function &F,
   const TargetLibraryInfo *TLI) {
   if (F.getLinkage() == GlobalValue::AvailableExternallyLinkage) return false;
@@ -2710,15 +2719,14 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   // Instrument.
   int NumInstrumented = 0;
   for (auto Inst : ToInstrument) {
-if (ClDebugMin < 0 || ClDebugMax < 0 ||
-(NumInstrumented >= ClDebugMin && NumInstrumented <= ClDebugMax)) {
+if (!suppressInstrumentationSiteForDebug(NumInstrumented)) {
   if (isInterestingMemoryAccess(Inst, &IsWrite, &TypeSize, &Alignment))
 instrumentMop(ObjSizeVis, Inst, UseCalls,
   F.getParent()->getDataLayout());
   else
 instrumentMemIntrinsic(cast(Inst));
 }
-NumInstrumented++;
+FunctionModified = true;
   }
 
   FunctionStackPoisoner FSP(F, *this);
@@ -2733,10 +2741,10 @@ bool AddressSanitizer::instrumentFunction(Function &F,
 
   for (auto Inst : PointerComparisonsOrSubtracts) {
 instrumentPointerComparisonOrSubtraction(Inst);
-NumInstrumented++;
+FunctionModified = true;
   }
 
-  if (NumInstrumented > 0 || ChangedStack || !NoReturnCalls.empty())
+  if (ChangedStack || !NoReturnCalls.empty())
 FunctionModified = true;
 
   LLVM_DEBUG(dbgs() << "ASAN done instrumenting: " << FunctionModified << " "



___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] 7df5537 - [AddressSanitizer] Refactor: Permit >1 interesting operands per instruction

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: 7df5537f8d3c4f8033b7b32809684cb688fe5ebe

URL: 
https://github.com/llvm/llvm-project/commit/7df5537f8d3c4f8033b7b32809684cb688fe5ebe
DIFF: 
https://github.com/llvm/llvm-project/commit/7df5537f8d3c4f8033b7b32809684cb688fe5ebe.diff

LOG: [AddressSanitizer] Refactor: Permit >1 interesting operands per instruction

Summary:
Refactor getInterestingMemoryOperands() so that information about the
pointer operand is returned through an array of structures instead of
passing each piece of information separately by-value.

This is in preparation for returning information about multiple pointer
operands from a single instruction.

A side effect is that, instead of repeatedly generating the same
information through isInterestingMemoryAccess(), it is now simply collected
once and then passed around; that's probably more efficient.

HWAddressSanitizer has a bunch of copypasted code from AddressSanitizer,
so these changes have to be duplicated.

This is patch 3/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

[glider: renamed llvm::InterestingMemoryOperand::Type to OpType to fix
GCC compilation]

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, jfb, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77618

Added: 
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h

Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git 
a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h 
b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
new file mode 100644
index ..ef33fa2147d1
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
@@ -0,0 +1,49 @@
+//===- Definition of the AddressSanitizer class -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file declares common infrastructure for AddressSanitizer and
+// HWAddressSanitizer.
+//
+//===--===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZERCOMMON_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZERCOMMON_H
+
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Module.h"
+
+namespace llvm {
+
+class InterestingMemoryOperand {
+public:
+  Use *PtrUse;
+  bool IsWrite;
+  Type *OpType;
+  uint64_t TypeSize;
+  unsigned Alignment;
+  // The mask Value, if we're looking at a masked load/store.
+  Value *MaybeMask;
+
+  InterestingMemoryOperand(Instruction *I, unsigned OperandNo, bool IsWrite,
+   class Type *OpType, unsigned Alignment,
+   Value *MaybeMask = nullptr)
+  : IsWrite(IsWrite), OpType(OpType), Alignment(Alignment),
+MaybeMask(MaybeMask) {
+const DataLayout &DL = I->getModule()->getDataLayout();
+TypeSize = DL.getTypeStoreSizeInBits(OpType);
+PtrUse = &I->getOperandUse(OperandNo);
+  }
+
+  Instruction *getInsn() { return cast(PtrUse->getUser()); }
+
+  Value *getPtr() { return PtrUse->get(); }
+};
+
+} // namespace llvm
+
+#endif

diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fcb2b17a7cf5..6226bcd35381 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -69,6 +69,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Instrumentation/AddressSanitizerCommon.h"
 #include "llvm/Transforms/Utils/ASanStackFrameLayout.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -612,16 +613,13 @@ struct AddressSanitizer {
   /// Check if we want (and can) handle this alloca.
   bool isInterestingAlloca(const AllocaInst &AI);
 
-  /// If it is an interesting memory access, return the PointerOperand
-  /// and set IsWrite/Alignment. Otherwise return nullptr.
-  /// MaybeMask is an output parameter for the mask Value, if we're looking at 
a
-  /// masked load/store.
-  Value *i

[llvm-branch-commits] [llvm] c9cba16 - [AddressSanitizer] Refactor ClDebug{Min, Max} handling

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: c9cba1600de842f803bb3d55cc4d97deba55f5c8

URL: 
https://github.com/llvm/llvm-project/commit/c9cba1600de842f803bb3d55cc4d97deba55f5c8
DIFF: 
https://github.com/llvm/llvm-project/commit/c9cba1600de842f803bb3d55cc4d97deba55f5c8.diff

LOG: [AddressSanitizer] Refactor ClDebug{Min,Max} handling

Summary:
A following commit will split the loop over ToInstrument into two.
To avoid having to duplicate the condition for suppressing instrumentation
sites based on ClDebug{Min,Max}, refactor it out into a new function.

While we're at it, we can also avoid the indirection through
NumInstrumented for setting FunctionModified.

This is patch 1/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77616

Added: 


Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 4556e3275a10..d6dedc6f76ab 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -638,6 +638,7 @@ struct AddressSanitizer {
  Value *SizeArgument, uint32_t Exp);
   void instrumentMemIntrinsic(MemIntrinsic *MI);
   Value *memToShadow(Value *Shadow, IRBuilder<> &IRB);
+  bool suppressInstrumentationSiteForDebug(int &Instrumented);
   bool instrumentFunction(Function &F, const TargetLibraryInfo *TLI);
   bool maybeInsertAsanInitAtFunctionEntry(Function &F);
   void maybeInsertDynamicShadowAtFunctionEntry(Function &F);
@@ -2610,6 +2611,14 @@ void AddressSanitizer::markEscapedLocalAllocas(Function 
&F) {
   }
 }
 
+bool AddressSanitizer::suppressInstrumentationSiteForDebug(int &Instrumented) {
+  bool ShouldInstrument =
+  ClDebugMin < 0 || ClDebugMax < 0 ||
+  (Instrumented >= ClDebugMin && Instrumented <= ClDebugMax);
+  Instrumented++;
+  return !ShouldInstrument;
+}
+
 bool AddressSanitizer::instrumentFunction(Function &F,
   const TargetLibraryInfo *TLI) {
   if (F.getLinkage() == GlobalValue::AvailableExternallyLinkage) return false;
@@ -2710,15 +2719,14 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   // Instrument.
   int NumInstrumented = 0;
   for (auto Inst : ToInstrument) {
-if (ClDebugMin < 0 || ClDebugMax < 0 ||
-(NumInstrumented >= ClDebugMin && NumInstrumented <= ClDebugMax)) {
+if (!suppressInstrumentationSiteForDebug(NumInstrumented)) {
   if (isInterestingMemoryAccess(Inst, &IsWrite, &TypeSize, &Alignment))
 instrumentMop(ObjSizeVis, Inst, UseCalls,
   F.getParent()->getDataLayout());
   else
 instrumentMemIntrinsic(cast(Inst));
 }
-NumInstrumented++;
+FunctionModified = true;
   }
 
   FunctionStackPoisoner FSP(F, *this);
@@ -2733,10 +2741,10 @@ bool AddressSanitizer::instrumentFunction(Function &F,
 
   for (auto Inst : PointerComparisonsOrSubtracts) {
 instrumentPointerComparisonOrSubtraction(Inst);
-NumInstrumented++;
+FunctionModified = true;
   }
 
-  if (NumInstrumented > 0 || ChangedStack || !NoReturnCalls.empty())
+  if (ChangedStack || !NoReturnCalls.empty())
 FunctionModified = true;
 
   LLVM_DEBUG(dbgs() << "ASAN done instrumenting: " << FunctionModified << " "



___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] 84d341f - [AddressSanitizer] Instrument byval call arguments

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:49+02:00
New Revision: 84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae

URL: 
https://github.com/llvm/llvm-project/commit/84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae
DIFF: 
https://github.com/llvm/llvm-project/commit/84d341f53d050d2d1b6d657ebf8e23dc4aaab2ae.diff

LOG: [AddressSanitizer] Instrument byval call arguments

Summary:
In the LLVM IR, "call" instructions read memory for each byval operand.
For example:

```
$ cat blah.c
struct foo { void *a, *b, *c; };
struct bar { struct foo foo; };
void func1(const struct foo);
void func2(struct bar *bar) { func1(bar->foo); }
$ [...]/bin/clang -S -flto -c blah.c -O2 ; cat blah.s
[...]
define dso_local void @func2(%struct.bar* %bar) local_unnamed_addr #0 {
entry:
  %foo = getelementptr inbounds %struct.bar, %struct.bar* %bar, i64 0, i32 0
  tail call void @func1(%struct.foo* byval(%struct.foo) align 8 %foo) #2
  ret void
}
[...]
$ [...]/bin/clang -S -c blah.c -O2 ; cat blah.s
[...]
func2:  # @func2
[...]
subq$24, %rsp
[...]
movq16(%rdi), %rax
movq%rax, 16(%rsp)
movups  (%rdi), %xmm0
movups  %xmm0, (%rsp)
callq   func1
addq$24, %rsp
[...]
retq
```

Let ASAN instrument these hidden memory accesses.

This is patch 4/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, dexonsmith, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77619

Added: 
llvm/test/Instrumentation/AddressSanitizer/byval-args.ll

Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 6226bcd35381..93326c8fd13a 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -213,6 +213,11 @@ static cl::opt ClInstrumentAtomics(
 cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
 cl::init(true));
 
+static cl::opt
+ClInstrumentByval("asan-instrument-byval",
+  cl::desc("instrument byval call arguments"), cl::Hidden,
+  cl::init(true));
+
 static cl::opt ClAlwaysSlowPath(
 "asan-always-slow-path",
 cl::desc("use instrumentation with slow path for all accesses"), 
cl::Hidden,
@@ -1414,6 +1419,14 @@ void AddressSanitizer::getInterestingMemoryOperands(
 Alignment = (unsigned)AlignmentConstant->getZExtValue();
   Value *Mask = CI->getOperand(2 + OpOffset);
   Interesting.emplace_back(I, OpOffset, IsWrite, Ty, Alignment, Mask);
+} else {
+  for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+ignoreAccess(CI->getArgOperand(ArgNo)))
+  continue;
+Type *Ty = CI->getParamByValType(ArgNo);
+Interesting.emplace_back(I, ArgNo, false, Ty, 1);
+  }
 }
   }
 }

diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 982c1d3516cc..0b9856b5126a 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -97,6 +97,10 @@ static cl::opt ClInstrumentAtomics(
 cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
 cl::init(true));
 
+static cl::opt ClInstrumentByval("hwasan-instrument-byval",
+   cl::desc("instrument byval arguments"),
+   cl::Hidden, cl::init(true));
+
 static cl::opt ClRecover(
 "hwasan-recover",
 cl::desc("Enable recovery mode (continue-after-error)."),
@@ -549,6 +553,14 @@ void HWAddressSanitizer::getInterestingMemoryOperands(
   return;
 Interesting.emplace_back(I, XCHG->getPointerOperandIndex(), true,
  XCHG->getCompareOperand()->getType(), 0);
+  } else if (auto CI = dyn_cast(I)) {
+for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+  if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+  ignoreAccess(CI->getArgOperand(ArgNo)))
+continue;
+  Type *Ty = CI->getParamByValType(ArgNo);
+  Interesting.emplace_back(I, ArgNo

[llvm-branch-commits] [llvm] eea0313 - [AddressSanitizer] Split out memory intrinsic handling

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: eea0313cd52e43cc48665ba989b6d861c6e35232

URL: 
https://github.com/llvm/llvm-project/commit/eea0313cd52e43cc48665ba989b6d861c6e35232
DIFF: 
https://github.com/llvm/llvm-project/commit/eea0313cd52e43cc48665ba989b6d861c6e35232.diff

LOG: [AddressSanitizer] Split out memory intrinsic handling

Summary:
In both AddressSanitizer and HWAddressSanitizer, we first collect
instructions whose operands should be instrumented and memory intrinsics,
then instrument them. Both during collection and when inserting
instrumentation, they are handled separately.

Collect them separately and instrument them separately. This is a bit
more straightforward, and prepares for collecting operands instead of
instructions in a future patch.

This is patch 2/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77617

Added: 


Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index d6dedc6f76ab..fcb2b17a7cf5 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2652,6 +2652,7 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   // are calls between uses).
   SmallPtrSet TempsToInstrument;
   SmallVector ToInstrument;
+  SmallVector IntrinToInstrument;
   SmallVector NoReturnCalls;
   SmallVector AllBlocks;
   SmallVector PointerComparisonsOrSubtracts;
@@ -2688,8 +2689,11 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   isInterestingPointerSubtraction(&Inst))) {
 PointerComparisonsOrSubtracts.push_back(&Inst);
 continue;
-  } else if (isa(Inst)) {
+  } else if (MemIntrinsic *MI = dyn_cast(&Inst)) {
 // ok, take it.
+IntrinToInstrument.push_back(MI);
+NumInsnsPerBB++;
+continue;
   } else {
 if (isa(Inst)) NumAllocas++;
 if (auto *CB = dyn_cast(&Inst)) {
@@ -2708,9 +2712,9 @@ bool AddressSanitizer::instrumentFunction(Function &F,
 }
   }
 
-  bool UseCalls =
-  (ClInstrumentationWithCallsThreshold >= 0 &&
-   ToInstrument.size() > (unsigned)ClInstrumentationWithCallsThreshold);
+  bool UseCalls = (ClInstrumentationWithCallsThreshold >= 0 &&
+   ToInstrument.size() + IntrinToInstrument.size() >
+   (unsigned)ClInstrumentationWithCallsThreshold);
   const DataLayout &DL = F.getParent()->getDataLayout();
   ObjectSizeOpts ObjSizeOpts;
   ObjSizeOpts.RoundToAlign = true;
@@ -2723,9 +2727,11 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   if (isInterestingMemoryAccess(Inst, &IsWrite, &TypeSize, &Alignment))
 instrumentMop(ObjSizeVis, Inst, UseCalls,
   F.getParent()->getDataLayout());
-  else
-instrumentMemIntrinsic(cast(Inst));
 }
+  }
+  for (auto Inst : IntrinToInstrument) {
+if (!suppressInstrumentationSiteForDebug(NumInstrumented))
+  instrumentMemIntrinsic(Inst);
 FunctionModified = true;
   }
 

diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 9470cb2cfb28..8c9bc428e7f4 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -720,11 +720,6 @@ bool HWAddressSanitizer::instrumentMemAccess(Instruction 
*I) {
   uint64_t TypeSize = 0;
   Value *MaybeMask = nullptr;
 
-  if (ClInstrumentMemIntrinsics && isa(I)) {
-instrumentMemIntrinsic(cast(I));
-return true;
-  }
-
   Value *Addr =
   isInterestingMemoryAccess(I, &IsWrite, &TypeSize, &Alignment, 
&MaybeMask);
 
@@ -1090,6 +1085,7 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
   LLVM_DEBUG(dbgs() << "Function: " << F.getName() << "\n");
 
   SmallVector ToInstrument;
+  SmallVector IntrinToInstrument;
   SmallVector AllocasToInstrument;
   SmallVector RetVec;
   SmallVector LandingPadVec;
@@ -1121,8 +1117,11 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
   uint64_t TypeSize;
   Value *Addr = isInterestingMemoryAccess(&Inst, &IsWrite, &TypeSize,

[llvm-branch-commits] [llvm] 7df5537 - [AddressSanitizer] Refactor: Permit >1 interesting operands per instruction

2020-04-30 Thread Alexander Potapenko via llvm-branch-commits 

Author: Jann Horn
Date: 2020-04-30T15:00:48+02:00
New Revision: 7df5537f8d3c4f8033b7b32809684cb688fe5ebe

URL: 
https://github.com/llvm/llvm-project/commit/7df5537f8d3c4f8033b7b32809684cb688fe5ebe
DIFF: 
https://github.com/llvm/llvm-project/commit/7df5537f8d3c4f8033b7b32809684cb688fe5ebe.diff

LOG: [AddressSanitizer] Refactor: Permit >1 interesting operands per instruction

Summary:
Refactor getInterestingMemoryOperands() so that information about the
pointer operand is returned through an array of structures instead of
passing each piece of information separately by-value.

This is in preparation for returning information about multiple pointer
operands from a single instruction.

A side effect is that, instead of repeatedly generating the same
information through isInterestingMemoryAccess(), it is now simply collected
once and then passed around; that's probably more efficient.

HWAddressSanitizer has a bunch of copypasted code from AddressSanitizer,
so these changes have to be duplicated.

This is patch 3/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor 
ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory 
intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit 
>1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval 
call arguments

[glider: renamed llvm::InterestingMemoryOperand::Type to OpType to fix
GCC compilation]

Reviewers: kcc, glider

Reviewed By: glider

Subscribers: hiraditya, jfb, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77618

Added: 
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h

Modified: 
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Removed: 




diff  --git 
a/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h 
b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
new file mode 100644
index ..ef33fa2147d1
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
@@ -0,0 +1,49 @@
+//===- Definition of the AddressSanitizer class -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file declares common infrastructure for AddressSanitizer and
+// HWAddressSanitizer.
+//
+//===--===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZERCOMMON_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZERCOMMON_H
+
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Module.h"
+
+namespace llvm {
+
+class InterestingMemoryOperand {
+public:
+  Use *PtrUse;
+  bool IsWrite;
+  Type *OpType;
+  uint64_t TypeSize;
+  unsigned Alignment;
+  // The mask Value, if we're looking at a masked load/store.
+  Value *MaybeMask;
+
+  InterestingMemoryOperand(Instruction *I, unsigned OperandNo, bool IsWrite,
+   class Type *OpType, unsigned Alignment,
+   Value *MaybeMask = nullptr)
+  : IsWrite(IsWrite), OpType(OpType), Alignment(Alignment),
+MaybeMask(MaybeMask) {
+const DataLayout &DL = I->getModule()->getDataLayout();
+TypeSize = DL.getTypeStoreSizeInBits(OpType);
+PtrUse = &I->getOperandUse(OperandNo);
+  }
+
+  Instruction *getInsn() { return cast(PtrUse->getUser()); }
+
+  Value *getPtr() { return PtrUse->get(); }
+};
+
+} // namespace llvm
+
+#endif

diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fcb2b17a7cf5..6226bcd35381 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -69,6 +69,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Instrumentation/AddressSanitizerCommon.h"
 #include "llvm/Transforms/Utils/ASanStackFrameLayout.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -612,16 +613,13 @@ struct AddressSanitizer {
   /// Check if we want (and can) handle this alloca.
   bool isInterestingAlloca(const AllocaInst &AI);
 
-  /// If it is an interesting memory access, return the PointerOperand
-  /// and set IsWrite/Alignment. Otherwise return nullptr.
-  /// MaybeMask is an output parameter for the mask Value, if we're looking at 
a
-  /// masked load/store.
-  Value *i

[llvm-branch-commits] [clang] [Clang] Add __builtin_allow_sanitize_check() (PR #172030)

2025-12-15 Thread Alexander Potapenko via llvm-branch-commits 


@@ -3549,6 +3549,38 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 llvm::MetadataAsValue::get(Ctx, llvm::MDString::get(Ctx, Kind)));
 return RValue::get(Allow);
   }
+  case Builtin::BI__builtin_allow_sanitize_check: {
+Intrinsic::ID IntrID = Intrinsic::not_intrinsic;
+StringRef SanitizerName =
+cast(E->getArg(0)->IgnoreParenCasts())->getString();
+
+if (SanitizerName == "address" || SanitizerName == "kernel-address") {

ramosian-glider wrote:

Won't this provoke the users to use "address" and "kernel-address" 
interchangeably?
If we want this, maybe it should be reflected in the documentation?

https://github.com/llvm/llvm-project/pull/172030
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Clang] Add __builtin_allow_sanitize_check() (PR #172030)

2025-12-15 Thread Alexander Potapenko via llvm-branch-commits 

https://github.com/ramosian-glider approved this pull request.

Two nits.

https://github.com/llvm/llvm-project/pull/172030
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Clang] Add __builtin_allow_sanitize_check() (PR #172030)

2025-12-15 Thread Alexander Potapenko via llvm-branch-commits 

https://github.com/ramosian-glider edited 
https://github.com/llvm/llvm-project/pull/172030
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Clang] Add __builtin_allow_sanitize_check() (PR #172030)

2025-12-15 Thread Alexander Potapenko via llvm-branch-commits 


@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void test_builtin_allow_sanitize_check() {
+  // Test with non-string literal argument

ramosian-glider wrote:

Shouldn't comments in this file end with a period?

https://github.com/llvm/llvm-project/pull/172030
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits