Bug 1120677 - Fix GetPcScript to ignore BaselineFrames with an override pc. r=shu, a=sledru
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 20 Jan 2015 17:36:39 +0100
changeset 249375 9aed7f7c9de8a189998e554b5febf2015f43b298
parent 249374 0fe217418754b6aeef7d4d3fd733ee4088ae60ea
child 249376 06da1141e8176035b45bd0a09214e597108107ee
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, sledru
bugs1120677
milestone37.0a2
Bug 1120677 - Fix GetPcScript to ignore BaselineFrames with an override pc. r=shu, a=sledru
js/src/jit/BaselineFrame.h
js/src/jit/JitFrameIterator.h
js/src/jit/JitFrames.cpp
js/src/jit/PcScriptCache.h
--- a/js/src/jit/BaselineFrame.h
+++ b/js/src/jit/BaselineFrame.h
@@ -339,23 +339,27 @@ class BaselineFrame
     void setDebugModeOSRInfo(BaselineDebugModeOSRInfo *info) {
         flags_ |= HAS_DEBUG_MODE_OSR_INFO;
         debugModeOSRInfo_ = info;
     }
 
     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;
     }
 
     void setOverridePc(jsbytecode *pc) {
         flags_ |= HAS_OVERRIDE_PC;
         overrideOffset_ = script()->pcToOffset(pc);
     }
--- a/js/src/jit/JitFrameIterator.h
+++ b/js/src/jit/JitFrameIterator.h
@@ -149,16 +149,22 @@ class JitFrameIterator
         return type_ == JitFrame_IonJS;
     }
     bool isBailoutJS() const {
         return type_ == JitFrame_Bailout;
     }
     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;
 
     bool isEntry() const {
         return type_ == JitFrame_Entry;
     }
     bool isFunctionFrame() const;
 
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -1452,73 +1452,87 @@ void UpdateJitActivationsForMinorGC(PerT
     }
 }
 
 void
 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) {
-        ++it;
-        MOZ_ASSERT(it.prevType() == JitFrame_BaselineStub ||
-                   it.prevType() == JitFrame_BaselineJS ||
-                   it.prevType() == JitFrame_IonJS);
-    }
-
-    // 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) {
+    uint8_t *retAddr;
+    if (it.type() == JitFrame_Exit) {
         ++it;
-        MOZ_ASSERT(it.prevType() == JitFrame_BaselineJS);
+
+        // 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();
     }
 
-    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());
+    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;
     }
 
-    // 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();
         pc = ifi.pc();
     } else {
         MOZ_ASSERT(it.isBaselineJS());
         it.baselineScriptAndPc(scriptRes, &pc);
     }
 
     if (pcRes)
         *pcRes = pc;
 
     // Add entry to cache.
-    if (rt->ionPcScriptCache)
+    if (retAddr && rt->ionPcScriptCache)
         rt->ionPcScriptCache->add(hash, retAddr, pc, *scriptRes);
 }
 
 void
 OsiIndex::fixUpOffset(MacroAssembler &masm)
 {
     callPointDisplacement_ = masm.actualOffset(callPointDisplacement_);
 }
--- a/js/src/jit/PcScriptCache.h
+++ b/js/src/jit/PcScriptCache.h
@@ -56,16 +56,19 @@ struct PcScriptCache
         *scriptRes = entries[hash].script;
         if (pcRes)
             *pcRes = entries[hash].pc;
 
         return true;
     }
 
     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;
     }
 
     static uint32_t Hash(uint8_t *addr) {
         uint32_t key = (uint32_t)((uintptr_t)addr);
         return ((key >> 3) * 2654435761u) % Length;