From b2de6fb9daf34b41c87e48d5a8cc42d25ad1b15b Mon Sep 17 00:00:00 2001
From: Paul Tarjan <ptarjan@fb.com>
Date: Sun, 12 May 2013 12:25:16 -0700
Subject: [PATCH] fix segfault for wikimedia

If you call a static method on a instance, we decRef the object before running the method. If that is the last reference to the object, then its destructor will be called right in the middle of setting up the ActRec for the static method. Currently, the top of the stack will be off by 2 Cells because we have already popped off one Cell (the object) and an ActRec takes up 3 Cells.

Instead, we have to have the top of the stack to be after the ActRec that we are building for the next method call.
---
 .../vm/translator/hopt/hhbctranslator.cpp     |  5 ++++
 hphp/runtime/vm/translator/targetcache.cpp    |  8 +++++++
 hphp/test/quick/destruct_segfault.php         | 19 +++++++++++++++
 hphp/test/quick/destruct_segfault.php.expect  |  1 +
 hphp/test/slow/redeclared_classes/1474.php    | 23 ++++++++++++++++++-
 5 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 hphp/test/quick/destruct_segfault.php
 create mode 100644 hphp/test/quick/destruct_segfault.php.expect

diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
index 4841c6346..c5e3b304f 100644
--- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
+++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
@@ -1771,6 +1771,11 @@ void HhbcTranslator::emitFPushObjMethodD(int32_t numParams,
                     nullptr);
     auto const actRec = spillStack();
     auto const objCls = gen(LdObjClass, obj);
+
+    // This is special. We need to move the stackpointer incase LdObjMethod
+    // calls a destructor. Otherwise it would clobber the ActRec we just pushed.
+    emitMarker();
+
     gen(LdObjMethod,
               objCls,
               cns(methodName),
diff --git a/hphp/runtime/vm/translator/targetcache.cpp b/hphp/runtime/vm/translator/targetcache.cpp
index 59a2e904c..c4b78b5ca 100644
--- a/hphp/runtime/vm/translator/targetcache.cpp
+++ b/hphp/runtime/vm/translator/targetcache.cpp
@@ -539,6 +539,14 @@ void methodCacheSlowPath(MethodCache::Pair* mce,
     ObjectData* arThis = ar->getThis();
     shouldBeObj->m_type = KindOfObject;
     shouldBeObj->m_data.pobj = arThis;
+
+    // There used to be a half-built ActRec on the stack that we need the
+    // unwinder to ignore. We overwrote 1/3 of it with the code above, but
+    // because of the emitMarker() in LdObjMethod we need the other two slots
+    // to not have any TypedValues.
+    tvWriteNull(shouldBeObj-1);
+    tvWriteNull(shouldBeObj-2);
+
     throw;
   }
 }
diff --git a/hphp/test/quick/destruct_segfault.php b/hphp/test/quick/destruct_segfault.php
new file mode 100644
index 000000000..8e1e799b5
--- /dev/null
+++ b/hphp/test/quick/destruct_segfault.php
@@ -0,0 +1,19 @@
+<?php
+class A {
+
+  public static function factory() {
+    return new A;
+  }
+
+  public static function b() {
+    return 'no segfault';
+  }
+
+  public function __destruct() {
+  }
+}
+
+function main() {
+  echo A::factory()->b();
+}
+main();
diff --git a/hphp/test/quick/destruct_segfault.php.expect b/hphp/test/quick/destruct_segfault.php.expect
new file mode 100644
index 000000000..d115e2bbe
--- /dev/null
+++ b/hphp/test/quick/destruct_segfault.php.expect
@@ -0,0 +1 @@
+no segfault
\ No newline at end of file
diff --git a/hphp/test/slow/redeclared_classes/1474.php b/hphp/test/slow/redeclared_classes/1474.php
index e4caef5a1..cbd4aceb5 100644
--- a/hphp/test/slow/redeclared_classes/1474.php
+++ b/hphp/test/slow/redeclared_classes/1474.php
@@ -1,3 +1,24 @@
 <?php
 
- function nop($en,$es){};set_error_handler('nop');class X { function bar() { var_dump($this); } }if (1) {  class U {  }} else {  class U extends X {  }}class V extends U {}function test() {  $x = new X;  $x->bar();  $x = new V;  $x->bar();}test();
\ No newline at end of file
+function nop($en,$es){};
+set_error_handler('nop');
+class X {
+  function bar() {
+    var_dump($this);
+  }
+}
+if (1) {
+  class U {  }
+} else {
+  class U extends X {  }
+}
+
+class V extends U {}
+
+function test() {
+  $x = new X;
+  $x->bar();
+  $x = new V;
+  $x->bar();
+}
+test();