[msan] Clear byval shadow in ignored functions

[vitalybuka]

Summary

If function has no sanitize_memory we still reset shadow for nested calls.
The first return from getShadow() correctly returned shadow for argument,
but it didn't reset shadow of byval pointee.

Depends on D117277

[vitalybuka]

update

[eugenis]

Maybe call it "clear byval shadow" instead of "fix".

LGTM

[kda]

LGTM

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Value *getCleanOrigin() {
     return Constant::getNullValue(MS.OriginTy);
   }
 
   /// Get the shadow value for a given Value.
   ///
   /// This function either returns the value set earlier with setShadow,
   /// or extracts if from ParamTLS (for function arguments).
   Value *getShadow(Value *V) {
-    if (!PropagateShadow) return getCleanShadow(V);
     if (Instruction *I = dyn_cast<Instruction>(V)) {
-      if (I->getMetadata("nosanitize"))
+      if (!PropagateShadow || I->getMetadata("nosanitize"))
         return getCleanShadow(V);
       // For instructions the shadow is already stored in the map.
       Value *Shadow = ShadowMap[V];
       if (!Shadow) {
         LLVM_DEBUG(dbgs() << "No shadow: " << *V << "\n" << *(I->getParent()));
         (void)I;
         assert(Shadow && "No shadow for a value");
       }
       return Shadow;
     }
     if (UndefValue *U = dyn_cast<UndefValue>(V)) {
-      Value *AllOnes = PoisonUndef ? getPoisonedShadow(V) : getCleanShadow(V);
+      Value *AllOnes = (PropagateShadow && PoisonUndef) ? getPoisonedShadow(V)
+                                                        : getCleanShadow(V);
       LLVM_DEBUG(dbgs() << "Undef: " << *U << " ==> " << *AllOnes << "\n");
       (void)U;
       return AllOnes;
     }
     if (Argument *A = dyn_cast<Argument>(V)) {
       // For arguments we compute the shadow on demand and store it in the map.
       Value **ShadowPtr = &ShadowMap[V];
       if (*ShadowPtr)
if (Argument *A = dyn_cast<Argument>(V)) {
             // Figure out maximal valid memcpy alignment.
             const Align ArgAlign = DL.getValueOrABITypeAlignment(
                 MaybeAlign(FArg.getParamAlignment()), FArg.getParamByValType());
             Value *CpShadowPtr =
                 getShadowOriginPtr(V, EntryIRB, EntryIRB.getInt8Ty(), ArgAlign,
                                    /*isStore*/ true)
                     .first;
             // TODO(glider): need to copy origins.
-            if (Overflow) {
+            if (!PropagateShadow || Overflow) {
               // ParamTLS overflow.
               EntryIRB.CreateMemSet(
                   CpShadowPtr, Constant::getNullValue(EntryIRB.getInt8Ty()),
                   Size, ArgAlign);
             } else {
               Value *Base = getShadowPtrForArgument(&FArg, EntryIRB, ArgOffset);
               const Align CopyAlign = std::min(ArgAlign, kShadowTLSAlignment);
               Value *Cpy = EntryIRB.CreateMemCpy(CpShadowPtr, CopyAlign, Base,
                                                  CopyAlign, Size);
               LLVM_DEBUG(dbgs() << "  ByValCpy: " << *Cpy << "\n");
               (void)Cpy;
             }
           }
 
-          if (Overflow || FArg.hasByValAttr() ||
+          if (!PropagateShadow || Overflow || FArg.hasByValAttr() ||
               (MS.EagerChecks && FArg.hasAttribute(Attribute::NoUndef))) {
             *ShadowPtr = getCleanShadow(V);
             setOrigin(A, getCleanOrigin());
           } else {
             // Shadow over TLS
             Value *Base = getShadowPtrForArgument(&FArg, EntryIRB, ArgOffset);
             *ShadowPtr = EntryIRB.CreateAlignedLoad(getShadowTy(&FArg), Base,
                                                     kShadowTLSAlignment);

llvm/test/Instrumentation/MemorySanitizer/byval.ll

 entry:
   %x = load i128, i128* %p
   ret i128 %x
 }
 
 define i128 @ByValArgumentNoSanitize(i32, i128* byval(i128) %p) {
 ; CHECK-LABEL: @ByValArgumentNoSanitize(
 ; CHECK-NEXT:  entry:
+; CHECK:         call void @llvm.memset.p0i8.i64(i8* align 8 {{.*}}, i8 0, i64 16, i1 false)
 ; CHECK:         %x = load i128, i128* %p
 ; CHECK:         store i128 0, i128* bitcast ([100 x i64]* @__msan_retval_tls to i128*)
 ; CHECK-NEXT:    store i32 0, i32* @__msan_retval_origin_tls
 ; CHECK-NEXT:    ret i128
 ;
 entry:
   %x = load i128, i128* %p
   ret i128 %x
 }
 
 ; FIXME: Origin of byval pointee is not propagated.
 define void @ByValForward(i32, i128* byval(i128) %p) sanitize_memory {
 ; CHECK-LABEL: @ByValForward(
 ; CHECK-NEXT:  entry:
 ; CHECK:         call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} @__msan_param_tls to i64), i64 8) to i8*), i64 16, i1 false)
 ; CHECK:         store i64 0, i64* getelementptr inbounds ([100 x i64], [100 x i64]* @__msan_param_tls, i32 0, i32 0)
 ; CHECK-NEXT:    call void @Fn(
 ; CHECK-NEXT:    ret void
 ;
 entry:
   call void @Fn(i128* %p)
   ret void
 }
 
-; FIXME: Shadow of byval pointee is copied but not reset.
 define void @ByValForwardNoSanitize(i32, i128* byval(i128) %p) {
 ; CHECK-LABEL: @ByValForwardNoSanitize(
 ; CHECK-NEXT:  entry:
+; CHECK:         call void @llvm.memset.p0i8.i64(i8* align 8 {{.*}}, i8 0, i64 16, i1 false)
 ; CHECK:         store i64 0, i64* getelementptr inbounds ([100 x i64], [100 x i64]* @__msan_param_tls, i32 0, i32 0)
 ; CHECK-NEXT:    call void @Fn(
 ; CHECK-NEXT:    ret void
 ;
 entry:
   call void @Fn(i128* %p)
   ret void
 }
 
 ; FIXME: Origin of %p byval pointee is not propagated.
 define void @ByValForwardByVal(i32, i128* byval(i128) %p) sanitize_memory {
 ; CHECK-LABEL: @ByValForwardByVal(
 ; CHECK-NEXT:  entry:
 ; CHECK:         call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} @__msan_param_tls to i64), i64 8) to i8*), i64 16, i1 false)
 ; CHECK:         call void @llvm.memcpy.p0i8.p0i8.i64(i8* bitcast ([100 x i64]* @__msan_param_tls to i8*), i8* {{.*}}, i64 16, i1 false)
 ; CHECK:         store i32 {{.*}}, i32* getelementptr inbounds ([200 x i32], [200 x i32]* @__msan_param_origin_tls, i32 0, i32 0)
 ; CHECK-NEXT:    call void @FnByVal(
 ; CHECK-NEXT:    ret void
 ;
 entry:
   call void @FnByVal(i128* byval(i128) %p)
   ret void
 }
 
-; FIXME: Shadow of byval pointee is copied but not reset.
+; FIXME: Shadow for byval should be reset not copied before the call.
 define void @ByValForwardByValNoSanitize(i32, i128* byval(i128) %p) {
 ; CHECK-LABEL: @ByValForwardByValNoSanitize(
 ; CHECK-NEXT:  entry:
+; CHECK:         call void @llvm.memset.p0i8.i64(i8* align 8 {{.*}}, i8 0, i64 16, i1 false)
 ; CHECK:         call void @llvm.memcpy.p0i8.p0i8.i64(i8* bitcast ([100 x i64]* @__msan_param_tls to i8*), i8* {{.*}}, i64 16, i1 false) 
 ; CHECK:         store i32 0, i32* getelementptr inbounds ([200 x i32], [200 x i32]* @__msan_param_origin_tls, i32 0, i32 0)
 ; CHECK-NEXT:    call void @FnByVal(
 ; CHECK-NEXT:    ret void
 ;
 entry:
   call void @FnByVal(i128* byval(i128) %p)
   ret void
 }