Extend the dfsan store/load callback with write/read address

[stephan.yichao.zhao]

Summary

This helped debugging.

[morehouse]

Please also update the event_callbacks.c test.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

173

Please update this comment.

377

Rather than diverging loads/stores, can we also add the address to the load callback?

1488–1489

Let's reuse the Int8Ptr member variable.

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

7

The variable numbers in this test seem potentially brittle. Can we avoid matching against them?

[stephan.yichao.zhao]

update

[stephan.yichao.zhao]

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

1488–1489

That one is defined in DataFlowSanitizer.

[morehouse]

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

173

Should it be const void* instead?

1488–1489

Yes let's use it.

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

7

All the other ones too. Look at the other tests in this directory. They avoid specifying variable numbers since those can easily change.

[stephan.yichao.zhao]

update

[stephan.yichao.zhao]

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

1488–1489

Done. Sorry. I did not realize DFSF is a struct, so all members are public.

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

7

replaced by regex.

[morehouse]

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

16

To use the variable it's just [[l]]. See https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks.

[stephan.yichao.zhao]

update

[stephan.yichao.zhao]

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

16

Done. Thank you.

[morehouse]

LGTM

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

static cl::opt<bool> ClDebugNonzeroLabels(
     cl::Hidden);
 
 // Experimental feature that inserts callbacks for certain data events.
 // Currently callbacks are only inserted for loads, stores, memory transfers
 // (i.e. memcpy and memmove), and comparisons.
 //
 // If this flag is set to true, the user must provide definitions for the
 // following callback functions:
-//   void __dfsan_load_callback(dfsan_label Label);
-//   void __dfsan_store_callback(dfsan_label Label);
+//   void __dfsan_load_callback(dfsan_label Label, void* addr);
+//   void __dfsan_store_callback(dfsan_label Label, void* addr);
 //   void __dfsan_mem_transfer_callback(dfsan_label *Start, size_t Len);
 //   void __dfsan_cmp_callback(dfsan_label CombinedLabel);
 static cl::opt<bool> ClEventCallbacks(
     "dfsan-event-callbacks",
     cl::desc("Insert calls to __dfsan_*_callback functions on data events."),
     cl::Hidden, cl::init(false));
 
 // Use a distinct bit for each base label, enabling faster unions with less
enum WrapperKind {
     /// pass the return value shadow in a register, while WK_Custom uses an
     /// extra pointer argument to return the shadow.  This allows the wrapped
     /// form of the function type to be expressed in C.
     WK_Custom
   };
 
   Module *Mod;
   LLVMContext *Ctx;
+  Type *Int8Ptr;
   IntegerType *ShadowTy;
   PointerType *ShadowPtrTy;
   IntegerType *IntptrTy;
   ConstantInt *ZeroShadow;
   ConstantInt *ShadowPtrMask;
   ConstantInt *ShadowPtrMul;
   Constant *ArgTLS;
   Constant *RetvalTLS;
   FunctionType *GetArgTLSTy;
   FunctionType *GetRetvalTLSTy;
   Constant *GetArgTLS;
   Constant *GetRetvalTLS;
   Constant *ExternalShadowMask;
   FunctionType *DFSanUnionFnTy;
   FunctionType *DFSanUnionLoadFnTy;
   FunctionType *DFSanUnimplementedFnTy;
   FunctionType *DFSanSetLabelFnTy;
   FunctionType *DFSanNonzeroLabelFnTy;
   FunctionType *DFSanVarargWrapperFnTy;
-  FunctionType *DFSanLoadStoreCmpCallbackFnTy;
+  FunctionType *DFSanCmpCallbackFnTy;
+  FunctionType *DFSanLoadStoreCallbackFnTy;
   FunctionType *DFSanMemTransferCallbackFnTy;
   FunctionCallee DFSanUnionFn;
   FunctionCallee DFSanCheckedUnionFn;
   FunctionCallee DFSanUnionLoadFn;
   FunctionCallee DFSanUnionLoadFast16LabelsFn;
   FunctionCallee DFSanUnimplementedFn;
   FunctionCallee DFSanSetLabelFn;
   FunctionCallee DFSanNonzeroLabelFn;
