[llvm] e989b8b - AutoUpgrade: Fix assertion on invalid name mangling usage

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2022-11-16T11:18:02-08:00
New Revision: e989b8bb5fb36abac6e8f82809f06144dd762113

URL: https://github.com/llvm/llvm-project/commit/e989b8bb5fb36abac6e8f82809f06144dd762113
DIFF: https://github.com/llvm/llvm-project/commit/e989b8bb5fb36abac6e8f82809f06144dd762113.diff

LOG: 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.

Added: 
    llvm/test/Assembler/autoupgrade-invalid-name-mangling.ll

Modified: 
    llvm/lib/IR/AutoUpgrade.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 15961c32ea8b3..9150b52085e80 100644
--- 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
index 0000000000000..9b3ae99594d60
--- /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"}