From 8075c39588c8e3f37587c07a5f406e973cf0c7ea Mon Sep 17 00:00:00 2001
From: mwilliams <mwilliams@fb.com>
Date: Wed, 3 Jul 2013 14:11:04 -0700
Subject: [PATCH] Fix incorrect assert

An assert was recently added that every local in the
origFunc was also a local in the genFunc, but its not necessarily
true. Also fix the usage to not corrupt the ActRec in this case.
---
 hphp/runtime/vm/jit/hhbctranslator.cpp | 25 +++++++++++--------------
 hphp/test/slow/yield/unused.php        | 14 ++++++++++++++
 hphp/test/slow/yield/unused.php.expect |  2 ++
 3 files changed, 27 insertions(+), 14 deletions(-)
 create mode 100644 hphp/test/slow/yield/unused.php
 create mode 100644 hphp/test/slow/yield/unused.php.expect

diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp
index eef9d7fed..cef0cd3dc 100644
--- a/hphp/runtime/vm/jit/hhbctranslator.cpp
+++ b/hphp/runtime/vm/jit/hhbctranslator.cpp
@@ -1005,19 +1005,14 @@ void HhbcTranslator::emitCIterFree(uint32_t iterId) {
 typedef std::map<int, int> ContParamMap;
 /*
  * mapContParams builds a mapping between named locals in origFunc and
- * corresponding named locals in genFunc. If this step succeeds and
- * there's no VarEnv at runtime, the continuation's variables can be
- * filled completely inline in the TC (assuming there aren't too
- * many).
+ * corresponding named locals in genFunc.
  */
 static
 void mapContParams(ContParamMap& map,
                    const Func* origFunc, const Func* genFunc) {
   const StringData* const* varNames = origFunc->localNames();
   for (Id i = 0; i < origFunc->numNamedLocals(); ++i) {
-    Id id = genFunc->lookupVarId(varNames[i]);
-    assert(id != kInvalidId);
-    map[i] = id;
+    map[i] = genFunc->lookupVarId(varNames[i]);
   }
 }
 
@@ -1055,13 +1050,15 @@ void HhbcTranslator::emitCreateCont(Id funNameStrId) {
   SSATmp* contAR = gen(
     LdRaw, Type::PtrToGen, cont, cns(RawMemSlot::ContARPtr));
   for (int i = 0; i < origLocals; ++i) {
-    // We must generate an AssertLoc because we don't have tracelet
-    // guards on the object type in these outer generator functions.
-    gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp());
-    // Copy the value of the local to the cont object and set the
-    // local to uninit so that we don't need to change refcounts.
-    gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i));
-    gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit());
+    if (params[i] != kInvalidId) {
+      // We must generate an AssertLoc because we don't have tracelet
+      // guards on the object type in these outer generator functions.
+      gen(AssertLoc, Type::Gen, LocalId(i), m_tb->fp());
+      // Copy the value of the local to the cont object and set the
+      // local to uninit so that we don't need to change refcounts.
+      gen(StMem, contAR, cns(-cellsToBytes(params[i] + 1)), ldLoc(i));
+      gen(StLoc, LocalId(i), m_tb->fp(), m_tb->genDefUninit());
+    }
   }
   if (fillThis) {
     assert(thisId != kInvalidId);
diff --git a/hphp/test/slow/yield/unused.php b/hphp/test/slow/yield/unused.php
new file mode 100644
index 000000000..cd188fd60
--- /dev/null
+++ b/hphp/test/slow/yield/unused.php
@@ -0,0 +1,14 @@
+<?php
+
+function test($a) {
+  $f = function() use($a) {
+    yield 1;
+    yield 2;
+  };
+
+  foreach ($f() as $v) {
+    var_dump($v);
+  }
+}
+
+test("unused");
diff --git a/hphp/test/slow/yield/unused.php.expect b/hphp/test/slow/yield/unused.php.expect
new file mode 100644
index 000000000..420ffd8a5
--- /dev/null
+++ b/hphp/test/slow/yield/unused.php.expect
@@ -0,0 +1,2 @@
+int(1)
+int(2)