bool DataFlowSanitizer::init(Module &M) {
   bool IsMIPS64 = TargetTriple.isMIPS64();
   bool IsAArch64 = TargetTriple.getArch() == Triple::aarch64 ||
                    TargetTriple.getArch() == Triple::aarch64_be;
 
   const DataLayout &DL = M.getDataLayout();
 
   Mod = &M;
   Ctx = &M.getContext();
+  Int8Ptr = Type::getInt8PtrTy(*Ctx);
   ShadowTy = IntegerType::get(*Ctx, ShadowWidthBits);
   ShadowPtrTy = PointerType::getUnqual(ShadowTy);
   IntptrTy = DL.getIntPtrType(*Ctx);
   ZeroShadow = ConstantInt::getSigned(ShadowTy, 0);
   ShadowPtrMul = ConstantInt::getSigned(IntptrTy, ShadowWidthBytes);
   if (IsX86_64)
     ShadowPtrMask = ConstantInt::getSigned(IntptrTy, ~0x700000000000LL);
   else if (IsMIPS64)
DFSanUnimplementedFnTy = FunctionType::get(
       Type::getVoidTy(*Ctx), Type::getInt8PtrTy(*Ctx), /*isVarArg=*/false);
   Type *DFSanSetLabelArgs[3] = { ShadowTy, Type::getInt8PtrTy(*Ctx), IntptrTy };
   DFSanSetLabelFnTy = FunctionType::get(Type::getVoidTy(*Ctx),
                                         DFSanSetLabelArgs, /*isVarArg=*/false);
   DFSanNonzeroLabelFnTy = FunctionType::get(
       Type::getVoidTy(*Ctx), None, /*isVarArg=*/false);
   DFSanVarargWrapperFnTy = FunctionType::get(
       Type::getVoidTy(*Ctx), Type::getInt8PtrTy(*Ctx), /*isVarArg=*/false);
-  DFSanLoadStoreCmpCallbackFnTy =
-      FunctionType::get(Type::getVoidTy(*Ctx), ShadowTy, /*isVarArg=*/false);
+  DFSanCmpCallbackFnTy = FunctionType::get(Type::getVoidTy(*Ctx), ShadowTy,
+                                           /*isVarArg=*/false);
+  Type *DFSanLoadStoreCallbackArgs[2] = {ShadowTy, Int8Ptr};
+  DFSanLoadStoreCallbackFnTy =
+      FunctionType::get(Type::getVoidTy(*Ctx), DFSanLoadStoreCallbackArgs,
+                        /*isVarArg=*/false);
   Type *DFSanMemTransferCallbackArgs[2] = {ShadowPtrTy, IntptrTy};
   DFSanMemTransferCallbackFnTy =
       FunctionType::get(Type::getVoidTy(*Ctx), DFSanMemTransferCallbackArgs,
                         /*isVarArg=*/false);
 
   ColdCallWeights = MDBuilder(*Ctx).createBranchWeights(1, 1000);
   return true;
 }
DFSanNonzeroLabelFn =
       Mod->getOrInsertFunction("__dfsan_nonzero_label", DFSanNonzeroLabelFnTy);
   DFSanVarargWrapperFn = Mod->getOrInsertFunction("__dfsan_vararg_wrapper",
                                                   DFSanVarargWrapperFnTy);
 }
 
 // Initializes event callback functions and declare them in the module
 void DataFlowSanitizer::initializeCallbackFunctions(Module &M) {
   DFSanLoadCallbackFn = Mod->getOrInsertFunction("__dfsan_load_callback",
-                                                 DFSanLoadStoreCmpCallbackFnTy);
-  DFSanStoreCallbackFn = Mod->getOrInsertFunction(
-      "__dfsan_store_callback", DFSanLoadStoreCmpCallbackFnTy);
+                                                 DFSanLoadStoreCallbackFnTy);
+  DFSanStoreCallbackFn = Mod->getOrInsertFunction("__dfsan_store_callback",
+                                                  DFSanLoadStoreCallbackFnTy);
   DFSanMemTransferCallbackFn = Mod->getOrInsertFunction(
       "__dfsan_mem_transfer_callback", DFSanMemTransferCallbackFnTy);
