Don't check ref annotations for calls within the runtime

Summary: These were triggering notices for callbacks inside the runtime which dispatch via `prepareArrayArgs`. Reviewed By: markw65 Differential Revision: D6445255 fbshipit-source-id: 215a385842ce632e2a3974e1b6122ece64e7dc4e
facebook · Dec 5, 2017 · 6b4e820 · 6b4e820
1 parent 98d991e
commit 6b4e820
Show file tree

Hide file tree

Showing 10 changed files with 204 additions and 16 deletions.
diff --git a/hphp/runtime/base/builtin-functions.cpp b/hphp/runtime/base/builtin-functions.cpp
@@ -443,7 +443,8 @@ vm_decode_function(const Variant& function,
 }
 
 Variant vm_call_user_func(const Variant& function, const Variant& params,
-                          bool forwarding /* = false */) {
+                          bool forwarding /* = false */,
+                          bool checkRef /* = false */) {
   ObjectData* obj = nullptr;
   Class* cls = nullptr;
   CallerFrame cf;
@@ -457,7 +458,7 @@ Variant vm_call_user_func(const Variant& function, const Variant& params,
   auto ret = Variant::attach(
     g_context->invokeFunc(f, params, obj, cls,
                           nullptr, invName, ExecutionContext::InvokeCuf,
-                          false, dynamic)
+                          false, dynamic, checkRef)
   );
   if (UNLIKELY(ret.getRawType()) == KindOfRef) {
     tvUnbox(*ret.asTypedValue());

diff --git a/hphp/runtime/base/builtin-functions.h b/hphp/runtime/base/builtin-functions.h
@@ -126,7 +126,7 @@ vm_decode_function(const Variant& function,
 }
 
 Variant vm_call_user_func(const Variant& function, const Variant& params,
-                          bool forwarding = false);
+                          bool forwarding = false, bool checkRef = false);
 
 Variant invoke_static_method(const String& s, const String& method,
                              const Variant& params, bool fatal = true);

diff --git a/hphp/runtime/base/execution-context.cpp b/hphp/runtime/base/execution-context.cpp
@@ -1636,7 +1636,8 @@ TypedValue ExecutionContext::invokeFunc(const Func* f,
                                         StringData* invName /* = NULL */,
                                         InvokeFlags flags /* = InvokeNormal */,
                                         bool useWeakTypes /* = false */,
-                                        bool dynamic /* = true */) {
+                                        bool dynamic /* = true */,
+                                        bool checkRefAnnot /* = false */) {
   const auto& args = *args_.asCell();
   assert(isContainerOrNull(args));
 
@@ -1683,7 +1684,8 @@ TypedValue ExecutionContext::invokeFunc(const Func* f,
         ? make_tv<KindOfArray>(staticEmptyArray())
         : args;
       auto prepResult = prepareArrayArgs(ar, prepArgs, vmStack(), 0,
-                                         flags & InvokeCuf, &retval);
+                                         flags & InvokeCuf, &retval,
+                                         checkRefAnnot);
       if (UNLIKELY(!prepResult)) {
         assert(KindOfNull == retval.m_type);
         return true;

diff --git a/hphp/runtime/base/execution-context.h b/hphp/runtime/base/execution-context.h
@@ -449,7 +449,8 @@ struct ExecutionContext {
                         StringData* invName = nullptr,
                         InvokeFlags flags = InvokeNormal,
                         bool useWeakTypes = false,
-                        bool dynamic = true);
+                        bool dynamic = true,
+                        bool checkRefAnnot = false);
 
   TypedValue invokeFunc(const CallCtx& ctx,
                         const Variant& args_,

diff --git a/hphp/runtime/ext/std/ext_std_function.cpp b/hphp/runtime/ext/std/ext_std_function.cpp
@@ -59,12 +59,14 @@ bool HHVM_FUNCTION(is_callable, const Variant& v, bool syntax /* = false */,
 
 Variant HHVM_FUNCTION(call_user_func, const Variant& function,
                       const Array& params /* = null_array */) {
-  return vm_call_user_func(function, params);
+  return vm_call_user_func(function, params, /* forward */ false,
+                           /* check ref */ true);
 }
 
 Variant HHVM_FUNCTION(call_user_func_array, const Variant& function,
                       const Array& params) {
-  return vm_call_user_func(function, params);
+  return vm_call_user_func(function, params, /* forward */ false,
+                           /* check ref */ true);
 }
 
 Variant HHVM_FUNCTION(check_user_func_async, const Variant& /*handles*/,
@@ -89,7 +91,7 @@ Variant HHVM_FUNCTION(forward_static_call, const Variant& function,
                               const Array& params /* = null_array */) {
   // Setting the bound parameter to true tells vm_call_user_func()
   // propogate the current late bound class
-  return vm_call_user_func(function, params, true);
+  return vm_call_user_func(function, params, true, /* check ref */ true);
 }
 
 String HHVM_FUNCTION(create_function, const String& args, const String& code) {

diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp
@@ -1277,7 +1277,7 @@ static void raiseFPassHintWarning(const StringData* fname, uint32_t id,
 // params precede the unpack.
 bool prepareArrayArgs(ActRec* ar, const Cell args, Stack& stack,
                       int nregular, bool doCufRefParamChecks,
-                      TypedValue* retval) {
+                      TypedValue* retval, bool checkRefAnnot) {
   assert(!cellIsNull(&args));
   assert(nregular >= 0);
   assert((stack.top() + nregular) == (void*) ar);
@@ -1314,10 +1314,16 @@ bool prepareArrayArgs(ActRec* ar, const Cell args, Stack& stack,
         cellDup(tvToCell(from), *to);
       } else if (LIKELY(from.m_type == KindOfRef &&
                         from.m_data.pref->hasMultipleRefs())) {
-        WRAP(raiseFPassHintWarning(f->fullDisplayName(), i, FPassHint::Cell));
+          if (checkRefAnnot) {
+            WRAP(
+              raiseFPassHintWarning(f->fullDisplayName(), i, FPassHint::Cell)
+            );
+          }
         refDup(from, *to);
       } else {
-        WRAP(raiseFPassHintWarning(f->fullDisplayName(), i, FPassHint::Cell));
+        if (checkRefAnnot) {
+          WRAP(raiseFPassHintWarning(f->fullDisplayName(), i, FPassHint::Cell));
+        }
         if (doCufRefParamChecks && f->mustBeRef(i)) {
           WRAP(raise_warning("Parameter %d to %s() expected to be a reference, "
                              "value given", i + 1, f->fullName()->data()));
@@ -5279,7 +5285,8 @@ static bool doFCallArray(PC& pc, int numStackValues,
     }
 
     auto prepResult = prepareArrayArgs(ar, args, vmStack(), numStackValues,
-                                       /* ref param checks */ true, nullptr);
+                                       /* ref param checks */ true, nullptr,
+                                       /* check ref annot */ true);
     if (UNLIKELY(!prepResult)) {
       vmStack().pushNull(); // return value is null if args are invalid
       return false;

diff --git a/hphp/runtime/vm/bytecode.h b/hphp/runtime/vm/bytecode.h
@@ -927,7 +927,7 @@ void enterVMAtFunc(ActRec* enterFnAr, StackArgsState stk, VarEnv* varEnv);
 void enterVMAtCurPC();
 bool prepareArrayArgs(ActRec* ar, const Cell args, Stack& stack,
                       int nregular, bool doCufRefParamChecks,
-                      TypedValue* retval);
+                      TypedValue* retval, bool checkRefAnnot);
 
 ///////////////////////////////////////////////////////////////////////////////
 

diff --git a/hphp/test/slow/ref-annotate.php b/hphp/test/slow/ref-annotate.php
@@ -79,8 +79,24 @@ function main($foo_str, $cuf, $cufa) {
   $cufa($foo_str, &$y);
   echo '$b[$c][$d] = $b['.$c.']['.$d.'] = '.$b[$c][$d]."\n";
 
+  echo "fb_intercept:\n";
+  if (!ini_get('hhvm.repo.authoritative')) intercept();
+
   echo "Fatal call:\n";
   foo('x', 'y', array());
 }
 
 main('foo', 'call_user_func', 'call_user_func_array');
+
+function foo2($x, $y, $z, $t, $q) {
+  var_dump("foo", $x);
+}
+
+function bar($name, $obj, $params, $data, &$done) {
+  var_dump("bar", func_get_args());
+}
+
+function intercept() {
+  fb_intercept('foo2', 'bar', null);
+  foo2(1, 2, 3, 4, 5);
+}
diff --git a/hphp/test/slow/ref-annotate.php.expectf b/hphp/test/slow/ref-annotate.php.expectf
@@ -126,8 +126,33 @@ Warning: call_user_func_array() expects parameter 2 by value, but the call was a
 
 Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 79
 $b[$c][$d] = $b[q][r] = 26
+fb_intercept:
+string(3) "bar"
+array(5) {
+  [0]=>
+  string(4) "foo2"
+  [1]=>
+  NULL
+  [2]=>
+  array(5) {
+    [0]=>
+    int(1)
+    [1]=>
+    int(2)
+    [2]=>
+    int(3)
+    [3]=>
+    int(4)
+    [4]=>
+    int(5)
+  }
+  [3]=>
+  NULL
+  [4]=>
+  bool(true)
+}
 Fatal call:
 
-Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 83
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 86
 
-Fatal error: Cannot pass parameter 3 by reference in %s/ref-annotate.php on line 83
+Fatal error: Cannot pass parameter 3 by reference in %s/ref-annotate.php on line 86
diff --git a/hphp/test/slow/ref-annotate.php.expectf-repo b/hphp/test/slow/ref-annotate.php.expectf-repo
@@ -0,0 +1,134 @@
+Builtin calls:
+
+Warning: ksort() expects parameter 1 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 12
+array(3) {
+  [0]=>
+  int(1)
+  [1]=>
+  int(4)
+  [2]=>
+  int(12)
+}
+Literal calls:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 18
+
+Warning: var_dump() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 20
+array(2) {
+  ["x"]=>
+  array(1) {
+    ["y"]=>
+    int(0)
+  }
+  ["a"]=>
+  array(1) {
+    ["b"]=>
+    int(1)
+  }
+}
+Plain calls:
+
+Warning: foo() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 26
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 26
+
+Warning: foo() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 27
+
+Warning: foo() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 28
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 28
+
+Warning: foo() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 29
+
+Warning: foo() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 30
+
+Warning: foo() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 30
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 30
+
+Warning: foo() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 31
+
+Warning: foo() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 31
+$b[$c][$d] = $b[q][r] = 7
+Dynamic calls:
+
+Warning: foo() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 35
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 35
+
+Warning: foo() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 36
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 36
+$b[$c][$d] = $b[q][r] = 10
+CUF/dynamic-CUF calls:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 41
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 42
+
+Warning: Parameter 3 to foo() expected to be a reference, value given in %s/ref-annotate.php on line 42
+$b[$c][$d] = $b[q][r] = 10
+CUF + ref calls:
+
+Warning: call_user_func() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 46
+
+Warning: call_user_func() expects parameter 4 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 46
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 46
+
+Warning: call_user_func() expects parameter 3 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 47
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 47
+$b[$c][$d] = $b[q][r] = 10
+dynamic-CUF + ref calls:
+
+Warning: call_user_func() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 52
+
+Warning: call_user_func() expects parameter 4 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 52
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 52
+
+Warning: call_user_func() expects parameter 3 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 53
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 53
+$b[$c][$d] = $b[q][r] = 10
+CUFA calls:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 61
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 62
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 63
+$b[$c][$d] = $b[q][r] = 19
+CUFA + ref calls:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 67
+
+Warning: call_user_func_array() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 68
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 68
+$b[$c][$d] = $b[q][r] = 21
+dynamic-CUFA calls:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 72
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 73
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 74
+$b[$c][$d] = $b[q][r] = 24
+dynamic-CUFA + ref calls:
+
+Warning: call_user_func_array() expects parameter 1 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 78
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 78
+
+Warning: call_user_func_array() expects parameter 2 by value, but the call was annotated with '&' in %s/ref-annotate.php on line 79
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 79
+$b[$c][$d] = $b[q][r] = 26
+fb_intercept:
+Fatal call:
+
+Warning: foo() expects parameter 3 by reference, but the call was not annotated with '&' in %s/ref-annotate.php on line 86
+
+Fatal error: Cannot pass parameter 3 by reference in %s/ref-annotate.php on line 86