[asan] Implemented flag to emit intrinsics to optimize ASan callbacks.

[kstoimenov]

Pulled constants like shadow base, etc out of the intrinsic.

[vitalybuka]

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

159

Could please please replace Module with targetTriple and pointerSizeInBits.
We don't need entire module for that even if current callers have it.

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

90 ↗

(On Diff #367500)

I don't see implementation of this one.

And I expected getAddressSanitizerParams here, not in AddressSanitizer.h. Is there a reson to declare it there?

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

566

It would be nice to introduce this function as a separate NFC patch.

[vitalybuka]

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

571

Unfortunately we need Kernel as argument, similar to RecoverShift in HWAsan.
Look at CL flags as internal per process constants. So if value is controlled by them then it's OK to assume the same value.
Kernel value however can be set by fronted e.g. from clang and does not match ClEnableKasan.

I propose to have own set of intrinsics for kernel/non-kernel to avoid fancy bit packing or unreadable 1/0 arguments.
Later we can cleunup hwasan as well.

[kstoimenov]

Update before expanding intrinsics.

[kstoimenov]

Removed unused encodeMemToShadowInfo.

[vitalybuka]

clang/test/CodeGen/asan-use-callbacks.cpp

9 ↗

(On Diff #367585)

for -mllvm flag we need test under llvm/test not clang.

llvm test needs to be more meaningful, e.g. check precice value passed into intrinsic
and have 3 versions:
... -mllvm -asan-optimize-callbacks=0
... -mllvm -asan-optimize-callbacks=1
... default

[vitalybuka]

clang/test/CodeGen/asan-use-callbacks.cpp

9 ↗

(On Diff #367585)

to clarify
Goal of clang/test/CodeGen/asan-use-callbacks.cpp to test -fsanitize-address-outline-instrumentation which is frontend (clang) flag.
-mllvm are not clang flags, even clang knows how to forward them to llvm.

I believe we will not add corresponding frontend clang flag for asan-optimize-callbacks. We will make it default ON after some testing and benchmarking.

[kstoimenov]

Removed test code which should be added under a different directory.

[vitalybuka]

Flag is not used anywhere?

[kstoimenov]

Moved files from the parent patch which didn't belong there.

[kstoimenov]

This time AddressSanitizer.cpp should be updated for real.

[vitalybuka]

The test is still missing?

[kstoimenov]

Attempt to split AddressSanitizer.cpp.

[kstoimenov]

Tests are still WIP. This is not ready for review yet. I will ping you when it it.

[vitalybuka]

In D108377#2962445, @kstoimenov wrote:

Tests are still WIP. This is not ready for review yet. I will ping you when it it.

There is also "Add Acction..."-> "Plan changes", which will remove it from reviewers dashboards.

[kstoimenov]

Thanks!

[kstoimenov]

Updating after pushing D107850.

[kstoimenov]

Upload after submitting the main patch.

[kstoimenov]

Added a test.

[kstoimenov]

Fixed the test.

[vitalybuka]

llvm/test/Instrumentation/AddressSanitizer/asan-optimize-callbacks.ll

6 ↗

(On Diff #368959)

Can you please include negative cases as well?

; RUN: opt < %s -asan -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan-optimize-callbacks=0 -S | FileCheck %s --implicit-check-not llvm.asan.check.memaccess
; RUN: opt < %s -asan -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan-optimize-callbacks=0 --asan-kernel -S | FileCheck %s --implicit-check-not llvm.asan.check.memaccess

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

FunctionPass *createAddressSanitizerFunctionPass(
     bool UseAfterScope = false,
     AsanDetectStackUseAfterReturnMode UseAfterReturn =
         AsanDetectStackUseAfterReturnMode::Runtime);
 ModulePass *createModuleAddressSanitizerLegacyPassPass(
     bool CompileKernel = false, bool Recover = false, bool UseGlobalsGC = true,
     bool UseOdrIndicator = true,
     AsanDtorKind DestructorKind = AsanDtorKind::Global);
 
-namespace ASanAccessInfo {
-
-// Bit field positions for accessinfo parameter to llvm.asan.check.memaccess.
-enum {
-  CompileKernelShift = 0, // 1 bit
-  AccessSizeShift = 1, // 4 bits
-  IsWriteShift = 5, // 1 bit
-};
-
-} // namespace ASanAccessInfo
-
 } // namespace llvm
 
 #endif

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

ClMappingOffset("asan-mapping-offset",
                     cl::Hidden, cl::init(0));
 
 // Optimization flags. Not user visible, used mostly for testing
 // and benchmarking the tool.
 
 static cl::opt<bool> ClOpt("asan-opt", cl::desc("Optimize instrumentation"),
                            cl::Hidden, cl::init(true));
 
+static cl::opt<bool> ClOptimizeCallbacks("asan-optimize-callbacks",
+                                         cl::desc("Optimize callbacks"),
+                                         cl::Hidden, cl::init(false));
+
 static cl::opt<bool> ClOptSameTemp(
     "asan-opt-same-temp", cl::desc("Instrument the same temp just once"),
     cl::Hidden, cl::init(true));
 
 static cl::opt<bool> ClOptGlobals("asan-opt-globals",
                                   cl::desc("Don't instrument scalar globals"),
                                   cl::Hidden, cl::init(true));
 
Mapping.OrShadowOffset = !IsAArch64 && !IsPPC64 && !IsSystemZ && !IsPS4CPU &&
                            Mapping.Offset != kDynamicShadowSentinel;
   bool IsAndroidWithIfuncSupport =
       IsAndroid && !TargetTriple.isAndroidVersionLT(21);
   Mapping.InGlobal = ClWithIfunc && IsAndroidWithIfuncSupport && IsArmOrThumb;
 
   return Mapping;
 }
 
 namespace llvm {
 void getAddressSanitizerParams(const Triple &TargetTriple, int LongSize,
                                bool IsKasan, uint64_t *ShadowBase,
                                int *MappingScale, bool *OrShadowOffset) {
   auto Mapping = getShadowMapping(TargetTriple, LongSize, IsKasan);
   *ShadowBase = Mapping.Offset;
   *MappingScale = Mapping.Scale;
   *OrShadowOffset = Mapping.OrShadowOffset;
 }
 } // namespace llvm
 
 static uint64_t getRedzoneSizeForScale(int MappingScale) {
   // Redzone used for stack and globals is at least 32 bytes.
   // For scales 6 and 7, the redzone has to be 64 and 128 bytes respectively.
AddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
         Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
         UseAfterScope(UseAfterScope || ClUseAfterScope),
         UseAfterReturn(ClUseAfterReturn.getNumOccurrences() ? ClUseAfterReturn
                                                             : UseAfterReturn),
         GlobalsMD(*GlobalsMD) {
     C = &(M.getContext());
     LongSize = M.getDataLayout().getPointerSizeInBits();
     IntptrTy = Type::getIntNTy(*C, LongSize);
+    Int8PtrTy = Type::getInt8PtrTy(*C);
     TargetTriple = Triple(M.getTargetTriple());
 
     Mapping = getShadowMapping(TargetTriple, LongSize, this->CompileKernel);
 
     assert(this->UseAfterReturn != AsanDetectStackUseAfterReturnMode::Invalid);
   }
 
   uint64_t getAllocaSizeInBytes(const AllocaInst &AI) const {
private:
   LLVMContext *C;
   Triple TargetTriple;
   int LongSize;
   bool CompileKernel;
   bool Recover;
   bool UseAfterScope;
   AsanDetectStackUseAfterReturnMode UseAfterReturn;
   Type *IntptrTy;
+  Type *Int8PtrTy;
   ShadowMapping Mapping;
   FunctionCallee AsanHandleNoReturnFunc;
   FunctionCallee AsanPtrCmpFunction, AsanPtrSubFunction;
   Constant *AsanShadowGlobal;
 
   // These arrays is indexed by AccessIsWrite, Experiment and log2(AccessSize).
   FunctionCallee AsanErrorCallback[2][2][kNumberOfAccessSizes];
   FunctionCallee AsanMemoryAccessCallback[2][2][kNumberOfAccessSizes];
InsertBefore = instrumentAMDGPUAddress(OrigIns, InsertBefore, Addr,
                                            TypeSize, IsWrite, SizeArgument);
     if (!InsertBefore)
       return;
   }
 
   IRBuilder<> IRB(InsertBefore);
   Value *AddrLong = IRB.CreatePointerCast(Addr, IntptrTy);
   size_t AccessSizeIndex = TypeSizeToSizeIndex(TypeSize);
-  const int32_t AccessInfo =
-      (CompileKernel << ASanAccessInfo::CompileKernelShift) +
-      (IsWrite << ASanAccessInfo::IsWriteShift) +
-      (AccessSizeIndex << ASanAccessInfo::AccessSizeShift);
-
 
   if (UseCalls) {
-    if (ClOptimizeCallbacks) {
-      Value *Ptr8 = IRB.CreatePointerCast(Addr, Int8PtrTy);
-      Module *M = IRB.GetInsertBlock()->getParent()->getParent();
-      IRB.CreateCall(
-          Intrinsic::getDeclaration(M, Intrinsic::asan_check_memaccess),
-          {Ptr8, ConstantInt::get(IRB.getInt32Ty(), AccessInfo)});
-    } else {
-      if (Exp == 0)
-        IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][0][AccessSizeIndex],
-                       AddrLong);
-      else
-        IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][1][AccessSizeIndex],
-                       {AddrLong, ConstantInt::get(IRB.getInt32Ty(), Exp)});
-    }
     return;
   }
 
   Type *ShadowTy =
       IntegerType::get(*C, std::max(8U, TypeSize >> Mapping.Scale));
   Type *ShadowPtrTy = PointerType::get(ShadowTy, 0);
   Value *ShadowPtr = memToShadow(AddrLong, IRB);
   Value *CmpVal = Constant::getNullValue(ShadowTy);