-  DFSanCmpCallbackFn = Mod->getOrInsertFunction("__dfsan_cmp_callback",
-                                                DFSanLoadStoreCmpCallbackFnTy);
+  DFSanCmpCallbackFn =
+      Mod->getOrInsertFunction("__dfsan_cmp_callback", DFSanCmpCallbackFnTy);
 }
 
 bool DataFlowSanitizer::runImpl(Module &M) {
   init(M);
 
   if (ABIList.isIn(M, "skip"))
     return false;
 
if (ClCombinePointerLabelsOnLoad) {
     Shadow = DFSF.combineShadows(Shadow, PtrShadow, &LI);
   }
   if (Shadow != DFSF.DFS.ZeroShadow)
     DFSF.NonZeroChecks.push_back(Shadow);
 
   DFSF.setShadow(&LI, Shadow);
   if (ClEventCallbacks) {
     IRBuilder<> IRB(&LI);
-    IRB.CreateCall(DFSF.DFS.DFSanLoadCallbackFn, Shadow);
+    Value *Addr8 = IRB.CreateBitCast(LI.getPointerOperand(), DFSF.DFS.Int8Ptr);
+    IRB.CreateCall(DFSF.DFS.DFSanLoadCallbackFn, {Shadow, Addr8});
   }
 }
 
 void DFSanFunction::storeShadow(Value *Addr, uint64_t Size, Align Alignment,
                                 Value *Shadow, Instruction *Pos) {
   if (AllocaInst *AI = dyn_cast<AllocaInst>(Addr)) {
     const auto i = AllocaShadowMap.find(AI);
     if (i != AllocaShadowMap.end()) {
void DFSanVisitor::visitStoreInst(StoreInst &SI) {
   Value* Shadow = DFSF.getShadow(SI.getValueOperand());
   if (ClCombinePointerLabelsOnStore) {
     Value *PtrShadow = DFSF.getShadow(SI.getPointerOperand());
     Shadow = DFSF.combineShadows(Shadow, PtrShadow, &SI);
   }
   DFSF.storeShadow(SI.getPointerOperand(), Size, Alignment, Shadow, &SI);
   if (ClEventCallbacks) {
     IRBuilder<> IRB(&SI);
-    IRB.CreateCall(DFSF.DFS.DFSanStoreCallbackFn, Shadow);
+    Value *Addr8 = IRB.CreateBitCast(SI.getPointerOperand(), DFSF.DFS.Int8Ptr);
+    IRB.CreateCall(DFSF.DFS.DFSanStoreCallbackFn, {Shadow, Addr8});
   }
 }
 
 void DFSanVisitor::visitUnaryOperator(UnaryOperator &UO) {
   visitOperandShadowInst(UO);
 }
 
 void DFSanVisitor::visitBinaryOperator(BinaryOperator &BO) {

llvm/test/Instrumentation/DataFlowSanitizer/callback.ll

+; RUN: opt < %s -dfsan -dfsan-event-callbacks=1 -S | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8 @load8(i8* %p) {
+  ; CHECK: call void @__dfsan_load_callback(i16 %{{.*}}, i8* %p)
+  ; CHECK: %a = load i8, i8* %p
+  
+  %a = load i8, i8* %p
+  ret i8 %a
+}
+
+define void @store8(i8* %p, i8 %a) {
+  ; CHECK: store i16 %[[l:.*]], i16* %{{.*}}
+  ; CHECK: call void @__dfsan_store_callback(i16 %[[l]], i8* %p)
+  ; CHECK: store i8 %a, i8* %p
+  
+  store i8 %a, i8* %p
+  ret void
+}
+
+define i1 @cmp(i8 %a, i8 %b) {
+  ; CHECK: call void @__dfsan_cmp_callback(i16 %[[l:.*]])
+  ; CHECK: %c = icmp ne i8 %a, %b
+  ; CHECK: store i16 %[[l]], i16* @__dfsan_retval_tls
+  
+  %c = icmp ne i8 %a, %b
+  ret i1 %c
+}
 No newline at end of file