[hwasan] Respect returns attribute when tracking values.

[fmayer]

formatting

[fmayer]

format

[fmayer]

comments

[fmayer]

add test

[vitalybuka]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

398

Isn't this in the call will make it unsafe anyway?

417

does SCEV can look through such calls?

llvm/lib/Analysis/ValueTracking.cpp

4536

There is "TEST_P(FindAllocaForValueTest, findAllocaForValue*" tests which need to be extended for this test.
Improving findAllocaForValue is a patch by itself and may affect other components, so I ask you to not mix this with StackSafetyAnalysis (if possible)

llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

140

Having that retptr has no body here, SAFETY should make it "call to retptr" unsafe anyway

[fmayer]

split change

[fmayer]

llvm/lib/Analysis/StackSafetyAnalysis.cpp

398

This is for being able to prove accesses safe, not the whole alloca.

417

Yes, it does.

llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

140

Yes, but it can tell that the store is safe.

llvm/lib/Analysis/StackSafetyAnalysis.cpp

for (const Use &UI : V->uses()) {
                    UI, Ptr, DL.getTypeStoreSize(I->getOperand(0)->getType())));
         break;
       }
 
       case Instruction::Ret:
         // Information leak.
         // FIXME: Process parameters correctly. This is a leak only if we return
         // alloca.
         US.addRange(I, UnknownRange);
         break;
 
       case Instruction::Call:
       case Instruction::Invoke: {
         if (I->isLifetimeStartOrEnd())
           break;
 
         if (AI && !SL.isAliveAfter(AI, I)) {
           US.addRange(I, UnknownRange);
           break;
         }
 
         if (const MemIntrinsic *MI = dyn_cast<MemIntrinsic>(I)) {
           US.addRange(I, getMemIntrinsicAccessRange(MI, UI, Ptr));
           break;
         }
 
         const auto &CB = cast<CallBase>(*I);
+        if (CB.getReturnedArgOperand() == V) {
+          if (Visited.insert(I).second)
+            WorkList.push_back(cast<const Instruction>(I));
+        }
         if (!CB.isArgOperand(&UI)) {
           US.addRange(I, UnknownRange);
           break;
         }
 
         unsigned ArgNo = CB.getArgOperandNo(&UI);
         if (CB.isByValArgument(ArgNo)) {
           US.addRange(I, getAccessRange(
                              UI, Ptr,
                              DL.getTypeStoreSize(CB.getParamByValType(ArgNo))));
           break;
         }
-
         // FIXME: consult devirt?
         // Do not follow aliases, otherwise we could inadvertently follow
         // dso_preemptable aliases or aliases with interposable linkage.
         const GlobalValue *Callee =
             dyn_cast<GlobalValue>(CB.getCalledOperand()->stripPointerCasts());
         if (!Callee) {
           US.addRange(I, UnknownRange);
           break;

llvm/lib/Analysis/ValueTracking.cpp

if (AllocaInst *AI = dyn_cast<AllocaInst>(V)) {
         AddWork(IncValue);
     } else if (auto *SI = dyn_cast<SelectInst>(V)) {
       AddWork(SI->getTrueValue());
       AddWork(SI->getFalseValue());
     } else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(V)) {
       if (OffsetZero && !GEP->hasAllZeroIndices())
         return nullptr;
       AddWork(GEP->getPointerOperand());
+    } else if (CallBase *CB = dyn_cast<CallBase>(V)) {
+      Value *Returned = CB->getReturnedArgOperand();
+      if (Returned) {
+        AddWork(Returned);
+      } else {
+        return nullptr;
+      }
     } else {
       return nullptr;
     }
   } while (!Worklist.empty());
 
   return Result;
 }
 

llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

entry:
   %buf.sroa.0 = alloca i8, align 4
   %ptr = call i8* @getptr(i8* %buf.sroa.0)
   call void @llvm.lifetime.start.p0i8(i64 10, i8* nonnull %ptr)
   store volatile i8 0, i8* %ptr, align 4, !tbaa !8
   call void @llvm.lifetime.end.p0i8(i64 10, i8* nonnull %ptr)
   ret i32 0
 }
 
+; Check an alloca with out of range GEP to ensure it gets a tag.
+define i32 @test_retptr(i32* %a) sanitize_hwaddress {
+entry:
+  ; CHECK-LABEL: @test_retptr
+  ; NOSAFETY: call {{.*}}__hwasan_generate_tag
+  ; NOSAFETY: call {{.*}}__hwasan_store
+  ; SAFETY: call {{.*}}__hwasan_generate_tag
+  ; SAFETY-NOT: call {{.*}}__hwasan_store
+  ; NOSTACK-NOT: call {{.*}}__hwasan_generate_tag
+  ; NOSTACK-NOT: call {{.*}}__hwasan_store
+  %buf.sroa.0 = alloca i8, align 4
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  %ptr = call i8* @retptr(i8* %buf.sroa.0)
+  store volatile i8 0, i8* %ptr, align 4, !tbaa !8
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %buf.sroa.0)
+  ret i32 0
+}
+
 ; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
 declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
 
 ; Function Attrs: argmemonly mustprogress nofree nosync nounwind willreturn
 declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
 
 declare void @use(i8* nocapture)
 declare i32 @getoffset()
 declare i8* @getptr(i8* nocapture)
+declare i8* @retptr(i8* returned)
 
 !8 = !{!9, !9, i64 0}
 !9 = !{!"omnipotent char", !10, i64 0}
 !10 = !{!"Simple C/C++ TBAA"}