[llvm] r352469 - [IPCP] Don't crash due to arg count/type mismatch between caller/callee

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 29 02:19:45 PST 2019


Author: bjope
Date: Tue Jan 29 02:19:44 2019
New Revision: 352469

URL: http://llvm.org/viewvc/llvm-project?rev=352469&view=rev
Log:
[IPCP] Don't crash due to arg count/type mismatch between caller/callee

Summary:
This patch avoids an assert in IPConstantPropagation when
there is a argument count/type mismatch between the caller and
the callee.

While this is actually UB on C-level (clang emits a warning),
the IR verifier seems to accept it. I'm not sure what other
frontends/languages might think about this, so simply bailing out
to avoid hitting an assert (in CallSiteBase<>::getArgOperand or
Value::doRAUW) seems like a simple solution.

The problem is exposed by the fact that AbstractCallSites will look
through a bitcast at the callee position of a call/invoke.

Reviewers: jdoerfert, reames, efriedma

Reviewed By: jdoerfert, efriedma

Subscribers: eli.friedman, efriedma, llvm-commits

Differential Revision: https://reviews.llvm.org/D57052

Added:
    llvm/trunk/test/Transforms/IPConstantProp/arg-count-mismatch.ll
    llvm/trunk/test/Transforms/IPConstantProp/arg-type-mismatch.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp

Modified: llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp?rev=352469&r1=352468&r2=352469&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp Tue Jan 29 02:19:44 2019
@@ -66,6 +66,13 @@ static bool PropagateConstantsIntoArgume
     if (!ACS)
       return false;
 
+    // Mismatched argument count is undefined behavior. Simply bail out to avoid
+    // handling of such situations below (avoiding asserts/crashes).
+    unsigned NumActualArgs = ACS.getNumArgOperands();
+    if (F.isVarArg() ? ArgumentConstants.size() > NumActualArgs
+                     : ArgumentConstants.size() != NumActualArgs)
+      return false;
+
     // Check out all of the potentially constant arguments.  Note that we don't
     // inspect varargs here.
     Function::arg_iterator Arg = F.arg_begin();
@@ -78,6 +85,11 @@ static bool PropagateConstantsIntoArgume
       Value *V = ACS.getCallArgOperand(i);
       Constant *C = dyn_cast_or_null<Constant>(V);
 
+      // Mismatched argument type is undefined behavior. Simply bail out to avoid
+      // handling of such situations below (avoiding asserts/crashes).
+      if (C && Arg->getType() != C->getType())
+        return false;
+
       // We can only propagate thread independent values through callbacks.
       // This is different to direct/indirect call sites because for them we
       // know the thread executing the caller and callee is the same. For

Added: llvm/trunk/test/Transforms/IPConstantProp/arg-count-mismatch.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IPConstantProp/arg-count-mismatch.ll?rev=352469&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IPConstantProp/arg-count-mismatch.ll (added)
+++ llvm/trunk/test/Transforms/IPConstantProp/arg-count-mismatch.ll Tue Jan 29 02:19:44 2019
@@ -0,0 +1,72 @@
+; RUN: opt < %s -ipconstprop -S -o - | FileCheck %s
+
+; The original C source looked like this:
+;
+;   long long a101, b101, e101;
+;   volatile long c101;
+;   int d101;
+;
+;   static inline int bar(p1, p2)
+;   {
+;       return 0;
+;   }
+;
+;   void foo(unsigned p1)
+;   {
+;       long long *f = &b101, *g = &e101;
+;       c101 = 0;
+;       (void)((*f |= a101) - (*g = bar(d101)));
+;       c101 = (*f |= a101 &= p1) == d101;
+;   }
+;
+; When compiled with Clang it gives a warning
+;   warning: too few arguments in call to 'bar'
+;
+; This ll reproducer has been reduced to only include tha call.
+;
+; Note that -lint will report this as UB, but it passes -verify.
+
+; This test is just to verify that we do not crash/assert due to mismatch in
+; argument count between the caller and callee.
+
+define dso_local void @foo(i16 %a) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[CALL:%.*]] = call i16 bitcast (i16 (i16, i16)* @bar to i16 (i16)*)(i16 [[A:%.*]])
+; CHECK-NEXT:    ret void
+;
+  %call = call i16 bitcast (i16 (i16, i16) * @bar to i16 (i16) *)(i16 %a)
+  ret void
+}
+
+define internal i16 @bar(i16 %p1, i16 %p2) {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT:    ret i16 0
+;
+  ret i16 0
+}
+
+;-------------------------------------------------------------------------------
+; Additional tests to verify that we still optimize when having a mismatch
+; in argument count due to varargs (as long as all non-variadic arguments have
+; been provided),
+
+define dso_local void @vararg_tests(i16 %a) {
+  %call1 = call i16 (i16, ...) @vararg_prop(i16 7, i16 8, i16 %a)
+  %call2 = call i16 bitcast (i16 (i16, i16, ...) * @vararg_no_prop to i16 (i16) *) (i16 7)
+  ret void
+}
+
+define internal i16 @vararg_prop(i16 %p1, ...) {
+; CHECK-LABEL: define internal i16 @vararg_prop(
+; CHECK-NEXT:    ret i16 7
+;
+  ret i16 %p1
+}
+
+define internal i16 @vararg_no_prop(i16 %p1, i16 %p2, ...) {
+; CHECK-LABEL: define internal i16 @vararg_no_prop(
+; CHECK-NEXT:    ret i16 [[P1:%.*]]
+;
+  ret i16 %p1
+}
+

Added: llvm/trunk/test/Transforms/IPConstantProp/arg-type-mismatch.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IPConstantProp/arg-type-mismatch.ll?rev=352469&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IPConstantProp/arg-type-mismatch.ll (added)
+++ llvm/trunk/test/Transforms/IPConstantProp/arg-type-mismatch.ll Tue Jan 29 02:19:44 2019
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -ipconstprop -S -o - | FileCheck %s
+
+; This test is just to verify that we do not crash/assert due to mismatch in
+; argument type between the caller and callee.
+
+define dso_local void @foo(i16 %a) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[CALL:%.*]] = call i16 bitcast (i16 (i16, i16)* @bar to i16 (i16, i32)*)(i16 [[A:%.*]], i32 7)
+; CHECK-NEXT:    ret void
+;
+  %call = call i16 bitcast (i16 (i16, i16) * @bar to i16 (i16, i32) *)(i16 %a, i32 7)
+  ret void
+}
+
+define internal i16 @bar(i16 %p1, i16 %p2) {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT:    ret i16 [[P2:%.*]]
+;
+  ret i16 %p2
+}
+
+




More information about the llvm-commits mailing list