Bug 1120677 - Fix GetPcScript to ignore BaselineFrames with an override pc. r=shu
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 20 Jan 2015 17:36:39 +0100
changeset 224727 491f9b6be0eecec3db1ba90d6bb792a3482710dc
parent 224726 7baad81da308226248009a7f33b6a3f298ba4e12
child 224728 b768bbcfa5465d252d8faa16e4427a3e1f8bd507
push id28143
push userryanvm@gmail.com
push dateWed, 21 Jan 2015 03:14:12 +0000
treeherdermozilla-central@540077a30866 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1120677
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1120677 - Fix GetPcScript to ignore BaselineFrames with an override pc. r=shu
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
@@ -327,23 +327,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
@@ -155,16 +155,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
@@ -1471,73 +1471,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;