Bug 1120677 - Fix GetPcScript to ignore BaselineFrames with an override pc. r=shu

js/src/jit/BaselineFrame.h

@@ -332,13 +332,17 @@ class BaselineFrame
     void deleteDebugModeOSRInfo();
 
     // See the HAS_OVERRIDE_PC comment.
+    bool hasOverridePc() const {
+        return flags_ & HAS_OVERRIDE_PC;
+    }
+
     jsbytecode *overridePc() const {
-        MOZ_ASSERT(flags_ & HAS_OVERRIDE_PC);
+        MOZ_ASSERT(hasOverridePc());
         return script()->offsetToPC(overrideOffset_);
     }
 
     jsbytecode *maybeOverridePc() const {
-        if (flags_ & HAS_OVERRIDE_PC)
+        if (hasOverridePc())
             return overridePc();
         return nullptr;
     }

js/src/jit/JitFrameIterator.h

@@ -160,6 +160,12 @@ class JitFrameIterator
     bool isBaselineStub() const {
         return type_ == JitFrame_BaselineStub;
     }
+    bool isBaselineStubMaybeUnwound() const {
+        return type_ == JitFrame_BaselineStub || type_ == JitFrame_Unwound_BaselineStub;
+    }
+    bool isRectifierMaybeUnwound() const {
+        return type_ == JitFrame_Rectifier || type_ == JitFrame_Unwound_Rectifier;
+    }
     bool isBareExit() const;
     template <typename T> bool isExitFrameLayout() const;
 

js/src/jit/JitFrames.cpp

@@ -1476,49 +1476,63 @@ GetPcScript(JSContext *cx, JSScript **scriptRes, jsbytecode **pcRes)
 {
     JitSpew(JitSpew_IonSnapshots, "Recover PC & Script from the last frame.");
 
+    // Recover the return address so that we can look it up in the
+    // PcScriptCache, as script/pc computation is expensive.
     JSRuntime *rt = cx->runtime();
-
-    // Recover the return address.
     JitActivationIterator iter(rt);
     JitFrameIterator it(iter);
-
-    // If the previous frame is a rectifier frame (maybe unwound),
-    // skip past it.
-    if (it.prevType() == JitFrame_Rectifier || it.prevType() == JitFrame_Unwound_Rectifier) {
+    uint8_t *retAddr;
+    if (it.type() == JitFrame_Exit) {
         ++it;
-        MOZ_ASSERT(it.prevType() == JitFrame_BaselineStub ||
-                   it.prevType() == JitFrame_BaselineJS ||
-                   it.prevType() == JitFrame_IonJS);
+
+        // Skip rectifier frames.
+        if (it.isRectifierMaybeUnwound()) {
+            ++it;
+            MOZ_ASSERT(it.isBaselineStub() || it.isBaselineJS() || it.isIonJS());
+        }
+
+        // Skip Baseline stub frames.
+        if (it.isBaselineStubMaybeUnwound()) {
+            ++it;
+            MOZ_ASSERT(it.isBaselineJS());
+        }
+
+        MOZ_ASSERT(it.isBaselineJS() || it.isIonJS());
+
+        // Don't use the return address if the BaselineFrame has an override pc.
+        // The override pc is cheap to get, so we won't benefit from the cache,
+        // and the override pc could change without the return address changing.
+        // Moreover, sometimes when an override pc is present during exception
+        // handling, the return address is set to nullptr as a sanity check,
+        // since we do not return to the frame that threw the exception.
+        if (!it.isBaselineJS() || !it.baselineFrame()->hasOverridePc()) {
+            retAddr = it.returnAddressToFp();
+            MOZ_ASSERT(retAddr);
+        } else {
+            retAddr = nullptr;
+        }
+    } else {
+        MOZ_ASSERT(it.isBailoutJS());
+        retAddr = it.returnAddress();
     }
 
-    // If the previous frame is a stub frame, skip the exit frame so that
-    // returnAddress below gets the return address into the BaselineJS
-    // frame.
-    if (it.prevType() == JitFrame_BaselineStub || it.prevType() == JitFrame_Unwound_BaselineStub) {
-        ++it;
-        MOZ_ASSERT(it.prevType() == JitFrame_BaselineJS);
+    uint32_t hash;
+    if (retAddr) {
+        hash = PcScriptCache::Hash(retAddr);
+
+        // Lazily initialize the cache. The allocation may safely fail and will not GC.
+        if (MOZ_UNLIKELY(rt->ionPcScriptCache == nullptr)) {
+            rt->ionPcScriptCache = (PcScriptCache *)js_malloc(sizeof(struct PcScriptCache));
+            if (rt->ionPcScriptCache)
+                rt->ionPcScriptCache->clear(rt->gc.gcNumber());
+        }
+
+        if (rt->ionPcScriptCache && rt->ionPcScriptCache->get(rt, hash, retAddr, scriptRes, pcRes))
+            return;
     }
 
-    uint8_t *retAddr = it.returnAddress();
-    uint32_t hash = PcScriptCache::Hash(retAddr);
-    MOZ_ASSERT(retAddr != nullptr);
-
-    // Lazily initialize the cache. The allocation may safely fail and will not GC.
-    if (MOZ_UNLIKELY(rt->ionPcScriptCache == nullptr)) {
-        rt->ionPcScriptCache = (PcScriptCache *)js_malloc(sizeof(struct PcScriptCache));
-        if (rt->ionPcScriptCache)
-            rt->ionPcScriptCache->clear(rt->gc.gcNumber());
-    }
-
-    // Attempt to lookup address in cache.
-    if (rt->ionPcScriptCache && rt->ionPcScriptCache->get(rt, hash, retAddr, scriptRes, pcRes))
-        return;
-
     // Lookup failed: undertake expensive process to recover the innermost inlined frame.
-    if (!it.isBailoutJS())
-        ++it; // Skip exit frame.
     jsbytecode *pc = nullptr;
-
     if (it.isIonJS() || it.isBailoutJS()) {
         InlineFrameIterator ifi(cx, &it);
         *scriptRes = ifi.script();
@@ -1532,7 +1546,7 @@ GetPcScript(JSContext *cx, JSScript **scriptRes, jsbytecode **pcRes)
         *pcRes = pc;
 
     // Add entry to cache.
-    if (rt->ionPcScriptCache)
+    if (retAddr && rt->ionPcScriptCache)
         rt->ionPcScriptCache->add(hash, retAddr, pc, *scriptRes);
 }
 

js/src/jit/PcScriptCache.h

@@ -61,6 +61,9 @@ struct PcScriptCache
     }
 
     void add(uint32_t hash, uint8_t *addr, jsbytecode *pc, JSScript *script) {
+        MOZ_ASSERT(addr);
+        MOZ_ASSERT(pc);
+        MOZ_ASSERT(script);
         entries[hash].returnAddress = addr;
         entries[hash].pc = pc;
         entries[hash].script = script;