Bug 783030: Prevent the profiled line number from incorrectly pointing to the start. r=bhackett
authorAlex Crichton <acrichton@mozilla.com>
Wed, 15 Aug 2012 16:56:08 -0700
changeset 108130 a0cf81efe4aa940dccc4c06b8fbe215ec71771c3
parent 108129 e8427646099454cfb6beeba73e27b595ca65daf6
child 108131 ea2ad8970f3e50e2ef2685ef568316448aae719a
push id1490
push userakeybl@mozilla.com
push dateMon, 08 Oct 2012 18:29:50 +0000
treeherdermozilla-beta@f335e7dacdc1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs783030
milestone17.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 783030: Prevent the profiled line number from incorrectly pointing to the start. r=bhackett
js/src/jsfriendapi.h
js/src/methodjit/BaseAssembler.h
js/src/vm/SPSProfiler.cpp
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -572,17 +572,17 @@ class ProfileEntry
      * A ProfileEntry represents both a C++ profile entry and a JS one. Both use
      * the string as a description, but JS uses the sp as NULL to indicate that
      * it is a JS entry. The script_ is then only ever examined for a JS entry,
      * and the idx is used by both, but with different meanings.
      */
     const char * volatile string; // Descriptive string of this entry
     void * volatile sp;           // Relevant stack pointer for the entry
     JSScript * volatile script_;  // if js(), non-null script which is running
-    uint32_t volatile idx;        // if js(), idx of pc, otherwise line number
+    int32_t volatile idx;         // if js(), idx of pc, otherwise line number
 
   public:
     /*
      * All of these methods are marked with the 'volatile' keyword because SPS's
      * representation of the stack is stored such that all ProfileEntry
      * instances are volatile. These methods would not be available unless they
      * were marked as volatile as well
      */
@@ -605,16 +605,23 @@ class ProfileEntry
     /* we can't know the layout of JSScript, so look in vm/SPSProfiler.cpp */
     JS_FRIEND_API(jsbytecode *) pc() volatile;
     JS_FRIEND_API(void) setPC(jsbytecode *pc) volatile;
 
     static size_t offsetOfString() { return offsetof(ProfileEntry, string); }
     static size_t offsetOfStackAddress() { return offsetof(ProfileEntry, sp); }
     static size_t offsetOfPCIdx() { return offsetof(ProfileEntry, idx); }
     static size_t offsetOfScript() { return offsetof(ProfileEntry, script_); }
+
+    /*
+     * The index used in the entry can either be a line number or the offset of
+     * a pc into a script's code. To signify a NULL pc, use a -1 index. This is
+     * checked against in pc() and setPC() to set/get the right pc.
+     */
+    static const int32_t NullPCIndex = -1;
 };
 
 JS_FRIEND_API(void)
 SetRuntimeProfilingStack(JSRuntime *rt, ProfileEntry *stack, uint32_t *size,
                          uint32_t max);
 
 JS_FRIEND_API(void)
 EnableRuntimeProfilingStack(JSRuntime *rt, bool enabled);
--- a/js/src/methodjit/BaseAssembler.h
+++ b/js/src/methodjit/BaseAssembler.h
@@ -1529,29 +1529,30 @@ static const JSC::MacroAssembler::Regist
         JS_STATIC_ASSERT(sizeof(ProfileEntry) == 4 * sizeof(void*));
         // 4 * sizeof(void*) * idx = idx << (2 + log(sizeof(void*)))
         lshift32(Imm32(2 + (sizeof(void*) == 4 ? 2 : 3)), reg);
         addPtr(ImmPtr(p->stack()), reg);
         return j;
     }
 
   public:
