Handle llvm.launder.invariant.group in msan.

[TokarIP]

Summary

[MSan] handle llvm.launder.invariant.group

Msan used to give false-positives in

class Foo {
 public:
  virtual ~Foo() {};
};

// Return true iff *x is set.
bool f1(void **x, bool flag);

Foo* f() {
  void *p;
  bool found;
  found = f1(&p,flag);
  if (found) {
    // p is always set here.
    return static_cast<Foo*>(p); // False positive here.
  }
  return nullptr;
}

[eugenis]

Thank you.
Do you mind handling llvm.strip.invariant.group in the same change?

I'm not sure if llvm.invariant.start and llvm.invariant.end need any handling. It kind of makes sense to ignore the shadow of their pointer arguments, since (a) it will be used in an actual load/store where uninit would be detected and (b) we don't want to raise an alarm in the case the intrinsic call is speculated.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

2567

Why bitcast? From the intrinsic definition, the types must match.

llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll

26

The test could be a lot simpler, something like

i8* f(i8 *p) {
return @llvm.launder.invariant.group(p);
}

[Prazek]

+1, strip.invariant.group probably needs the same handling. Thanks for fixing!

[TokarIP]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

2567

You are right, fixed.

llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll

26

This is just a n IR from minimal reproducer for false-positive, so I wanted to keep it as a regression test.
I've tried your test case, but it generates same IR with and without this patch.

[eugenis]

LGTM with one more test

llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll

26

This does not sound right. Before this patch there should be a shadow check for the intrinsic argument, after the patch - only this:

%0 = load i64, i64* getelementptr inbounds ([100 x i64], [100 x i64]* @__msan_param_tls, i32 0, i32 0), align 8
%q = call i8* @llvm.launder.invariant.group.p0i8(i8* %p)
store i64 %0, i64* getelementptr inbounds ([100 x i64], [100 x i64]* @__msan_retval_tls, i32 0, i32 0), align 8
ret i8* %q

Sure, let's keep this test, it gives extra context about the problem, but please add a minimal one for llvm.strip.invariant.group.

[TokarIP]

Added llvm.strip.invariant.group test, checked that if fails without this patch.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

bool handleUnknownIntrinsic(IntrinsicInst &I) {
     if (I.doesNotAccessMemory())
       if (maybeHandleSimpleNomemIntrinsic(I))
         return true;
 
     // FIXME: detect and handle SSE maskstore/maskload
     return false;
   }
 
+  void handleLaunderInvariantGroup(IntrinsicInst &I) {
+    IRBuilder<> IRB(&I);
+    setShadow(&I, IRB.CreateBitCast(getShadow(&I, 0), getShadowTy(&I)));
+    setOrigin(&I, getOrigin(&I, 0));
+  }
+
   void handleLifetimeStart(IntrinsicInst &I) {
     if (!PoisonStack)
       return;
     DenseMap<Value *, AllocaInst *> AllocaForValue;
     AllocaInst *AI =
         llvm::findAllocaForValue(I.getArgOperand(1), AllocaForValue);
     if (!AI)
       InstrumentLifetimeStart = false;
void handleBmiIntrinsic(IntrinsicInst &I) {
     setOriginForNaryOp(I);
   }
 
   void visitIntrinsicInst(IntrinsicInst &I) {
     switch (I.getIntrinsicID()) {
     case Intrinsic::lifetime_start:
       handleLifetimeStart(I);
       break;
+    case Intrinsic::launder_invariant_group:
+      handleLaunderInvariantGroup(I);
+      break;
     case Intrinsic::bswap:
       handleBswap(I);
       break;
     case Intrinsic::masked_store:
       handleMaskedStore(I);
       break;
     case Intrinsic::masked_load:
       handleMaskedLoad(I);

llvm/test/Instrumentation/MemorySanitizer/msan_llvm_launder_invariant.ll

+; Make sure MSan handles llvm.launder.invariant.group correctly.
+
+; RUN: opt < %s -msan -msan-kernel=1 -O1 -S | FileCheck -check-prefixes=CHECK %s
+; RUN: opt < %s -msan -O1 -S | FileCheck -check-prefixes=CHECK %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%class.Foo = type { i32 (...)** }
+@flag = dso_local local_unnamed_addr global i8 0, align 1
+
+define dso_local %class.Foo* @_Z1fv() local_unnamed_addr #0 {
+entry:
+  %p = alloca i8*, align 8
+  %0 = bitcast i8** %p to i8*
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+  %1 = load i8, i8* @flag, align 1
+  %tobool = icmp ne i8 %1, 0
+  %call = call zeroext i1 @_Z2f1PPvb(i8** nonnull %p, i1 zeroext %tobool)
+  %2 = load i8*, i8** %p, align 8
+  %3 = call i8* @llvm.launder.invariant.group.p0i8(i8* %2)
+  %4 = bitcast i8* %3 to %class.Foo*
+  %retval.0 = select i1 %call, %class.Foo* %4, %class.Foo* null
+  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+  ret %class.Foo* %retval.0
+}
+
+; CHECK-NOT: call void @__msan_warning_noreturn
+
+declare dso_local zeroext i1 @_Z2f1PPvb(i8**, i1 zeroext) local_unnamed_addr
+
+declare i8* @llvm.launder.invariant.group.p0i8(i8*)
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+attributes #0 = { sanitize_memory uwtable }