[Fix] serialization ref problem with array

Summary:
When we serialize array with ref, we still check whether the ArrayData
already exist in pointer map. If the same array happens to be serialized
by value earlier, it will be treated as a ref to that array. This
behavior is incorrect and can also manifest as an off-by-1 problem in
unserialization when counting the sequence number.

Test Plan:
make fast_tests

The newly added test case become:
a:3:{i:0;a:1:{i:0;i:123;}i:1;a:1:{i:0;i:123;}i:2;R:4;}
Without this diff:
a:3:{i:0;a:1:{i:0;i:123;}i:1;R:2;i:2;R:4;}

also tried dumping a huge stack in hphpd and verify the previous failed
case worked

Reviewers: mwilliams, myang

Reviewed By: myang

CC: gregschechte, andrewparoski, ps, mwilliams, qigao, myang

Differential Revision: 338346

src/runtime/base/array/array_data.cpp

@@ -417,13 +417,13 @@ void ArrayData::serializeImpl(VariableSerializer *serializer) const {
 }
 
 void ArrayData::serialize(VariableSerializer *serializer,
-                          bool isObject /* = false */) const {
+                          bool skipNestCheck /* = false */) const {
   if (size() == 0) {
     serializer->writeArrayHeader(this, 0);
     serializer->writeArrayFooter(this);
     return;
   }
-  if (!isObject) {
+  if (!skipNestCheck) {
     if (serializer->incNestedLevel((void*)this)) {
       serializer->writeOverflow((void*)this);
     } else {

src/runtime/base/array/array_data.h

@@ -312,7 +312,7 @@ class ArrayData : public Countable {
    * specialized implementation, which is normally more optimized.
    */
   void serialize(VariableSerializer *serializer,
-                 bool isObject = false) const;
+                 bool skipNestCheck = false) const;
 
   virtual void dump();
   virtual void dump(std::string &out);

src/runtime/base/type_variant.cpp

@@ -3603,7 +3603,8 @@ void Variant::setEvalScalar() const {
 // output functions
 
 void Variant::serialize(VariableSerializer *serializer,
-                        bool isArrayKey /* = false */) const {
+                        bool isArrayKey /* = false */,
+                        bool skipNestCheck /* = false */) const {
   if (m_type == KindOfVariant) {
     // Ugly, but behavior is different for serialize
     if (serializer->getType() == VariableSerializer::Serialize ||
@@ -3612,7 +3613,8 @@ void Variant::serialize(VariableSerializer *serializer,
       if (serializer->incNestedLevel(m_data.pvar)) {
         serializer->writeOverflow(m_data.pvar);
       } else {
-        m_data.pvar->serialize(serializer, isArrayKey);
+        // Tell the inner variant to skip the nesting check for data inside
+        m_data.pvar->serialize(serializer, isArrayKey, true);
       }
       serializer->decNestedLevel(m_data.pvar);
     } else {
@@ -3641,7 +3643,8 @@ void Variant::serialize(VariableSerializer *serializer,
     break;
   case KindOfArray:
     ASSERT(!isArrayKey);
-    m_data.parr->serialize(serializer);     break;
+    m_data.parr->serialize(serializer, skipNestCheck);
+    break;
   case KindOfObject:
     ASSERT(!isArrayKey);
     m_data.pobj->serialize(serializer);     break;

src/runtime/base/type_variant.h

@@ -709,7 +709,8 @@ class Variant {
    * Output functions
    */
   void serialize(VariableSerializer *serializer,
-                 bool isArrayKey = false) const;
+                 bool isArrayKey = false,
+                 bool skipNestCheck = false) const;
   void unserialize(VariableUnserializer *unserializer);
 
   /**

src/test/test_code_run.cpp

@@ -509,6 +509,7 @@ bool TestCodeRun::RunTests(const std::string &which) {
   RUN_TEST(TestCopyProp);
   RUN_TEST(TestParser);
   RUN_TEST(TestTypeAssertions);
+  RUN_TEST(TestSerialize);
 
   // PHP 5.3 features
   RUN_TEST(TestVariableClassName);
@@ -18658,6 +18659,21 @@ bool TestCodeRun::TestCopyProp() {
   return true;
 }
 
+bool TestCodeRun::TestSerialize() {
+  MVCR("<?php\n"
+       "function f() {\n"
+       "  $a = array(123);\n"
+       "  $b = $a;\n"
+       "  $c = &$b;\n"
+       "  $d = new stdClass();\n"
+       "  $v = array(&$a, &$b, &$c, $d, $d);\n"
+       "  $s = serialize($v);\n"
+       "  echo $s;\n"
+       "}\n"
+       "f();\n");
+  return true;
+}
+
 bool TestCodeRun::TestVariableClassName() {
   MVCRO(
     "<?php\n"

src/test/test_code_run.h

@@ -180,6 +180,7 @@ class TestCodeRun : public TestBase {
   bool TestMaxInt();
   bool TestParser();
   bool TestTypeAssertions();
+  bool TestSerialize();
 
   // PHP 5.3
   bool TestVariableClassName();