Permalink
Browse files

Make reading from array with [] an error in most contexts

Summary:
$y = $x[] is banned. This adds a runtime error for its usage.

In my tests, $x[] is allowed only in a few contexts:
- In the context of a reference, i.e. &$x[]
- As an argument to a function
- As an lvalue

Reviewed By: oulgen

Differential Revision: D7058363

fbshipit-source-id: ab959b2d66ddb682874847ffa1ff7abcc33f3431
  • Loading branch information...
jamesjwu authored and hhvm-bot committed Mar 1, 2018
1 parent d2b881d commit a434618f11dee01c7952aac743c581f0457a02db
Showing with 38 additions and 20 deletions.
  1. +12 −7 hphp/hack/src/hhbc/emit_env.ml
  2. +26 −7 hphp/hack/src/hhbc/emit_expression.ml
  3. +0 −6 hphp/test/hhcodegen_failing_tests_zend
@@ -9,12 +9,13 @@
*)
type t = {
env_pipe_var : Local.t option;
env_scope : Ast_scope.Scope.t;
env_namespace : Namespace_env.env;
env_needs_local_this: bool;
env_jump_targets : Jump_targets.t;
env_in_try : bool
env_pipe_var : Local.t option;
env_scope : Ast_scope.Scope.t;
env_namespace : Namespace_env.env;
env_needs_local_this : bool;
env_jump_targets : Jump_targets.t;
env_in_try : bool;
env_allows_array_append : bool;
}
type global_state =
@@ -75,6 +76,7 @@ let empty = {
env_needs_local_this = false;
env_jump_targets = Jump_targets.empty;
env_in_try = false;
env_allows_array_append = false;
}
let get_pipe_var env = env.env_pipe_var
@@ -83,6 +85,7 @@ let get_namespace env = env.env_namespace
let get_needs_local_this env = env.env_needs_local_this
let get_jump_targets env = env.env_jump_targets
let is_in_try env = env.env_in_try
let does_env_allow_array_append env = env.env_allows_array_append
(* Environment is second parameter so we can chain these e.g.
* empty |> with_scope scope |> with_namespace ns
@@ -98,7 +101,9 @@ let with_pipe_var v env =
let make_class_env ast_class =
{ env_pipe_var = None; env_scope = [Ast_scope.ScopeItem.Class ast_class];
env_namespace = ast_class.Ast.c_namespace; env_needs_local_this = false;
env_jump_targets = Jump_targets.empty; env_in_try = false; }
env_jump_targets = Jump_targets.empty; env_in_try = false;
env_allows_array_append = false;
}
let with_try env = { env with env_in_try = true }
let do_in_loop_body break_label continue_label ?iter env s f =
@@ -2500,6 +2500,8 @@ and emit_array_get_worker ?(no_final=false) ?mode
match base_expr, opt_elem_expr with
| (pos, A.Array _), None ->
Emit_fatal.raise_fatal_parse pos "Can't use array() as base in write context"
| (pos, _), None when not (Emit_env.does_env_allow_array_append env)->
Emit_fatal.raise_fatal_runtime pos "Can't use [] for reading"
| _ ->
let local_temp_kind =
get_local_temp_kind inout_param_info env opt_elem_expr in
@@ -2877,10 +2879,13 @@ and emit_base_worker ~is_object ~notice ~inout_param_info env mode base_offset
| Some i -> FPassBaseGC (i, base_offset)
)))
1
(* $a[] can not be used as the base of an array get unless as an lval *)
| A.Array_get(_, None) when not (Emit_env.does_env_allow_array_append env) ->
Emit_fatal.raise_fatal_runtime pos "Can't use [] for reading"
(* base is in turn array_get - do a specific handling for inout params
if necessary *)
| A.Array_get(base_expr, opt_elem_expr) ->
let local_temp_kind =
get_local_temp_kind inout_param_info env opt_elem_expr in
let elem_expr_instrs, elem_stack_size =
@@ -3211,7 +3216,11 @@ and emit_args_and_call env call_pos args uargs =
]
end
end
else next @@ emit_array_get ~need_ref:false env (Some (i, hint))
else
next @@ emit_array_get
~need_ref:false
{ env with Emit_env.env_allows_array_append = true }
(Some (i, hint))
QueryOp.CGet base_expr opt_elem_expr
| A.Obj_get (e1, e2, nullflavor) ->
@@ -3904,6 +3913,14 @@ and emit_lval_op_nonlist env pos op e rhs_instrs rhs_stack_size =
]
and emit_lval_op_nonlist_steps env op (pos, expr_) rhs_instrs rhs_stack_size =
let env =
match op with
(* Unbelieveably, $test[] += 5; is legal in PHP, but $test[] = $test[] + 5 is not *)
| LValOp.SetRef
| LValOp.Set
| LValOp.SetOp _
| LValOp.IncDec _ -> { env with Emit_env.env_allows_array_append = true }
| _ -> env in
let handle_dollar e final_op =
match e with
_, A.Lvar id ->
@@ -3920,6 +3937,7 @@ and emit_lval_op_nonlist_steps env op (pos, expr_) rhs_instrs rhs_stack_size =
final_op op
]
| _ ->
let instrs = emit_expr ~need_ref:false env e in
instrs,
rhs_instrs,
@@ -3965,7 +3983,8 @@ and emit_lval_op_nonlist_steps env op (pos, expr_) rhs_instrs rhs_stack_size =
index_instrs,
rhs_instrs,
final_global_op_instrs
| A.Array_get (_, None) when not (Emit_env.does_env_allow_array_append env) ->
Emit_fatal.raise_fatal_runtime pos "Can't use [] for reading"
| A.Array_get (base_expr, opt_elem_expr) ->
let mode =
match op with
@@ -3978,9 +3997,8 @@ and emit_lval_op_nonlist_steps env op (pos, expr_) rhs_instrs rhs_stack_size =
base_expr_instrs_end,
base_setup_instrs,
base_stack_size =
emit_base
~notice:Notice ~is_object:false
env mode base_offset None base_expr
emit_base ~notice:Notice ~is_object:false env
mode base_offset None base_expr
in
let mk = get_elem_member_key env rhs_stack_size opt_elem_expr in
let total_stack_size = elem_stack_size + base_stack_size in
@@ -4079,7 +4097,8 @@ and from_unop op =
| A.Uincr | A.Udecr | A.Upincr | A.Updecr | A.Uref | A.Usilence ->
emit_nyi "unop - probably does not need translation"
and emit_expr_as_ref env e = emit_expr ~need_ref:true env e
and emit_expr_as_ref env e =
emit_expr ~need_ref:true { env with Emit_env.env_allows_array_append = true} e
and emit_unop ~need_ref env pos op e =
let unop_instr = Emit_pos.emit_pos_then pos @@ from_unop op in
@@ -1,8 +1,5 @@
zend/good/Zend/tests/argument_restriction_004.php
zend/good/Zend/tests/bug32296.php
zend/good/Zend/tests/bug41351.php
zend/good/Zend/tests/bug41351_2.php
zend/good/Zend/tests/bug41351_3.php
zend/good/Zend/tests/bug43332_1.php
zend/good/Zend/tests/bug43343.php
zend/good/Zend/tests/bug55086.php
@@ -11,9 +8,6 @@ zend/good/Zend/tests/class_name_as_scalar_error_001.php
zend/good/Zend/tests/class_name_as_scalar_error_002.php
zend/good/Zend/tests/closure_005.php
zend/good/Zend/tests/closure_014.php
zend/good/Zend/tests/errmsg_006.php
zend/good/Zend/tests/errmsg_007.php
zend/good/Zend/tests/errmsg_008.php
zend/good/Zend/tests/errmsg_012.php
zend/good/Zend/tests/errmsg_015.php
zend/good/Zend/tests/errmsg_019.php

0 comments on commit a434618

Please sign in to comment.