Fix off-by-one in multi-line function call traces

Fixes off-by-one in line number reporting in multi-line function calls

hphp/runtime/vm/bytecode.cpp

@@ -2346,8 +2346,23 @@ Array VMExecutionContext::debugBacktrace(bool skip /* = false */,
       const char *filename = unit->filepath()->data();
       assert(filename);
       frame.set(String(s_file), filename, true);
+
+      // In the normal method case, the "saved pc" for line number printing is
+      // pointing at the cell conversion (Unbox/Pop) instruction, not the call
+      // itself. For multi-line calls, this instruction is associated with the
+      // subsequent line which results in an off-by-n. We're subtracting one
+      // in order to look up the line associated with the FCall/FCallArray
+      // instruction. Exception handling and the other opcodes (ex. BoxR)
+      // already do the right thing. The emitter associates object access with
+      // the subsequent expression and this would be difficult to modify.
+      Opcode* opAtSavedPc = (Opcode*)unit->at(pc);
+      assert(opAtSavedPc);
+      Offset pcAdjust = 0;
+      if (*opAtSavedPc == Op::OpPopR || *opAtSavedPc == Op::OpUnboxR) {
+        pcAdjust = 1;
+      }
       frame.set(String(s_line),
-                prevFp->m_func->unit()->getLineNumber(pc), true);
+                prevFp->m_func->unit()->getLineNumber(pc - pcAdjust), true);
     }
     // check for include
     String funcname = const_cast<StringData*>(fp->m_func->name());

hphp/runtime/vm/unit.cpp

@@ -1399,6 +1399,8 @@ Func* Unit::getMain(Class* cls /*= NULL*/) const {
   return f;
 }
 
+// This uses range lookups so offsets in the middle of instructions are
+// supported.
 int Unit::getLineNumber(Offset pc) const {
   LineEntry key = LineEntry(pc, -1);
   std::vector<LineEntry>::const_iterator it =

hphp/test/quick/debug_backtrace_multiline.php

@@ -0,0 +1,55 @@
+<?hh
+
+function compare_lines($line_from_macro, $line_from_trace) {
+  if ($line_from_macro != $line_from_trace) {
+    echo "Expected line: ".$line_from_macro."\n";
+    echo "Got line: ".$line_from_trace."\n";
+  }
+}
+
+class A {
+  function x($line, $trace_elem = 0) {
+    compare_lines($line, debug_backtrace()[$trace_elem]['line']);
+    return $this;
+  }
+}
+
+class B {
+  function __construct($line) {
+    compare_lines($line, debug_backtrace()[0]['line']);
+  }
+}
+
+class C {
+}
+
+function newInstanceOfA($line) {
+  return (new A())->x($line, 1);
+}
+
+// UnboxR
+(new A())->x(__LINE__)->
+  x(__LINE__)->
+  x(__LINE__);
+
+// PopR
+(new A())->x(__LINE__)
+  ;
+
+// BoxR
+$var = &newInstanceOfA(__LINE__);
+$var =
+  &newInstanceOfA(__LINE__)
+  ;
+$var =
+  &newInstanceOfA(__LINE__);
+
+// handled exception
+try {
+  throw new Exception(__LINE__);
+} catch (Exception $e) {
+  compare_lines($e->getMessage(), $e->getLine());
+}
+
+// unhandled exception
+throw new Exception(__LINE__);

hphp/test/quick/debug_backtrace_multiline.php.expectf

@@ -0,0 +1 @@
+HipHop Fatal error: Uncaught exception 'Exception' with message '55' in %s:55\nStack trace:\n#0 {main}