Bug 1451268: RematerializedFrames may not be cached, even when younger frames are. r=jorendorff

If the Debugger API tries to inspect or modify an IonMonkey frame, much of
the information it expects to find in a frame is missing: function calls
may have been inlined, variables may have been optimized out, and so on. So
when this happens, SpiderMonkey builds one or more Rematerialized frames
from the IonMonkey frame, using metadata built by Ion to reconstruct the
missing parts. The Rematerialized frames are now the authority on the state
of those frames, and the Ion frame is ignored: stack iterators ignore the
Ion frame, producing the Rematerialized frames in their stead; and when
control returns to the Ion frame, we pop it, rebuild Baseline frames from
the Rematerialized frames, and resume execution in Baseline.

Thus, Rematerialized frames are always created with their
hasCachedSavedFrame bits clear: although there may be extant SavedFrames
built from the original IonMonkey frame, the Rematerialized frames will not
have cache entries for them until they are traversed in a capture themselves.

This means that, oddly, it is not always true that, once we reach a frame
with its hasCachedSavedFrame bit set, all its parents will have the bit set
as well. However, clear bits under younger set bits will only occur on
Rematerialized frames.

Differential Revision: https://phabricator.services.mozilla.com/D29785

--HG--
extra : moz-landing-system : lando

js/src/jit-test/tests/saved-stacks/bug-1451268.js

@@ -0,0 +1,23 @@
+// |jit-test| --no-threads; --ion-eager
+// Walking into Rematerialized frames under ordinary frames with their
+// hasCachedSavedFrame bits set shouldn't cause an assertion.
+
+enableTrackAllocations();
+var g = newGlobal({ newCompartment: true });
+var dbg = new Debugger;
+g.toggle = function toggle(x, d) {
+    if (d) {
+        dbg.addDebuggee(g);
+        var frame = dbg.getNewestFrame().older;
+    }
+};
+g.eval("" + function f(x, d) {
+    g(x, d);
+});
+g.eval("" + function g(x, d) {
+    toggle(x, d);
+});
+g.eval("(" + function test() {
+    for (var i = 0; i < 5; i++) f(42, false);
+    f(42, true);
+} + ")();");

js/src/jit/RematerializedFrame.h

@@ -19,10 +19,19 @@
 namespace js {
 namespace jit {
 
+// RematerializedFrame: An optimized frame that has been rematerialized with
+// values read out of Snapshots.
 //
-// An optimized frame that has been rematerialized with values read out of
-// Snapshots.
-//
+// If the Debugger API tries to inspect or modify an IonMonkey frame, much of
+// the information it expects to find in a frame is missing: function calls may
+// have been inlined, variables may have been optimized out, and so on. So when
+// this happens, SpiderMonkey builds one or more Rematerialized frames from the
+// IonMonkey frame, using the snapshot metadata built by Ion to reconstruct the
+// missing parts. The Rematerialized frames are now the authority on the state
+// of those frames, and the Ion frame is ignored: stack iterators ignore the Ion
+// frame, producing the Rematerialized frames in their stead; and when control
+// returns to the Ion frame, we pop it, rebuild Baseline frames from the
+// Rematerialized frames, and resume execution in Baseline.
 class RematerializedFrame {
   // See DebugScopes::updateLiveScopes.
   bool prevUpToDate_;

js/src/vm/SavedStacks.cpp

@@ -1422,7 +1422,10 @@ bool SavedStacks::insertFrames(JSContext* cx, MutableHandleSavedFrame frame,
         LiveSavedFrameCache::FramePtr::create(iter);
 
     if (framePtr) {
-      MOZ_ASSERT_IF(seenCached, framePtr->hasCachedSavedFrame());
+      // See the comment in Stack.h for why RematerializedFrames
+      // are a special case here.
+      MOZ_ASSERT_IF(seenCached, framePtr->hasCachedSavedFrame() ||
+                                    framePtr->isRematerializedFrame());
       seenCached |= framePtr->hasCachedSavedFrame();
     }
 

js/src/vm/Stack.h

@@ -1139,7 +1139,7 @@ namespace js {
 //
 //     P  > Q  > T  > U
 //
-// Some details:
+// Details:
 //
 // - When we find a cache entry whose frame address matches our frame F, we know
 //   that F has never left the stack, but it may certainly be the case that
@@ -1221,6 +1221,17 @@ namespace js {
 //   select the appropriate parent at that point: rather than the next-older
 //   frame, we find the SavedFrame for the eval's target frame. The skip appears
 //   in the SavedFrame chains, even as the traversal covers all the frames.
+//
+// - Rematerialized frames (see ../jit/RematerializedFrame.h) are always created
+//   with their hasCachedSavedFrame bits clear: although there may be extant
+//   SavedFrames built from the original IonMonkey frame, the Rematerialized
+//   frames will not have cache entries for them until they are traversed in a
+//   capture themselves.
+//
+//   This means that, oddly, it is not always true that, once we reach a frame
+//   with its hasCachedSavedFrame bit set, all its parents will have the bit set
+//   as well. However, clear bits under younger set bits will only occur on
+//   Rematerialized frames.
 class LiveSavedFrameCache {
  public:
   // The address of a live frame for which we can cache SavedFrames: it has a
@@ -1263,6 +1274,11 @@ class LiveSavedFrameCache {
       return *ptr.as<InterpreterFrame*>();
     }
 
+    // Return true if this FramePtr refers to a rematerialized frame.
+    inline bool isRematerializedFrame() const {
+      return ptr.is<jit::RematerializedFrame*>();
+    }
+
     bool operator==(const FramePtr& rhs) const { return rhs.ptr == this->ptr; }
     bool operator!=(const FramePtr& rhs) const { return !(rhs == *this); }
   };