From 4d923bf6797b52563a8e142808e3ffc81b87c97e Mon Sep 17 00:00:00 2001
From: Drew Paroski <andrewparoski@fb.com>
Date: Fri, 17 May 2013 21:01:57 -0700
Subject: [PATCH] Fix foreach with superglobals

HHVM was segfaulting on "foreach ($x as $_FILES) {}" during bytecode
emission. This diff fixes the issue.

Github issue 734: https://github.com/facebook/hiphop-php/issues/734
---
 hphp/compiler/analysis/alias_manager.cpp        | 13 ++++++++-----
 hphp/compiler/analysis/emitter.cpp              | 17 +++++++++++------
 hphp/test/quick/foreach-superglobals.php        |  8 ++++++++
 hphp/test/quick/foreach-superglobals.php.expect |  6 ++++++
 4 files changed, 33 insertions(+), 11 deletions(-)
 create mode 100644 hphp/test/quick/foreach-superglobals.php
 create mode 100644 hphp/test/quick/foreach-superglobals.php.expect

diff --git a/hphp/compiler/analysis/alias_manager.cpp b/hphp/compiler/analysis/alias_manager.cpp
index 776b0741b..1404007ea 100644
--- a/hphp/compiler/analysis/alias_manager.cpp
+++ b/hphp/compiler/analysis/alias_manager.cpp
@@ -14,6 +14,8 @@
    +----------------------------------------------------------------------+
 */
 
+#include "hphp/compiler/analysis/alias_manager.h"
+
 #include "hphp/compiler/analysis/analysis_result.h"
 #include "hphp/compiler/analysis/function_scope.h"
 #include "hphp/compiler/expression/expression.h"
@@ -54,7 +56,6 @@
 #include "hphp/compiler/statement/try_statement.h"
 #include "hphp/compiler/statement/global_statement.h"
 #include "hphp/compiler/statement/static_statement.h"
-#include "hphp/compiler/analysis/alias_manager.h"
 #include "hphp/compiler/analysis/control_flow.h"
 #include "hphp/compiler/analysis/variable_table.h"
 #include "hphp/compiler/analysis/data_flow.h"
@@ -2205,13 +2206,15 @@ int AliasManager::collectAliasInfoRecur(ConstructPtr cs, bool unused) {
         ForEachStatementPtr fs(static_pointer_cast<ForEachStatement>(s));
         SimpleVariablePtr name = dpc(SimpleVariable, fs->getNameExp());
         if (name) {
-          Symbol *sym = name->getSymbol();
-          sym->setNeeded();
+          if (Symbol *sym = name->getSymbol()) {
+            sym->setNeeded();
+          }
         }
         SimpleVariablePtr value = dpc(SimpleVariable, fs->getValueExp());
         if (value) {
-          Symbol *sym = value->getSymbol();
-          sym->setNeeded();
+          if (Symbol *sym = value->getSymbol()) {
+            sym->setNeeded();
+          }
         }
         break;
       }
diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp
index f3544a8ce..2a52caa09 100644
--- a/hphp/compiler/analysis/emitter.cpp
+++ b/hphp/compiler/analysis/emitter.cpp
@@ -5731,6 +5731,13 @@ void EmitterVisitor::emitVirtualLocal(int localId,
   }
 }
 
+static bool isNormalLocalVariable(const ExpressionPtr& expr) {
+  SimpleVariable* sv = static_cast<SimpleVariable*>(expr.get());
+  return (expr->is(Expression::KindOfSimpleVariable) &&
+          !sv->isSuperGlobal() &&
+          !sv->isThis());
+}
+
 template<class Expr>
 void EmitterVisitor::emitVirtualClassBase(Emitter& e, Expr* node) {
   prepareEvalStack();
@@ -5741,10 +5748,8 @@ void EmitterVisitor::emitVirtualClassBase(Emitter& e, Expr* node) {
     m_evalStack.setClsBaseType(SymbolicStack::CLS_LATE_BOUND);
   } else if (node->getClass()) {
     const ExpressionPtr& expr = node->getClass();
-    SimpleVariable* sv = static_cast<SimpleVariable*>(expr.get());
-    if (expr->is(Expression::KindOfSimpleVariable) &&
-        !sv->isSuperGlobal() &&
-        !sv->isThis()) {
+    if (isNormalLocalVariable(expr)) {
+      SimpleVariable* sv = static_cast<SimpleVariable*>(expr.get());
       StringData* name = StringData::GetStaticString(sv->getName());
       Id locId = m_curFunc->lookupVarId(name);
       m_evalStack.setClsBaseType(SymbolicStack::CLS_NAMED_LOCAL);
@@ -6475,8 +6480,8 @@ void EmitterVisitor::emitForeach(Emitter& e, ForEachStatementPtr fe) {
   Offset bIterStart;
   Id itId = m_curFunc->allocIterator();
   ForeachIterGuard fig(*this, itId, strong);
-  bool simpleCase = (!key || key->is(Expression::KindOfSimpleVariable)) &&
-                             val->is(Expression::KindOfSimpleVariable);
+  bool simpleCase = (!key || isNormalLocalVariable(key)) &&
+                    isNormalLocalVariable(val);
 
   if (simpleCase) {
     SimpleVariablePtr svVal(static_pointer_cast<SimpleVariable>(val));
diff --git a/hphp/test/quick/foreach-superglobals.php b/hphp/test/quick/foreach-superglobals.php
new file mode 100644
index 000000000..d944917ca
--- /dev/null
+++ b/hphp/test/quick/foreach-superglobals.php
@@ -0,0 +1,8 @@
+<?php
+function foo() {
+  $arr = array(12,34,56);
+  foreach ($arr as $_ENV => $_FILES) {
+    var_dump($_ENV, $_FILES);
+  }
+}
+foo();
diff --git a/hphp/test/quick/foreach-superglobals.php.expect b/hphp/test/quick/foreach-superglobals.php.expect
new file mode 100644
index 000000000..3bdd27182
--- /dev/null
+++ b/hphp/test/quick/foreach-superglobals.php.expect
@@ -0,0 +1,6 @@
+int(0)
+int(12)
+int(1)
+int(34)
+int(2)
+int(56)