[esan|cfrag] Disable load/store instrumentation for cfrag

[zhaoqin]

Summary

Adds ClInstrumentFastpath option to control fastpath instrumentation.

Avoids the load/store instrumentation for the cache fragmentation tool.

Renames cache_frag_basic.ll to working_set_slow.ll for slowpath
instrumentation test.

Adds the __esan_init check in struct_field_count_basic.ll.

[aizatsky]

lib/Transforms/Instrumentation/EfficiencySanitizer.cpp

129

Make this part of Options. Do not change flag values please.

[zhaoqin]

Adds instrumentation control in Options

[bruening]

include/llvm/Transforms/Instrumentation.h

131 ↗

(On Diff #60064)

This doesn't make sense to me: why expose these here ina public header when they are internal to the instrumentation pass? Having them just be opt options as they were originally seems the better way to go. We want cl::opt options whose defaults vary according to the tool selected. What is needed is for cl::opt to provide a flag indicating whether the option was specified by the user or not, or else to provide more dynamic defaults, or to go with the original CL.

[zhaoqin]

include/llvm/Transforms/Instrumentation.h

131 ↗

(On Diff #60064)

Would an internal option struct works?

[zhaoqin]

revert changes on options.

[zhaoqin]

Since I am not sure how to change Cl options to meet both reviews. I implement a different way to skip load/store instrumentation without changing options.
PTAL.

lib/Transforms/Instrumentation/EfficiencySanitizer.cpp

 // in the code.
 
 static cl::opt<bool> ClInstrumentLoadsAndStores(
     "esan-instrument-loads-and-stores", cl::init(true),
     cl::desc("Instrument loads and stores"), cl::Hidden);
 static cl::opt<bool> ClInstrumentMemIntrinsics(
     "esan-instrument-memintrinsics", cl::init(true),
     cl::desc("Instrument memintrinsics (memset/memcpy/memmove)"), cl::Hidden);
+static cl::opt<bool> ClInstrumentFastpath(
+    "esan-instrument-fastpath", cl::init(true),
+    cl::desc("Instrument fastpath"), cl::Hidden);
 
 // Experiments show that the performance difference can be 2x or more,
 // and accuracy loss is typically negligible, so we turn this on by default.
 static cl::opt<bool> ClAssumeIntraCacheLine(
     "esan-assume-intra-cache-line", cl::init(true),
     cl::desc("Assume each memory access touches just one cache line, for "
              "better performance but with a potential loss of accuracy."),
     cl::Hidden);
OverrideOptionsFromCL(EfficiencySanitizerOptions Options) {
 
   // Direct opt invocation with no params will have the default ESAN_None.
   // We run the default tool in that case.
   if (Options.ToolType == EfficiencySanitizerOptions::ESAN_None)
     Options.ToolType = EfficiencySanitizerOptions::ESAN_CacheFrag;
 
   return Options;
 }
 
 // Create a constant for Str so that we can pass it to the run-time lib.
 static GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str,
                                                     bool AllowMerging) {
   Constant *StrConst = ConstantDataArray::getString(M.getContext(), Str);
   // We use private linkage for module-local strings. If they can be merged
   // with another one, we set the unnamed_addr attribute.
   GlobalVariable *GV =
     new GlobalVariable(M, StrConst->getType(), true,
else
     NumInstrumentedLoads++;
   int Idx = getMemoryAccessFuncIndex(Addr, DL);
   if (Idx < 0) {
     OnAccessFunc = IsStore ? EsanUnalignedStoreN : EsanUnalignedLoadN;
     IRB.CreateCall(OnAccessFunc,
                    {IRB.CreatePointerCast(Addr, IRB.getInt8PtrTy()),
                     ConstantInt::get(IntptrTy, TypeSizeBytes)});
   } else {
-    if (instrumentFastpath(I, DL, IsStore, Addr, Alignment)) {
+    if (ClInstrumentFastpath &&
+        instrumentFastpath(I, DL, IsStore, Addr, Alignment)) {
       NumFastpaths++;
       return true;
     }
     if (Alignment == 0 || Alignment >= 8 || (Alignment % TypeSizeBytes) == 0)
       OnAccessFunc = IsStore ? EsanAlignedStore[Idx] : EsanAlignedLoad[Idx];
     else
       OnAccessFunc = IsStore ? EsanUnalignedStore[Idx] : EsanUnalignedLoad[Idx];
     IRB.CreateCall(OnAccessFunc,
bool EfficiencySanitizer::instrumentFastpath(Instruction *I,
   }
   return false;
 }
 
 bool EfficiencySanitizer::instrumentFastpathCacheFrag(Instruction *I,
                                                       const DataLayout &DL,
                                                       Value *Addr,
                                                       unsigned Alignment) {
-  // TODO(bruening): implement a fastpath for aligned accesses
-  return false;
+  // Do nothing.
+  return true; // Return true to avoid slowpath instrumentation.
 }
 
 bool EfficiencySanitizer::instrumentFastpathWorkingSet(
     Instruction *I, const DataLayout &DL, Value *Addr, unsigned Alignment) {
   assert(ShadowScale[Options.ToolType] == 6); // The code below assumes this
   IRBuilder<> IRB(I);
   Type *OrigTy = cast<PointerType>(Addr->getType())->getElementType();
   const uint32_t TypeSize = DL.getTypeStoreSizeInBits(OrigTy);

test/Instrumentation/EfficiencySanitizer/struct_field_count_basic.ll

 ; Test basic EfficiencySanitizer struct field count instrumentation.
 ;
-; RUN: opt < %s -esan -esan-cache-frag -esan-instrument-loads-and-stores=false -esan-instrument-memintrinsics=false -S | FileCheck %s
+; RUN: opt < %s -esan -esan-cache-frag -S | FileCheck %s
 
 %struct.A = type { i32, i32 }
 %union.U = type { double }
 %struct.C = type { %struct.anon, %union.anon, [10 x i8] }
 %struct.anon = type { i32, i32 }
 %union.anon = type { double }
 
 define i32 @main() {

 ; CHECK-NEXT:   %16 = load i64, i64* getelementptr inbounds ([3 x i64], [3 x i64]* @"struct.C#3#14#13#13", i32 0, i32 2)
 ; CHECK-NEXT:   %17 = add i64 %16, 1
 ; CHECK-NEXT:   store i64 %17, i64* getelementptr inbounds ([3 x i64], [3 x i64]* @"struct.C#3#14#13#13", i32 0, i32 2)
 ; CHECK-NEXT:   %c11 = getelementptr inbounds %struct.C, %struct.C* %arrayidx10, i32 0, i32 2
 ; CHECK-NEXT:   %arrayidx12 = getelementptr inbounds [10 x i8], [10 x i8]* %c11, i64 0, i64 2
 ; CHECK-NEXT:   %k1 = load %struct.A*, %struct.A** %k, align 8
 ; CHECK-NEXT:   %arrayidx13 = getelementptr inbounds %struct.A, %struct.A* %k1, i64 0
 ; CHECK-NEXT:   ret i32 0
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+; Top-level:
+
+; CHECK: define internal void @esan.module_ctor()
+; CHECK: call void @__esan_init(i32 1, i8* bitcast ({ i8*, i32, { i8*, i32, i64*, i8** }* }* @21 to i8*))
+; CHECK: define internal void @esan.module_dtor()
+; CHECK: call void @__esan_exit(i8* bitcast ({ i8*, i32, { i8*, i32, i64*, i8** }* }* @21 to i8*))

test/Instrumentation/EfficiencySanitizer/working_set_slow.ll

-; Test basic EfficiencySanitizer cache frag instrumentation.
+; Test basic EfficiencySanitizer slowpath instrumentation.
 ;
-; RUN: opt < %s -esan -esan-cache-frag -S | FileCheck %s
+; RUN: opt < %s -esan -esan-working-set -esan-instrument-fastpath=false -S | FileCheck %s
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ; Aligned loads:
 
 define i8 @loadAligned1(i8* %a) {
 entry:
   %tmp1 = load i8, i8* %a, align 1
   ret i8 %tmp1

 ; CHECK: call i8* @memset
 ; CHECK: ret void
 }
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ; Top-level:
 
 ; CHECK: define internal void @esan.module_ctor()
-; CHECK: call void @__esan_init(i32 1, i8* bitcast ({ i8*, i32, { i8*, i32, i64*, i8** }* }* @1 to i8*))
+; CHECK: call void @__esan_init(i32 2, i8* null)
 ; CHECK: define internal void @esan.module_dtor()
-; CHECK: call void @__esan_exit(i8* bitcast ({ i8*, i32, { i8*, i32, i64*, i8** }* }* @1 to i8*))
+; CHECK: call void @__esan_exit(i8* null)