Fix closure dispatch when there's more than one Func*

We need to make sure that we execute the code
corresponding to the actual Func*. This re-uses the
prolog array to hold the entry points for the cloned
Func*.

hphp/runtime/vm/func.cpp

@@ -365,6 +365,12 @@ const FPIEnt* Func::findPrecedingFPI(Offset o) const {
   return fe;
 }
 
+bool Func::isClonedClosure() const {
+  if (!isClosureBody()) return false;
+  if (!cls()) return true;
+  return cls()->lookupMethod(name()) != this;
+}
+
 bool Func::isNameBindingImmutable(const Unit* fromUnit) const {
   if (RuntimeOption::EvalJitEnableRenameFunction ||
       m_attrs & AttrDynamicInvoke) {

hphp/runtime/vm/func.h

@@ -145,9 +145,9 @@ struct Func {
 #endif
   }
 
-  FuncId getFuncId() const { 
+  FuncId getFuncId() const {
     assert(m_funcId != InvalidId);
-    return m_funcId; 
+    return m_funcId;
   }
   void setFuncId(FuncId id);
   void setNewFuncId();
@@ -321,6 +321,7 @@ struct Func {
   bool top() const { return shared()->m_top; }
   const StringData* docComment() const { return shared()->m_docComment; }
   bool isClosureBody() const { return shared()->m_isClosureBody; }
+  bool isClonedClosure() const;
   bool isGenerator() const { return shared()->m_isGenerator; }
   bool isGeneratorFromClosure() const {
     return shared()->m_isGeneratorFromClosure;

hphp/runtime/vm/translator/translator-x64-helpers.cpp

@@ -86,9 +86,18 @@ void TranslatorX64::reqLitHelper(const ReqLitStaticArgs* args) {
  */
 
 static TCA callAndResume(ActRec *ar) {
-  VMRegAnchor _(ar, true);
-  g_vmContext->doFCall<true>(ar, g_vmContext->m_pc);
-  return Translator::Get()->getResumeHelperRet();
+  if (!ar->m_func->isClonedClosure()) {
+    /*
+     * If the func is a cloned closure, then the original
+     * closure has already run the prolog, and the prologs
+     * array is just being used as entry points for the
+     * dv funclets. Dont run the prolog again.
+     */
+    VMRegAnchor _(ar, true);
+    g_vmContext->doFCall<true>(ar, g_vmContext->m_pc);
+    return Translator::Get()->getResumeHelperRet();
+  }
+  return Translator::Get()->getResumeHelper();
 }
 
 static_assert(rStashedAR == reg::r15,
@@ -99,14 +108,22 @@ asm(
   ".globl __fcallHelperThunk\n"
 "__fcallHelperThunk:\n"
 #if defined(__x86_64__)
+  // fcallHelper is used for prologs, and (in the case of
+  // closures) for dispatch to the function body. In the first
+  // case, there's a call, in the second, there's a jmp.
+  // We can differentiate by comparing r15 and rVmFp
+  "mov %r15, %rdi\n"
+  "cmp %r15, %rbp\n"
+  "jne 1f\n"
+  "call fcallHelper\n"
+  "jmp *%rax\n"
   // fcallHelper may call doFCall. doFCall changes the return ip
   // pointed to by r15 so that it points to TranslatorX64::m_retHelper,
   // which does a REQ_POST_INTERP_RET service request. So we need to
   // to pop the return address into r15 + m_savedRip before calling
   // fcallHelper, and then push it back from r15 + m_savedRip after
   // fcallHelper returns in case it has changed it.
-  "pop 0x8(%r15)\n"
-  "mov %r15, %rdi\n"
+  "1: pop 0x8(%r15)\n"
   "call fcallHelper\n"
   "push 0x8(%r15)\n"
   "jmp *%rax\n"
@@ -122,7 +139,7 @@ extern "C"
 TCA fcallHelper(ActRec* ar) {
   try {
     TCA tca =
-      Translator::Get()->funcPrologue((Func*)ar->m_func, ar->numArgs());
+      Translator::Get()->funcPrologue((Func*)ar->m_func, ar->numArgs(), ar);
     if (tca) {
       return tca;
     }

hphp/runtime/vm/translator/translator-x64.cpp

@@ -2039,8 +2039,16 @@ TranslatorX64::emitPopRetIntoActRec(Asm& a) {
   a.    pop  (rStashedAR[AROFF(m_savedRip)]);
 }
 
+static void interp_set_regs(ActRec* ar, Cell* sp, Offset pcOff) {
+  assert(tl_regState == REGSTATE_DIRTY);
+  tl_regState = REGSTATE_CLEAN;
+  vmfp() = (Cell*)ar;
+  vmsp() = sp;
+  vmpc() = curUnit()->at(pcOff);
+}
+
 TCA
-TranslatorX64::funcPrologue(Func* func, int nPassed) {
+TranslatorX64::funcPrologue(Func* func, int nPassed, ActRec* ar) {
   func->validate();
   TRACE(1, "funcPrologue %s(%d)\n", func->fullName()->data(), nPassed);
   int numParams = func->numParams();
@@ -2051,6 +2059,27 @@ TranslatorX64::funcPrologue(Func* func, int nPassed) {
   // Do a quick test before grabbing the write lease
   TCA prologue;
   if (checkCachedPrologue(func, paramIndex, prologue)) return prologue;
+  if (func->isClonedClosure()) {
+    assert(ar);
+    const Func::ParamInfoVec& paramInfo = func->params();
+    Offset entry = func->base();
+    for (int i = nPassed; i < numParams; ++i) {
+      const Func::ParamInfo& pi = paramInfo[i];
+      if (pi.hasDefaultValue()) {
+        entry = pi.funcletOff();
+        break;
+      }
+    }
+    interp_set_regs(ar, (Cell*)ar - func->numSlotsInFrame(), entry);
+    SrcKey funcBody(func, entry);
+    TCA tca = getTranslation(funcBody, false);
+    tl_regState = REGSTATE_DIRTY;
+    if (tca) {
+      // racy, but ok...
+      func->setPrologue(paramIndex, tca);
+    }
+    return tca;
+  }
 
   // If the translator is getting replaced out from under us, refuse to
   // provide a prologue; we don't know whether this request is running on the
@@ -2060,6 +2089,7 @@ TranslatorX64::funcPrologue(Func* func, int nPassed) {
   // Double check the prologue array now that we have the write lease
   // in case another thread snuck in and set the prologue already.
   if (checkCachedPrologue(func, paramIndex, prologue)) return prologue;
+
   SpaceRecorder sr("_FuncPrologue", a);
   // If we're close to a cache line boundary, just burn some space to
   // try to keep the func and its body on fewer total lines.
@@ -2394,7 +2424,15 @@ TranslatorX64::emitPrologue(Func* func, int nPassed) {
   // code
   emitCheckSurpriseFlagsEnter(false, fixup);
 
-  emitBindJmp(funcBody);
+  if (func->isClosureBody() && func->cls()) {
+    int entry = nPassed <= numParams ? nPassed : numParams + 1;
+    // Relying on rStashedAR == rVmFp here
+    a.    loadq   (rStashedAR[AROFF(m_func)], rax);
+    a.    loadq   (rax[Func::prologueTableOff() + sizeof(TCA)*entry], rax);
+    a.    jmp     (rax);
+  } else {
+    emitBindJmp(funcBody);
+  }
   return funcBody;
 }
 
@@ -4001,14 +4039,6 @@ TranslatorX64::binaryArithLocal(const NormalizedInstruction &i,
   }
 }
 
-static void interp_set_regs(ActRec* ar, Cell* sp, Offset pcOff) {
-  assert(tl_regState == REGSTATE_DIRTY);
-  tl_regState = REGSTATE_CLEAN;
-  vmfp() = (Cell*)ar;
-  vmsp() = sp;
-  vmpc() = curUnit()->at(pcOff);
-}
-
 #define O(opcode, imm, pusph, pop, flags) \
 /**
  * The interpOne methods saves m_pc, m_fp, and m_sp ExecutionContext,

hphp/runtime/vm/translator/translator-x64.h

@@ -939,7 +939,7 @@ private:
   static int  shuffleArgsForMagicCall(ActRec* ar);
   static void setArgInActRec(ActRec* ar, int argNum, uint64_t datum,
                              DataType t);
-  TCA funcPrologue(Func* func, int nArgs);
+  TCA funcPrologue(Func* func, int nArgs, ActRec* ar = nullptr);
   bool checkCachedPrologue(const Func* func, int param, TCA& plgOut) const;
   SrcKey emitPrologue(Func* func, int nArgs);
   int32_t emitNativeImpl(const Func*, bool emitSavedRIPReturn);

hphp/runtime/vm/translator/translator.h

@@ -891,7 +891,7 @@ public:
   virtual void requestInit() = 0;
   virtual void requestExit() = 0;
   virtual void analyzeInstr(Tracelet& t, NormalizedInstruction& i) = 0;
-  virtual TCA funcPrologue(Func* f, int nArgs) = 0;
+  virtual TCA funcPrologue(Func* f, int nArgs, ActRec* ar = nullptr) = 0;
   virtual TCA getCallToExit() = 0;
   virtual TCA getRetFromInterpretedFrame() = 0;
   virtual void defineCns(StringData* name) = 0;

hphp/test/vm/closure_clone_2.php

@@ -0,0 +1,33 @@
+<?php
+
+trait T {
+  function f() {
+    return function ($a) {
+      if ($a) {
+        return $this->foo;
+      }
+    };
+  }
+}
+
+class X {
+  private $foo = 1;
+  use T;
+}
+
+class Y {
+  private $foo = 2;
+  use T;
+}
+
+function test() {
+  $x = new X;
+  $c = $x->f();
+  var_dump($c(true));
+
+  $y = new Y;
+  $c = $y->f();
+  var_dump($c("foo"));
+}
+
+test();

hphp/test/vm/closure_clone_2.php.exp

@@ -0,0 +1,2 @@
+int(1)
+int(2)