From 00a558e262ca65d7ef4a5c5dbd990c4db4b8ee52 Mon Sep 17 00:00:00 2001
From: Bill Fumerola <bill@fb.com>
Date: Mon, 25 Mar 2019 12:16:51 -0700
Subject: [PATCH] Emit fatal when unit takes a reference to an element in
 $GLOBALS

Summary:
this behavior was announced in https://hhvm.com/blog/2019/02/11/hhvm-4.0.0.html ("Binding references to array elements", "references to superglobals")

the typechecker already has rules against both references to elements (typing check) and to superglobals (naming check)

Reviewed By: kmeht, alexeyt

Differential Revision: D14218899

fbshipit-source-id: 10628a8568ed371dc7ab869687bf24c989e66ad2
---
 hphp/hack/src/hhbc/emit_expression.ml            | 7 +++++--
 hphp/test/quick/GByRef.php                       | 6 ++++--
 hphp/test/slow/rx/body/ref-non-local.php         | 5 ++++-
 hphp/test/slow/rx/body/ref-non-local.php.expectf | 6 ------
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/hphp/hack/src/hhbc/emit_expression.ml b/hphp/hack/src/hhbc/emit_expression.ml
index de9f7fc7965..cd04423fed4 100644
--- a/hphp/hack/src/hhbc/emit_expression.ml
+++ b/hphp/hack/src/hhbc/emit_expression.ml
@@ -1674,10 +1674,13 @@ and emit_expr env ~need_ref (pos, expr_ as expr) =
       emit_conditional_expression env pos etest etrue efalse
   | A.Expr_list es -> gather @@ List.map es ~f:(emit_expr ~need_ref:false env)
   | A.Array_get((_, A.Lvar (_, x)), Some e) when x = SN.Superglobals.globals ->
-    gather [
+    if need_ref then
+      Emit_fatal.raise_fatal_runtime pos
+      "$GLOBALS elements may not be taken by reference"
+    else gather [
       emit_expr ~need_ref:false env e;
       emit_pos pos;
-      instr (IGet (if need_ref then VGetG else CGetG))
+      instr (IGet CGetG)
     ]
   | A.Array_get(base_expr, opt_elem_expr) ->
     let query_op = if need_ref then QueryOp.Empty else QueryOp.CGet in
diff --git a/hphp/test/quick/GByRef.php b/hphp/test/quick/GByRef.php
index a4e532409f9..d6b56355fc4 100644
--- a/hphp/test/quick/GByRef.php
+++ b/hphp/test/quick/GByRef.php
@@ -19,10 +19,12 @@ function insideByRef($a) {
   $a['baz'] = &$zz;
 }
 
+
+$shadow = $GLOBALS;
 var_dump($GLOBALS['foo']);
 var_dump($GLOBALS['bar']);
-byRef(&$GLOBALS['foo']['bar']);
-byRef(&$GLOBALS['bar']);
+byRef(&$shadow['foo']['bar']);
+byRef(&$shadow['bar']);
 var_dump($GLOBALS['foo']);
 var_dump($GLOBALS['bar']);
 
diff --git a/hphp/test/slow/rx/body/ref-non-local.php b/hphp/test/slow/rx/body/ref-non-local.php
index 47e8f6cf052..a75deb3080d 100644
--- a/hphp/test/slow/rx/body/ref-non-local.php
+++ b/hphp/test/slow/rx/body/ref-non-local.php
@@ -8,7 +8,10 @@ function test() {
 
   $y = 42;
   $_GET =& $y;           // VGetL, BindG (superglobals)
-  $b = &$GLOBALS['foo']; // VGetG (globals array)
+
+  // the following VGetG is covered by
+  // $GLOBALS elements may not be taken by reference
+  // $b = &$GLOBALS['foo']; // VGetG (globals array)
 
   // the following VGetG are covered by
   // Fatal error: Superglobals may not be taken by reference
diff --git a/hphp/test/slow/rx/body/ref-non-local.php.expectf b/hphp/test/slow/rx/body/ref-non-local.php.expectf
index b135e1f37b9..8f61f726765 100644
--- a/hphp/test/slow/rx/body/ref-non-local.php.expectf
+++ b/hphp/test/slow/rx/body/ref-non-local.php.expectf
@@ -10,12 +10,6 @@ Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): refe
 
 Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: PopV
 
-Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: VGetG
-
-Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: BindL
-
-Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: PopV
-
 Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: VGetS
 
 Verification Error (unit %s/test/slow/rx/body/ref-non-local.php func test): references are forbidden in Rx functions: BindS
-- 
2.11.4.GIT