[ASan] Fixed null pointer bug introduced in D112098.

[kstoimenov]

Summary

Also added some more test to cover the "else if" part.

[RKSimon]

LGTM - thanks

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

void AddressSanitizer::getInterestingMemoryOperands(
   if (I->hasMetadata("nosanitize"))
     return;
 
   // Do not instrument the load fetching the dynamic shadow address.
   if (LocalDynamicShadow == I)
     return;
 
   if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
-    if (!ClInstrumentReads || ignoreAccess(LI, LI->getPointerOperand()))
+    if (!ClInstrumentReads || ignoreAccess(I, LI->getPointerOperand()))
       return;
     Interesting.emplace_back(I, LI->getPointerOperandIndex(), false,
                              LI->getType(), LI->getAlign());
   } else if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
-    if (!ClInstrumentWrites || ignoreAccess(LI, SI->getPointerOperand()))
+    if (!ClInstrumentWrites || ignoreAccess(I, SI->getPointerOperand()))
       return;
     Interesting.emplace_back(I, SI->getPointerOperandIndex(), true,
                              SI->getValueOperand()->getType(), SI->getAlign());
   } else if (AtomicRMWInst *RMW = dyn_cast<AtomicRMWInst>(I)) {
-    if (!ClInstrumentAtomics || ignoreAccess(LI, RMW->getPointerOperand()))
+    if (!ClInstrumentAtomics || ignoreAccess(I, RMW->getPointerOperand()))
       return;
     Interesting.emplace_back(I, RMW->getPointerOperandIndex(), true,
                              RMW->getValOperand()->getType(), None);
   } else if (AtomicCmpXchgInst *XCHG = dyn_cast<AtomicCmpXchgInst>(I)) {
-    if (!ClInstrumentAtomics || ignoreAccess(LI, XCHG->getPointerOperand()))
+    if (!ClInstrumentAtomics || ignoreAccess(I, XCHG->getPointerOperand()))
       return;
     Interesting.emplace_back(I, XCHG->getPointerOperandIndex(), true,
                              XCHG->getCompareOperand()->getType(), None);
   } else if (auto CI = dyn_cast<CallInst>(I)) {
     if (CI->getIntrinsicID() == Intrinsic::masked_load ||
         CI->getIntrinsicID() == Intrinsic::masked_store) {
       bool IsWrite = CI->getIntrinsicID() == Intrinsic::masked_store;
       // Masked store has an initial operand for the value.
       unsigned OpOffset = IsWrite ? 1 : 0;
       if (IsWrite ? !ClInstrumentWrites : !ClInstrumentReads)
         return;
 
       auto BasePtr = CI->getOperand(OpOffset);
-      if (ignoreAccess(LI, BasePtr))
+      if (ignoreAccess(I, BasePtr))
         return;
       Type *Ty = IsWrite ? CI->getArgOperand(0)->getType() : CI->getType();
       MaybeAlign Alignment = Align(1);
       // Otherwise no alignment guarantees. We probably got Undef.
       if (auto *Op = dyn_cast<ConstantInt>(CI->getOperand(1 + OpOffset)))
         Alignment = Op->getMaybeAlignValue();
       Value *Mask = CI->getOperand(2 + OpOffset);
       Interesting.emplace_back(I, OpOffset, IsWrite, Ty, Alignment, Mask);
     } else {
       for (unsigned ArgNo = 0; ArgNo < CI->arg_size(); ArgNo++) {
         if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
-            ignoreAccess(LI, CI->getArgOperand(ArgNo)))
+            ignoreAccess(I, CI->getArgOperand(ArgNo)))
           continue;
         Type *Ty = CI->getParamByValType(ArgNo);
         Interesting.emplace_back(I, ArgNo, false, Ty, Align(1));
       }
     }
   }
 }
 

llvm/test/Instrumentation/AddressSanitizer/asan-stack-safety.ll

 ; REQUIRES: x86-registered-target
 
 ; RUN: opt < %s -S -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan \
 ; RUN:     -asan-use-stack-safety=0  -o - | FileCheck %s --check-prefixes=NOSAFETY
 ; RUN: opt < %s -S -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan \
 ; RUN:     -asan-use-stack-safety=1 -o - | FileCheck %s --check-prefixes=SAFETY
 ; RUN: opt < %s -S -enable-new-pm=1 -asan-instrumentation-with-call-threshold=0 \
 ; RUN:     -passes='asan-pipeline' -asan-use-stack-safety=0 -o - | FileCheck %s --check-prefixes=NOSAFETY
 ; RUN: opt < %s -S -enable-new-pm=1 -asan-instrumentation-with-call-threshold=0 \
 ; RUN:     -passes='asan-pipeline' -asan-use-stack-safety=1 -o - | FileCheck %s --check-prefixes=SAFETY
 ; NOSAFETY: call void @__asan_load1
+; NOSAFETY: call void @__asan_store1
+; NOSAFETY: call void @__asan_store1
+; NOSAFETY: call void @__asan_store1
 ; SAFETY-NOT: call void @__asan_load1
+; SAFETY-NOT: call void @__asan_store1
+; SAFETY-NOT: call void @__asan_store1
+; SAFETY-NOT: call void @__asan_store1
 
-define i32 @stack-safety() sanitize_address {
+define i32 @load() sanitize_address {
   %buf = alloca [10 x i8], align 1
   %arrayidx = getelementptr inbounds [10 x i8], [10 x i8]* %buf, i64 0, i64 0
   %1 = load i8, i8* %arrayidx, align 1
   ret i32 0
 }
+
+define i32 @store() sanitize_address {
+  %buf = alloca [10 x i8], align 1
+  %arrayidx = getelementptr inbounds [10 x i8], [10 x i8]* %buf, i64 0, i64 0
+  store i8 0, i8* %arrayidx
+  ret i32 0
+}
+
+
+define void @atomicrmw() sanitize_address {
+  %buf = alloca [10 x i8], align 1
+  %arrayidx = getelementptr inbounds [10 x i8], [10 x i8]* %buf, i64 0, i64 0
+  %1 = atomicrmw add i8* %arrayidx, i8 1 seq_cst
+  ret void
+}
+
+define void @cmpxchg(i8 %compare_to, i8 %new_value) sanitize_address {
+  %buf = alloca [10 x i8], align 1
+  %arrayidx = getelementptr inbounds [10 x i8], [10 x i8]* %buf, i64 0, i64 0
+  %1 = cmpxchg i8* %arrayidx, i8 %compare_to, i8 %new_value seq_cst seq_cst
+  ret void
+}