IR: Check inalloca call argument originating from parameter

[majnemer]

Summary

Catch more mismatches involving inalloca.

[rnk]

So, we don't generally fail verification because somebody gave us well-formed but nonsensical IR. I kind of compromised on the last verifier change because I think it will save us lots of time, and we can revert it if somebody objects. Consider the FAQ about calling convention mismatches:

http://llvm.org/docs/FAQ.html#why-does-instcombine-simplifycfg-turn-a-call-to-a-function-with-a-mismatched-calling-convention-into-unreachable-why-not-make-the-verifier-reject-it

It's not clear to me that this new check will find lots of optimizer bugs for us, but I could be proven wrong, and yes, it might take more time to figure it out without a verifier check. Anyway, I think we need a second opinion to keep going in this direction, so I added Nick.

If we do go this direction, we can strengthen this check to fail if the call site isn't musttail, because the argument memory won't match up unless tail call succeeds.

[majnemer]

Address review comments.

lib/Analysis/Lint.cpp

Assert1(FT->isVarArg() ?
               FT->getNumParams() == NumActualArgs,
             "Undefined behavior: Call argument count mismatches callee "
             "argument count", &I);
 
     Assert1(FT->getReturnType() == I.getType(),
             "Undefined behavior: Call return type mismatches "
             "callee return type", &I);
 
+    // Conservatively check the inalloca argument.
+    if (CS.hasInAllocaArgument()) {
+      Value *InAllocaArg =
+          CS.getArgument(FT->getNumParams() - 1)->stripInBoundsOffsets();
+      // We have a bug if we can find that there is an underlying alloca without
+      // inalloca.
+      if (auto AI = dyn_cast<AllocaInst>(InAllocaArg))
+        Assert2(AI->isUsedWithInAlloca(),
+                "inalloca argument for call has mismatched alloca", AI, &I);
+
+      // If our inalloca argument came from one of our arguments, check it for
+      // inalloca as well.
+      if (auto A = dyn_cast<Argument>(InAllocaArg)) {
+        Assert2(A->hasInAllocaAttr(), "inalloca argument for call originated "
+                                      "from argument without inalloca",
+                A, &I);
+        Assert1(CS.isMustTailCall(), "inalloca argument comes from previous "
+                                     "caller, call site should be annotated "
+                                     "with musttail",
+                &I);
+      }
+    }
+
     // Check argument types (in case the callee was casted) and attributes.
     // TODO: Verify that caller and callee attributes are compatible.
     Function::arg_iterator PI = F->arg_begin(), PE = F->arg_end();
     CallSite::arg_iterator AI = CS.arg_begin(), AE = CS.arg_end();
     for (; AI != AE; ++AI) {
       Value *Actual = *AI;
       if (PI != PE) {
         Argument *Formal = PI++;

lib/IR/Verifier.cpp

void Verifier::VerifyCallSite(CallSite CS) {
   AttributeSet Attrs = CS.getAttributes();
 
   Assert1(VerifyAttributeCount(Attrs, CS.arg_size()),
           "Attribute after last parameter!", I);
 
   // Verify call attributes.
   VerifyFunctionAttrs(FTy, Attrs, I);
 
-  // Conservatively check the inalloca argument.
-  // We have a bug if we can find that there is an underlying alloca without
-  // inalloca.
-  if (CS.hasInAllocaArgument()) {
-    Value *InAllocaArg = CS.getArgument(FTy->getNumParams() - 1);
-    if (auto AI = dyn_cast<AllocaInst>(InAllocaArg->stripInBoundsOffsets()))
-      Assert2(AI->isUsedWithInAlloca(),
-              "inalloca argument for call has mismatched alloca", AI, I);
-  }
-
   if (FTy->isVarArg()) {
     // FIXME? is 'nest' even legal here?
     bool SawNest = false;
     bool SawReturned = false;
 
     for (unsigned Idx = 1; Idx < 1 + FTy->getNumParams(); ++Idx) {
       if (Attrs.hasAttribute(Idx, Attribute::Nest))
         SawNest = true;

test/Analysis/Lint/inalloca1.ll

-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: opt -lint %s -o /dev/null 2>&1 | FileCheck %s
 
 
 declare void @doit(i64* inalloca %a)
 
 define void @a() {
 entry:
   %a = alloca [2 x i32]
   %b = bitcast [2 x i32]* %a to i64*
   call void @doit(i64* inalloca %b)
 ; CHECK: inalloca argument for call has mismatched alloca
   ret void
 }

test/Analysis/Lint/inalloca2.ll

+; RUN: opt -lint %s -o /dev/null 2>&1 | FileCheck %s
+
+declare void @doit(i64* inalloca %a)
+
+define void @a(i64* %a) {
+entry:
+  call void @doit(i64* inalloca %a)
+; CHECK: inalloca argument for call originated from argument without inalloca
+  ret void
+}

test/Analysis/Lint/inalloca3.ll

+; RUN: opt -lint %s -o /dev/null 2>&1 | FileCheck %s
+
+declare void @doit(i64* inalloca %a)
+
+define void @a(i64* inalloca %a) {
+entry:
+  call void @doit(i64* inalloca %a)
+; CHECK: inalloca argument comes from previous caller, call site should be annotated with musttail
+  ret void
+}