Fix a bug with debug_backtrace during an unwind

The change to stop zeroing locals during RetC was too
aggressive---we stopped doing it during unwinding also.  When
unwinding, we need to zero locals and $this because another
destructing object may still run debug_backtrace, and we won't have
the *pc == OpRet{C,V} trick to tell us to ignore the junk.

hphp/runtime/vm/bytecode.cpp

@@ -1026,10 +1026,15 @@ UnwindStatus Stack::unwindFrag(ActRec* fp, int offset,
      *
      *   - Finally, the exit hook for the returning function can
      *     throw, but this happens last so everything is destructed.
+     *
      */
     if (!unwindingReturningFrame) {
       try {
-        frame_free_locals_inl(fp, func->numLocals());
+        // Note that we must convert locals and the $this to
+        // uninit/zero during unwind.  This is because a backtrace
+        // from another destructing object during this unwind may try
+        // to read them.
+        frame_free_locals_unwind(fp, func->numLocals());
       } catch (...) {}
     }
     ndiscard(func->numSlotsInFrame());

hphp/runtime/vm/event_hook.cpp

@@ -164,7 +164,7 @@ bool EventHook::RunInterceptHandler(ActRec* ar) {
 
     Variant ret = vm_call_user_func(h->asCArrRef()[0], intArgs);
     if (doneFlag.toBoolean()) {
-      frame_free_locals_inl_no_hook(ar, ar->m_func->numLocals());
+      frame_free_locals_inl_no_hook<true>(ar, ar->m_func->numLocals());
       Stack& stack = g_vmContext->getStack();
       stack.top() = (Cell*)(ar + 1);
       tvDup(ret.asTypedValue(), stack.allocTV());

hphp/runtime/vm/runtime.h

@@ -81,6 +81,16 @@ frame_local_inner(const ActRec* fp, int n) {
   return ret->m_type == KindOfRef ? ret->m_data.pref->tv() : ret;
 }
 
+/*
+ * 'Unwinding' versions of the below frame_free_locals_* functions
+ * zero locals and the $this pointer.
+ *
+ * This is necessary during unwinding because another object being
+ * destructed by the unwind may decide to do a debug_backtrace and
+ * read a destructed value.
+ */
+
+template<bool unwinding>
 inline void ALWAYS_INLINE
 frame_free_locals_helper_inl(ActRec* fp, int numLocals) {
   assert(numLocals == fp->m_func->numLocals());
@@ -106,32 +116,46 @@ frame_free_locals_helper_inl(ActRec* fp, int numLocals) {
     DataType t = loc->m_type;
     if (IS_REFCOUNTED_TYPE(t)) {
       uint64_t datum = loc->m_data.num;
+      if (unwinding) {
+        tvWriteUninit(loc);
+      }
       tvDecRefHelper(t, datum);
     }
   }
 }
 
+template<bool unwinding>
 inline void ALWAYS_INLINE
 frame_free_locals_inl_no_hook(ActRec* fp, int numLocals) {
   if (fp->hasThis()) {
     ObjectData* this_ = fp->getThis();
+    if (unwinding) {
+      fp->setThis(nullptr);
+    }
     decRefObj(this_);
   }
-  frame_free_locals_helper_inl(fp, numLocals);
+  frame_free_locals_helper_inl<unwinding>(fp, numLocals);
 }
 
 inline void ALWAYS_INLINE
 frame_free_locals_inl(ActRec* fp, int numLocals) {
-  frame_free_locals_inl_no_hook(fp, numLocals);
+  frame_free_locals_inl_no_hook<false>(fp, numLocals);
+  EventHook::FunctionExit(fp);
+}
+
+inline void ALWAYS_INLINE
+frame_free_locals_unwind(ActRec* fp, int numLocals) {
+  frame_free_locals_inl_no_hook<true>(fp, numLocals);
   EventHook::FunctionExit(fp);
 }
 
 inline void ALWAYS_INLINE
 frame_free_locals_no_this_inl(ActRec* fp, int numLocals) {
-  frame_free_locals_helper_inl(fp, numLocals);
+  frame_free_locals_helper_inl<false>(fp, numLocals);
   EventHook::FunctionExit(fp);
 }
 
+// Helper for iopFCallBuiltin.
 inline void ALWAYS_INLINE
 frame_free_args(TypedValue* args, int count) {
   for (int i = 0; i < count; i++) {
@@ -139,10 +163,13 @@ frame_free_args(TypedValue* args, int count) {
     DataType t = loc->m_type;
     if (IS_REFCOUNTED_TYPE(t)) {
       uint64_t datum = loc->m_data.num;
+      // We don't have to write KindOfUninit here, because a
+      // debug_backtrace wouldn't be able to see these slots (they are
+      // stack cells).  But note we're also relying on the destructors
+      // not throwing.
       tvDecRefHelper(t, datum);
     }
   }
-
 }
 
 Unit*

hphp/test/quick/unwind_backtrace.php

@@ -0,0 +1,32 @@
+<?php
+
+function blah() {
+  throw new Exception('asd');
+}
+
+class something {
+  public function __destruct() {
+    echo "~something\n";
+    $k = debug_backtrace()[1]['object'];
+    var_dump($k);
+  }
+  public function yo() {
+    blah();
+  }
+}
+
+class Bar {
+  public function foo() {
+    $k = new something();
+    echo "wat\n";
+    blah();
+    echo "eh\n";
+  }
+}
+
+function main() {
+  (new Bar)->foo();
+}
+
+main();
+

hphp/test/quick/unwind_backtrace.php.expectf

@@ -0,0 +1,5 @@
+wat
+~something
+HipHop Notice: Undefined index: object in %s on line 10
+NULL
+HipHop Fatal error: Uncaught exception 'Exception' with message 'asd' in %s