[GlobalOpt][Evaluator] Don't evaluate calls with signature mismatch

[nikic]

The global ctor evaluator tries to evalute function calls where the call function type and function type do not match, by performing bitcasts. This currently causes a crash when calling a void function with non-void return type.

I've opted to remove this functionality entirely rather than fixing this specific case. With opaque pointers, there shouldn't be a legitimate use case for this anymore, as we don't need to look through pointer type casts. Doing other bitcasts is very iffy because it ignores ABI considerations. We should at least leave adjusting the signatures to make them line up to InstCombine (which also does some iffy things, but is at least somewhat more constrained).

Fixes #118725.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

The global ctor evaluator tries to evalute function calls where the call function type and function type do not match, by performing bitcasts. This currently causes a crash when calling a void function with non-void return type.

I've opted to remove this functionality entirely rather than fixing this specific case. With opaque pointers, there shouldn't be a legitimate use case for this anymore, as we don't need to look through pointer type casts. Doing other bitcasts is very iffy because it ignores ABI considerations. We should at least leave adjusting the signatures to make them line up to InstCombine (which also does some iffy things, but is at least somewhat more constrained).

Fixes #118725.

---

Full diff: https://github.com/llvm/llvm-project/pull/119548.diff

5 Files Affected:

• (modified) llvm/include/llvm/Transforms/Utils/Evaluator.h (-3)
• (modified) llvm/lib/Transforms/Utils/Evaluator.cpp (+6-34)
• (modified) llvm/test/Transforms/GlobalOpt/evaluate-call-errors.ll (+1-2)
• (modified) llvm/test/Transforms/GlobalOpt/evaluate-constfold-call.ll (+4-7)
• (added) llvm/test/Transforms/GlobalOpt/evaluate-ret-void-mismatch.ll (+26)

diff --git a/llvm/include/llvm/Transforms/Utils/Evaluator.h b/llvm/include/llvm/Transforms/Utils/Evaluator.h
index 5d53773b5d6b65..118037625421a7 100644
--- a/llvm/include/llvm/Transforms/Utils/Evaluator.h
+++ b/llvm/include/llvm/Transforms/Utils/Evaluator.h
@@ -125,9 +125,6 @@ class Evaluator {
     ValueStack.back()[V] = C;
   }
 
-  /// Casts call result to a type of bitcast call expression
-  Constant *castCallResultIfNeeded(Type *ReturnType, Constant *RV);
-
   /// Given call site return callee and list of its formal arguments
   Function *getCalleeWithFormalArgs(CallBase &CB,
                                     SmallVectorImpl<Constant *> &Formals);
