Cleanup a few dynamically callable issues

Summary: - In/out wrappers should automatically inherit their dynamic callability from the function they are wrapping. - HackC should make all functions in systemlib be implicitly dynamically callable, not just native functions. - The HHAS assembler should not implicity mark things parsed during systemlib parsing as dynamically callable. Instead require the HHAS to explicitly indicate this (this prevents things like 86sinit from being marked dynamically callable). - Add a few more test cases Reviewed By: paulbiss Differential Revision: D7088310 fbshipit-source-id: 47700ca16d07093914863c059e3caa3a7f135542
facebook · Feb 28, 2018 · 02a301ef75a9850dc7e1bc7c719a736906fd6b93 · 02a301e
1 parent 20bcc3b
commit 02a301ef75a9850dc7e1bc7c719a736906fd6b93
diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp
@@ -9060,6 +9060,8 @@ determine_type_constraint(const ParameterExpressionPtr& par) {
  return determine_type_constraint_from_annot(par->annotation(), false, false);
 }

+const char* attr_DynamicallyCallable = "__DynamicallyCallable";
+
 template<class F1, class F2>
 void EmitterVisitor::emitFuncWrapper(PostponedMeth& p, FuncEmitter* fe,
                                     F1 getParam, F2 getReturn, bool isInOut) {
@@ -9072,6 +9074,13 @@ void EmitterVisitor::emitFuncWrapper(PostponedMeth& p, FuncEmitter* fe,
  if (isInOut) fe->attrs |= AttrTakesInOutParams;
  else fe->attrs = Attr(fe->attrs & ~AttrTakesInOutParams);

+  auto const& attrs = meth->getFunctionScope()->userAttributes();
+  if (!SystemLib::s_inited ||
+      meth->getFunctionScope()->isSystem() ||
+      attrs.count(attr_DynamicallyCallable)) {
+    fe->dynamicallyCallable = true;
+  }
+
  auto region = createRegion(meth, Region::Kind::FuncBody);
  enterRegion(region);
  SCOPE_EXIT { leaveRegion(region); };
@@ -9400,7 +9409,6 @@ void EmitterVisitor::bindUserAttributes(MethodStatementPtr meth,

 const StaticString s_Void("HH\\void");
 const char* attr_Deprecated = "__Deprecated";
-const char* attr_DynamicallyCallable = "__DynamicallyCallable";

 Attr EmitterVisitor::bindNativeFunc(MethodStatementPtr meth,
                                    FuncEmitter *fe) {
diff --git a/hphp/hack/src/hhbc/emit_body.ml b/hphp/hack/src/hhbc/emit_body.ml
@@ -517,7 +517,7 @@ let emit_body
    body_instrs
    decl_vars
    false (*is_memoize_wrapper*)
-    (is_dynamically_callable || is_native)
+    is_dynamically_callable
    params
    (Some return_type_info)
    svar_instrs
diff --git a/hphp/hack/src/hhbc/emit_function.ml b/hphp/hack/src/hhbc/emit_function.ml
@@ -28,7 +28,9 @@ let emit_function : A.fun_ * bool -> Hhas_function.t list =
  let is_memoize = Hhas_attribute.is_memoized function_attributes in
  let is_native = Hhas_attribute.is_native function_attributes in
  let deprecation_info = Hhas_attribute.deprecation_info function_attributes in
-  let is_dynamically_callable = Hhas_attribute.is_dynamically_callable function_attributes in
+  let is_dynamically_callable =
+    ((Hhas_attribute.is_dynamically_callable function_attributes) || Emit_env.is_systemlib ())
+  in
  let wrapper_type_opt, inout_param_locations =
    Emit_inout_helpers.extract_function_inout_or_ref_param_locations ast_fun in
  let has_inout_args = Option.is_some wrapper_type_opt in
@@ -50,7 +52,7 @@ let emit_function : A.fun_ * bool -> Hhas_function.t list =
      ~is_memoize
      ~is_native
      ~is_async:function_is_async
-      ~is_dynamically_callable
+      ~is_dynamically_callable:(is_dynamically_callable && (not is_memoize))
      ~deprecation_info:(if is_memoize then None else deprecation_info)
      ~skipawaitable:(ast_fun.Ast.f_fun_kind = Ast_defs.FAsync)
      ~is_return_by_ref
diff --git a/hphp/hack/src/hhbc/emit_inout_function.ml b/hphp/hack/src/hhbc/emit_inout_function.ml
@@ -112,7 +112,7 @@ let emit_body_instrs ~wrapper_type env params call_instrs =
  ]

 (* Construct the wrapper function body *)
-let make_wrapper_body env doc decl_vars return_type params instrs =
+let make_wrapper_body env doc decl_vars return_type is_dynamically_callable params instrs =
  let params = List.map params ~f:Hhas_param.without_type in
  let return_user_type = Hhas_type_info.user_type return_type in
  let return_tc = Hhas_type_constraint.make None [] in
@@ -121,7 +121,7 @@ let make_wrapper_body env doc decl_vars return_type params instrs =
    instrs
    decl_vars
    false (* is_memoize_wrapper *)
-    false (* is_dynamically_callable *)
+    is_dynamically_callable
    params
    (Some return_type)
    [] (* static_inits: this is intentionally empty *)
@@ -143,6 +143,9 @@ let emit_wrapper_function
    ~scope ast_fun.Ast.f_params in
  let function_attributes =
    Emit_attribute.from_asts namespace ast_fun.Ast.f_user_attributes in
+  let is_dynamically_callable =
+    ((Hhas_attribute.is_dynamically_callable function_attributes) || Emit_env.is_systemlib ())
+  in
  let scope = [Ast_scope.ScopeItem.Function ast_fun] in
  let return_type_info =
    Emit_body.emit_return_type_info
@@ -166,7 +169,9 @@ let emit_wrapper_function
  let doc = ast_fun.A.f_doc_comment in
  let body =
    make_wrapper_body
-      env doc decl_vars return_type_info modified_params body_instrs in
+      env doc decl_vars return_type_info
+      is_dynamically_callable modified_params body_instrs
+  in
  let return_by_ref = ast_fun.Ast.f_ret_by_ref in
  let is_interceptable =
    Interceptable.is_function_interceptable namespace ast_fun in
@@ -210,6 +215,9 @@ let emit_wrapper_method
    List.mem ast_method.Ast.m_kind Ast.Static in
  let method_attributes = Emit_attribute.from_asts
      (Emit_env.get_namespace env) ast_method.Ast.m_user_attributes in
+  let is_dynamically_callable =
+    ((Hhas_attribute.is_dynamically_callable method_attributes) || Emit_env.is_systemlib ())
+  in
  let method_is_private =
    List.mem ast_method.Ast.m_kind Ast.Private in
  let method_is_protected =
@@ -261,7 +269,9 @@ let emit_wrapper_method
  in
  let doc = ast_method.A.m_doc_comment in
  let body =
-    make_wrapper_body env doc decl_vars return_type_info params body_instrs in
+    make_wrapper_body env doc decl_vars return_type_info
+                      is_dynamically_callable params body_instrs
+  in
  let method_is_interceptable =
    Interceptable.is_method_interceptable namespace ast_class original_id in
  Hhas_method.make
diff --git a/hphp/hack/src/hhbc/emit_method.ml b/hphp/hack/src/hhbc/emit_method.ml
@@ -85,7 +85,9 @@ let from_ast_wrapper : bool -> _ ->
  let is_native_opcode_impl =
    Hhas_attribute.is_native_opcode_impl method_attributes in
  let deprecation_info = Hhas_attribute.deprecation_info method_attributes in
-  let is_dynamically_callable = Hhas_attribute.is_dynamically_callable method_attributes in
+  let is_dynamically_callable =
+    ((Hhas_attribute.is_dynamically_callable method_attributes) || Emit_env.is_systemlib ())
+  in
  let (pos, original_name) = ast_method.Ast.m_name in
  let (_, class_name) = ast_class.Ast.c_name in
  let class_name = SU.Xhp.mangle @@ Utils.strip_ns class_name in
@@ -205,7 +207,7 @@ let from_ast_wrapper : bool -> _ ->
        ~is_memoize
        ~is_native
        ~is_async:method_is_async
-        ~is_dynamically_callable
+        ~is_dynamically_callable:(is_dynamically_callable && (not is_memoize))
        ~deprecation_info
        ~skipawaitable:(ast_method.Ast.m_fun_kind = Ast_defs.FAsync)
        ~is_return_by_ref
diff --git a/hphp/runtime/vm/as.cpp b/hphp/runtime/vm/as.cpp
@@ -2214,7 +2214,6 @@ void parse_function(AsmState& as) {
  as.fe->init(line0, line1, as.ue->bcPos(), attrs, isTop, 0);
  std::tie(as.fe->retUserType, as.fe->retTypeConstraint) = typeInfo;
  as.fe->userAttributes = userAttrs;
-  as.fe->dynamicallyCallable = !SystemLib::s_inited;

  parse_parameter_list(as);
  parse_function_flags(as);
@@ -2252,7 +2251,6 @@ void parse_method(AsmState& as) {
              as.ue->bcPos(), attrs, false, 0);
  std::tie(as.fe->retUserType, as.fe->retTypeConstraint) = typeInfo;
  as.fe->userAttributes = userAttrs;
-  as.fe->dynamicallyCallable = !SystemLib::s_inited;

  parse_parameter_list(as);
  parse_function_flags(as);
diff --git a/hphp/system/php/array_filter.hhas b/hphp/system/php/array_filter.hhas
@@ -1,4 +1,5 @@
 .function [skip_frame] array_filter($arr = no_args, $func = start("""null"""), $flag = start("""null"""), $res = start) {
+          .dynamicallycallable;
          .numiters 2;

 # if we get here, a value was supplied for $res
diff --git a/hphp/system/php/array_map.hhas b/hphp/system/php/array_map.hhas
@@ -1,4 +1,5 @@
 .function [skip_frame] array_map($func = no_args, $arr = one_arg, $res = one_array, ...$extra) {
+          .dynamicallycallable;
          .numiters 2;

 # If we get here, a value was supplied for $res (or we did not get an
diff --git a/hphp/system/php/array_reduce.hhas b/hphp/system/php/array_reduce.hhas
@@ -1,4 +1,5 @@
 .function [skip_frame] array_reduce($input = no_args, $func = one_arg, $res = two_args, $extra = got_args) {
+          .dynamicallycallable;
          .numiters 2;

 # If we get here, a value was supplied for $extra
diff --git a/hphp/test/slow/dynamic-calls/inout.php b/hphp/test/slow/dynamic-calls/inout.php
@@ -0,0 +1,86 @@
+<?hh
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+function wrap($e) { echo "Exception: {$e->getMessage()}\n"; }
+
+function func1(inout $x) {}
+<<__DynamicallyCallable>> function func4(inout $x) {}
+
+class A {
+  public function func2(inout $x) {}
+  public static function func3(inout $x) {}
+}
+
+class B {
+  <<__DynamicallyCallable>> public function func5(inout $x) {}
+  <<__DynamicallyCallable>> public static function func6(inout $x) {}
+}
+
+function positive_tests() {
+  echo "====================== positive tests ========================\n";
+  $v = 123;
+
+  try { $x = 'func1'; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'func1'; $x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A::func2'; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A::func2'; $x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A::func3'; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A::func3'; $x(inout $v); } catch (Exception $e) { wrap($e); }
+
+  try { $x = ['A', 'func2']; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = ['A', 'func2']; $x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = ['A', 'func3']; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = ['A', 'func3']; $x(inout $v); } catch (Exception $e) { wrap($e); }
+
+  try { $x = [new A, 'func2']; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = [new A, 'func2']; $x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = [new A, 'func3']; $x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = [new A, 'func3']; $x(inout $v); } catch (Exception $e) { wrap($e); }
+
+  try { $x = 'A'; $x::func2($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A'; $x::func2(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A'; $x::func3($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'A'; $x::func3(inout $v); } catch (Exception $e) { wrap($e); }
+
+  try { $x = 'func2'; A::$x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'func2'; A::$x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'func3'; A::$x($v); } catch (Exception $e) { wrap($e); }
+  try { $x = 'func3'; A::$x(inout $v); } catch (Exception $e) { wrap($e); }
+
+  try { $obj = new A; $x = 'func2'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+  try { $obj = new A; $x = 'func2'; $obj->$x(inout $v); } catch (Exception $e) { wrap($e); }
+  try { $obj = new A; $x = 'func3'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+  try { $obj = new A; $x = 'func3'; $obj->$x(inout $v); } catch (Exception $e) { wrap($e); }
+}
+
+function negative_tests() {
+  echo "====================== negative tests ========================\n";
+  $v = 123;
+
+  $x = 'func4'; $x($v);
+  $x = 'func4'; $x(inout $v);
+  $x = 'B::func6'; $x($v);
+  $x = 'B::func6'; $x(inout $v);
+
+  $x = ['B', 'func6']; $x($v);
+  $x = ['B', 'func6']; $x(inout $v);
+
+  $x = [new B, 'func5']; $x($v);
+  $x = [new B, 'func5']; $x(inout $v);
+  $x = [new B, 'func6']; $x($v);
+  $x = [new B, 'func6']; $x(inout $v);
+
+  $x = 'B'; $x::func6($v);
+  $x = 'B'; $x::func6(inout $v);
+
+  $x = 'func6'; B::$x($v);
+  $x = 'func6'; B::$x(inout $v);
+
+  $obj = new B; $x = 'func5'; $obj->$x($v);
+  $obj = new B; $x = 'func5'; $obj->$x(inout $v);
+  $obj = new B; $x = 'func6'; $obj->$x($v);
+  $obj = new B; $x = 'func6'; $obj->$x(inout $v);
+}
+
+positive_tests();
+negative_tests();
diff --git a/hphp/test/slow/dynamic-calls/inout.php.expect b/hphp/test/slow/dynamic-calls/inout.php.expect
@@ -0,0 +1,28 @@
+====================== positive tests ========================
+Exception: 'func1' called dynamically
+Exception: 'func1' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func2' called dynamically
+Exception: 'A::func3' called dynamically
+Exception: 'A::func3' called dynamically
+====================== negative tests ========================
diff --git a/hphp/test/slow/dynamic-calls/inout.php.hphp_opts b/hphp/test/slow/dynamic-calls/inout.php.hphp_opts
@@ -0,0 +1 @@
+-vRuntime.Eval.ForbidDynamicCalls=2 -d hhvm.php7.all=0
diff --git a/hphp/test/slow/dynamic-calls/inout.php.opts b/hphp/test/slow/dynamic-calls/inout.php.opts
@@ -0,0 +1 @@
+-vEval.ForbidDynamicCalls=2
diff --git a/hphp/test/slow/dynamic-calls/memoize.php b/hphp/test/slow/dynamic-calls/memoize.php
@@ -0,0 +1,97 @@
+<?hh
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+function wrap($e) { echo "Exception: {$e->getMessage()}\n"; }
+
+<<__Memoize>> function func1($x) {}
+<<__Memoize, __DynamicallyCallable>> function func4($x) {}
+
+class A {
+  <<__Memoize>> public function func2($x) {}
+  <<__Memoize>> public static function func3($x) {}
+
+  public static function positive_tests() {
+    echo "====================== positive tests (A) ==================\n";
+    $v = 123;
+
+    try { $x = 'func1'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func1$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func4$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'A::func2'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'A::func2$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'A::func3'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'A::func3$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = ['A', 'func2']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = ['A', 'func2$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = ['A', 'func3']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = ['A', 'func3$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = [new A, 'func2']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = [new A, 'func2$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = [new A, 'func3']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = [new A, 'func3$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = 'A'; $x::func2($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'A'; $x::func3($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = 'func2'; A::$x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func2$memoize_impl'; A::$x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func3'; A::$x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func3$memoize_impl'; A::$x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $obj = new A; $x = 'func2'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+    try { $obj = new A; $x = 'func2$memoize_impl'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+    try { $obj = new A; $x = 'func3'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+    try { $obj = new A; $x = 'func3$memoize_impl'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+  }
+}
+
+class B {
+  <<__Memoize, __DynamicallyCallable>> public function func5($x) {}
+  <<__Memoize, __DynamicallyCallable>> public static function func6($x) {}
+
+  public static function positive_tests() {
+    echo "====================== positive tests (B) ==================\n";
+    $v = 123;
+
+    try { $x = 'func4$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'B::func6$memoize_impl'; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = ['B', 'func5$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = ['B', 'func6$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = [new B, 'func5$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = [new B, 'func6$memoize_impl']; $x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $x = 'func5$memoize_impl'; B::$x($v); } catch (Exception $e) { wrap($e); }
+    try { $x = 'func6$memoize_impl'; B::$x($v); } catch (Exception $e) { wrap($e); }
+
+    try { $obj = new B; $x = 'func5$memoize_impl'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+    try { $obj = new B; $x = 'func6$memoize_impl'; $obj->$x($v); } catch (Exception $e) { wrap($e); }
+  }
+
+  public static function negative_tests() {
+    echo "====================== negative tests ======================\n";
+    $v = 123;
+
+    $x = 'func4'; $x($v);
+    $x = 'B::func6'; $x($v);
+
+    $x = ['B', 'func6']; $x($v);
+
+    $x = [new B, 'func5']; $x($v);
+    $x = [new B, 'func6']; $x($v);
+
+    $x = 'B'; $x::func6($v);
+
+    $x = 'func6'; B::$x($v);
+
+    $obj = new B; $x = 'func5'; $obj->$x($v);
+    $obj = new B; $x = 'func6'; $obj->$x($v);
+  }
+}
+
+A::positive_tests();
+B::positive_tests();
+B::negative_tests();