AutoUpgrade: Fix assertion on invalid name mangling usage

This was trying to auto-upgrade a read_register call with missing type
mangling. This first would break since getCalledFunction checks the
callee type is consistent, so this would assert there. After that,
the replacement code would die on the type mismatch. Be more
defensive and let the verifier code produce an error that the IR
is broken.

diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 15961c3..9150b52 100644 (file)
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -2042,13 +2042,17 @@ static Value *UpgradeARMIntrinsicCall(StringRef Name, CallBase *CI, Function *F,
 /// Upgrade a call to an old intrinsic. All argument and return casting must be
 /// provided to seamlessly integrate with existing context.
 void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) {
-  Function *F = CI->getCalledFunction();
+  // Note dyn_cast to Function is not quite the same as getCalledFunction, which
+  // checks the callee's function type matches. It's likely we need to handle
+  // type changes here.
+  Function *F = dyn_cast<Function>(CI->getCalledOperand());
+  if (!F)
+    return;
+
   LLVMContext &C = CI->getContext();
   IRBuilder<> Builder(C);
   Builder.SetInsertPoint(CI->getParent(), CI->getIterator());
 
-  assert(F && "Intrinsic call is not direct?");
-
   if (!NewFn) {
     // Get the Function's name.
     StringRef Name = F->getName();
@@ -3909,21 +3913,29 @@ void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) {
     }
 
     // This must be an upgrade from a named to a literal struct.
-    auto *OldST = cast<StructType>(CI->getType());
-    assert(OldST != NewFn->getReturnType() && "Return type must have changed");
-    assert(OldST->getNumElements() ==
-               cast<StructType>(NewFn->getReturnType())->getNumElements() &&
-           "Must have same number of elements");
-
-    SmallVector<Value *> Args(CI->args());
-    Value *NewCI = Builder.CreateCall(NewFn, Args);
-    Value *Res = PoisonValue::get(OldST);
-    for (unsigned Idx = 0; Idx < OldST->getNumElements(); ++Idx) {
-      Value *Elem = Builder.CreateExtractValue(NewCI, Idx);
-      Res = Builder.CreateInsertValue(Res, Elem, Idx);
+    if (auto *OldST = dyn_cast<StructType>(CI->getType())) {
+      assert(OldST != NewFn->getReturnType() &&
+             "Return type must have changed");
+      assert(OldST->getNumElements() ==
+                 cast<StructType>(NewFn->getReturnType())->getNumElements() &&
+             "Must have same number of elements");
+
+      SmallVector<Value *> Args(CI->args());
+      Value *NewCI = Builder.CreateCall(NewFn, Args);
+      Value *Res = PoisonValue::get(OldST);
+      for (unsigned Idx = 0; Idx < OldST->getNumElements(); ++Idx) {
+        Value *Elem = Builder.CreateExtractValue(NewCI, Idx);
+        Res = Builder.CreateInsertValue(Res, Elem, Idx);
+      }
+      CI->replaceAllUsesWith(Res);
+      CI->eraseFromParent();
+      return;
     }
-    CI->replaceAllUsesWith(Res);
-    CI->eraseFromParent();
+
+    // We're probably about to produce something invalid. Let the verifier catch
+    // it instead of dying here.
+    CI->setCalledOperand(
+        ConstantExpr::getPointerCast(NewFn, CI->getCalledOperand()->getType()));
     return;
   };
   CallInst *NewCall = nullptr;

diff --git a/llvm/test/Assembler/autoupgrade-invalid-name-mangling.ll b/llvm/test/Assembler/autoupgrade-invalid-name-mangling.ll
new file mode 100644 (file)
index 0000000..9b3ae99
--- /dev/null
+++ b/llvm/test/Assembler/autoupgrade-invalid-name-mangling.ll
@@ -0,0 +1,14 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: Intrinsic called with incompatible signature
+; CHECK-NEXT: %reg = call i32 @llvm.read_register.i64(
+; CHECK: Invalid user of intrinsic instruction!
+; CHECK-NEXT: %reg = call i32 @llvm.read_register.i64(
+define i32 @read_register_missing_mangling() {
+  %reg = call i32 @llvm.read_register(metadata !0)
+  ret i32 %reg
+}
+
+declare i64 @llvm.read_register(metadata)
+
+!0 = !{!"foo"}