diff --git a/llvm/lib/Transforms/Utils/Evaluator.cpp b/llvm/lib/Transforms/Utils/Evaluator.cpp
index cf1a8b4af11266..2af447aadce22e 100644
--- a/llvm/lib/Transforms/Utils/Evaluator.cpp
+++ b/llvm/lib/Transforms/Utils/Evaluator.cpp
@@ -253,40 +253,17 @@ Evaluator::getCalleeWithFormalArgs(CallBase &CB,
 
 bool Evaluator::getFormalParams(CallBase &CB, Function *F,
                                 SmallVectorImpl<Constant *> &Formals) {
-  if (!F)
-    return false;
-
   auto *FTy = F->getFunctionType();
-  if (FTy->getNumParams() > CB.arg_size()) {
-    LLVM_DEBUG(dbgs() << "Too few arguments for function.\n");
+  if (FTy != CB.getFunctionType()) {
+    LLVM_DEBUG(dbgs() << "Signature mismatch.\n");
     return false;
   }
 
-  auto ArgI = CB.arg_begin();
-  for (Type *PTy : FTy->params()) {
-    auto *ArgC = ConstantFoldLoadThroughBitcast(getVal(*ArgI), PTy, DL);
-    if (!ArgC) {
-      LLVM_DEBUG(dbgs() << "Can not convert function argument.\n");
-      return false;
-    }
-    Formals.push_back(ArgC);
-    ++ArgI;
-  }
+  for (Value *Arg : CB.args())
+    Formals.push_back(getVal(Arg));
   return true;
 }
 
-/// If call expression contains bitcast then we may need to cast
-/// evaluated return value to a type of the call expression.
-Constant *Evaluator::castCallResultIfNeeded(Type *ReturnType, Constant *RV) {
-  if (!RV || RV->getType() == ReturnType)
-    return RV;
-
-  RV = ConstantFoldLoadThroughBitcast(RV, ReturnType, DL);
-  if (!RV)
-    LLVM_DEBUG(dbgs() << "Failed to fold bitcast call expr\n");
-  return RV;
-}
-
 /// Evaluate all instructions in block BB, returning true if successful, false
 /// if we can't evaluate it.  NewBB returns the next BB that control flows into,
 /// or null upon return. StrippedPointerCastsForAliasAnalysis is set to true if
@@ -520,9 +497,7 @@ bool Evaluator::EvaluateBlock(BasicBlock::iterator CurInst, BasicBlock *&NextBB,
         if (Callee->isDeclaration()) {
           // If this is a function we can constant fold, do it.
           if (Constant *C = ConstantFoldCall(&CB, Callee, Formals, TLI)) {
-            InstResult = castCallResultIfNeeded(CB.getType(), C);
-            if (!InstResult)
-              return false;
+            InstResult = C;
             LLVM_DEBUG(dbgs() << "Constant folded function call. Result: "
                               << *InstResult << "\n");
           } else {
@@ -544,10 +519,7 @@ bool Evaluator::EvaluateBlock(BasicBlock::iterator CurInst, BasicBlock *&NextBB,
             return false;
           }
           ValueStack.pop_back();
-          InstResult = castCallResultIfNeeded(CB.getType(), RetVal);
-          if (RetVal && !InstResult)
-            return false;
-
+          InstResult = RetVal;
           if (InstResult) {
             LLVM_DEBUG(dbgs() << "Successfully evaluated function. Result: "
                               << *InstResult << "\n\n");
diff --git a/llvm/test/Transforms/GlobalOpt/evaluate-call-errors.ll b/llvm/test/Transforms/GlobalOpt/evaluate-call-errors.ll
index 510edae69d7c4c..853bbb562644cc 100644
--- a/llvm/test/Transforms/GlobalOpt/evaluate-call-errors.ll
+++ b/llvm/test/Transforms/GlobalOpt/evaluate-call-errors.ll
@@ -3,8 +3,7 @@
 ; REQUIRES: asserts
 ; RUN: opt -passes=globalopt,instcombine -S -debug-only=evaluator %s -o %t 2>&1 | FileCheck %s
 
-; CHECK: Failed to fold bitcast call expr
-; CHECK: Can not convert function argument
+; CHECK: Signature mismatch.
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.12.0"
diff --git a/llvm/test/Transforms/GlobalOpt/evaluate-constfold-call.ll b/llvm/test/Transforms/GlobalOpt/evaluate-constfold-call.ll
index 83d8bc6e7c24fb..6400988e7cc096 100644
--- a/llvm/test/Transforms/GlobalOpt/evaluate-constfold-call.ll
+++ b/llvm/test/Transforms/GlobalOpt/evaluate-constfold-call.ll
@@ -1,12 +1,9 @@
-; Check if we can evaluate a bitcasted call to a function which is constant folded.
-; Evaluator folds call to fmodf, replacing it with constant value in case both operands
-; are known at compile time.
+; Check that we do not try to evaluate function calls with signature
+; mismatches.
 ; RUN: opt -passes=globalopt,instcombine %s -S -o - | FileCheck %s
 
-; CHECK:        @_q = dso_local local_unnamed_addr global %struct.Q { i32 1066527622 }
-; CHECK:        define dso_local i32 @main
-; CHECK-NEXT:     %[[V:.+]] = load i32, ptr @_q
-; CHECK-NEXT:     ret i32 %[[V]]
+; CHECK: @_q = dso_local global %struct.Q zeroinitializer
+; CHECK: @llvm.global_ctors
 
 source_filename = "main.cpp"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/Transforms/GlobalOpt/evaluate-ret-void-mismatch.ll b/llvm/test/Transforms/GlobalOpt/evaluate-ret-void-mismatch.ll
new file mode 100644
index 00000000000000..4cc4bf9cedbaf5
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/evaluate-ret-void-mismatch.ll
@@ -0,0 +1,26 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=globalopt < %s | FileCheck %s
+
+; Don't evaluate call with return value type mismatch.
+
+@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init, ptr null }]
+
+define void @__cxa_guard_acquire() {
+; CHECK-LABEL: define void @__cxa_guard_acquire() local_unnamed_addr {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    ret void
+;
+entry:
+  ret void
+}
+
+define void @__cxx_global_var_init() {
+; CHECK-LABEL: define void @__cxx_global_var_init() {
+; CHECK-NEXT:    [[RES:%.*]] = call i32 @__cxa_guard_acquire()
+; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[RES]], 0
+; CHECK-NEXT:    ret void
+;
+  %res = call i32 @__cxa_guard_acquire()
+  %tobool.not = icmp eq i32 %res, 0
+  ret void
+}