-    void spsUpdatePCIdx(SPSProfiler *p, uint32_t idx, RegisterID reg) {
+    void spsUpdatePCIdx(SPSProfiler *p, int32_t idx, RegisterID reg) {
         Jump j = spsProfileEntryAddress(p, -1, reg);
         store32(Imm32(idx), Address(reg, ProfileEntry::offsetOfPCIdx()));
         j.linkTo(label(), this);
     }
 
     void spsPushFrame(SPSProfiler *p, const char *str, JSScript *s, RegisterID reg) {
         Jump j = spsProfileEntryAddress(p, 0, reg);
 
         storePtr(ImmPtr(str),  Address(reg, ProfileEntry::offsetOfString()));
         storePtr(ImmPtr(s),    Address(reg, ProfileEntry::offsetOfScript()));
         storePtr(ImmPtr(NULL), Address(reg, ProfileEntry::offsetOfStackAddress()));
-        store32(Imm32(0),      Address(reg, ProfileEntry::offsetOfPCIdx()));
+        store32(Imm32(ProfileEntry::NullPCIndex),
+                Address(reg, ProfileEntry::offsetOfPCIdx()));
 
         /* Always increment the stack size, regardless if we actually pushed */
         j.linkTo(label(), this);
         add32(Imm32(1), AbsoluteAddress(p->size()));
     }
 
     void spsPopFrame(SPSProfiler *p) {
         sub32(Imm32(1), AbsoluteAddress(p->size()));
@@ -1631,17 +1632,17 @@ SPSInstrumentation::push(JSContext *cx, 
  */
 inline void
 SPSInstrumentation::pushManual(Assembler &masm, RegisterID scratch)
 {
     JS_ASSERT(!frame->pushed);
     JS_ASSERT(frame->left == 0);
     if (!enabled())
         return;
-    masm.spsUpdatePCIdx(profiler_, 0, scratch);
+    masm.spsUpdatePCIdx(profiler_, ProfileEntry::NullPCIndex, scratch);
     frame->pushed = true;
 }
 
 /*
  * Signals that the current function is leaving for a function call. This can
  * happen both on JS function calls and also calls to C++. This internally
  * manages how many leave() calls have been seen, and only the first leave()
  * emits instrumentation. Similarly, only the last corresponding reenter()
@@ -1661,17 +1662,17 @@ SPSInstrumentation::leave(Assembler &mas
 inline void
 SPSInstrumentation::reenter(Assembler &masm, RegisterID scratch)
 {
     if (!enabled() || !frame->pushed || frame->left-- != 1)
         return;
     if (frame->skipNext)
         frame->skipNext = false;
     else
-        masm.spsUpdatePCIdx(profiler_, 0, scratch);
+        masm.spsUpdatePCIdx(profiler_, ProfileEntry::NullPCIndex, scratch);
 }
 
 /*
  * Signifies exiting a JS frame, popping the SPS entry. Because there can be
  * multiple return sites of a function, this does not cease instrumentation
  * emission.
  */
 inline void
--- a/js/src/vm/SPSProfiler.cpp
+++ b/js/src/vm/SPSProfiler.cpp
@@ -141,18 +141,17 @@ SPSProfiler::push(const char *string, vo
     volatile uint32_t *size = size_;
     uint32_t current = *size;
 
     JS_ASSERT(enabled());
     if (current < max_) {
         stack[current].setLabel(string);
         stack[current].setStackAddress(sp);
         stack[current].setScript(script);
-        if (pc != NULL)
-            stack_[current].setPC(pc);
+        stack[current].setPC(pc);
     }
     *size = current + 1;
 }
 
 void
 SPSProfiler::pop()
 {
     JS_ASSERT(installed());
@@ -399,15 +398,15 @@ SPSEntryMarker::SPSEntryMarker(JSRuntime
 SPSEntryMarker::~SPSEntryMarker()
 {
     if (profiler != NULL)
         profiler->pop();
 }
 
 JS_FRIEND_API(jsbytecode*)
 ProfileEntry::pc() volatile {
-    return script()->code + idx;
+    return idx == NullPCIndex ? NULL : script()->code + idx;
 }
 
 JS_FRIEND_API(void)
 ProfileEntry::setPC(jsbytecode *pc) volatile {
-    idx = pc - script()->code;
+    idx = pc == NULL ? NullPCIndex : pc - script()->code;
 }