Bug 1019182 - ProfileEntry flags should be zeroed when setting frame data, r=djvj
authorVictor Porof <vporof@mozilla.com>
Wed, 04 Jun 2014 14:37:49 -0400
changeset 205802 d185d33cfce5abbbb03157af9f471570478476c3
parent 205801 b2b3a983b1a901958e4a210bf00adf84711500f6
child 205803 748b4ffb759e97e4495d6f92948c9f8fed585155
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1019182
milestone32.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 1019182 - ProfileEntry flags should be zeroed when setting frame data, r=djvj
js/public/ProfilingStack.h
js/src/vm/SPSProfiler.cpp
tools/profiler/PseudoStack.h
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -42,21 +42,19 @@ class ProfileEntry
 
     // Stack pointer for non-JS entries, the script pointer otherwise.
     void * volatile spOrScript;
 
     // Line number for non-JS entries, the bytecode offset otherwise.
     int32_t volatile lineOrPc;
 
     // General purpose storage describing this frame.
-    uint32_t volatile flags;
+    uint32_t volatile flags_;
 
   public:
-    ProfileEntry(void) : flags(0) {}
-
     // These traits are bit masks. Make sure they're powers of 2.
     enum Flags {
         // Indicate whether a profile entry represents a CPP frame. If not set,
         // a JS frame is assumed by default. You're not allowed to publicly
         // change the frame type. Instead, call `setJsFrame` or `setCppFrame`.
         IS_CPP_ENTRY = 0x01,
 
         // Indicate that copying the frame label is not necessary when taking a
@@ -88,36 +86,39 @@ class ProfileEntry
     bool isJs() const volatile { return !isCpp(); }
 
     bool isCopyLabel() const volatile { return hasFlag(FRAME_LABEL_COPY); };
 
     void setLabel(const char *aString) volatile { string = aString; }
     const char *label() const volatile { return string; }
 
     void setJsFrame(JSScript *aScript, jsbytecode *aPc) volatile {
-        flags &= ~IS_CPP_ENTRY;
+        flags_ = 0;
         spOrScript = aScript;
         setPC(aPc);
     }
     void setCppFrame(void *aSp, uint32_t aLine) volatile {
-        flags |= IS_CPP_ENTRY;
+        flags_ = IS_CPP_ENTRY;
         spOrScript = aSp;
         lineOrPc = static_cast<int32_t>(aLine);
     }
 
     void setFlag(uint32_t flag) volatile {
         MOZ_ASSERT(flag != IS_CPP_ENTRY);
-        flags |= flag;
+        flags_ |= flag;
     }
     void unsetFlag(uint32_t flag) volatile {
         MOZ_ASSERT(flag != IS_CPP_ENTRY);
-        flags &= ~flag;
+        flags_ &= ~flag;
     }
     bool hasFlag(uint32_t flag) const volatile {
-        return bool(flags & uint32_t(flag));
+        return bool(flags_ & flag);
+    }
+    uint32_t flags() const volatile {
+        return flags_;
     }
 
     void *stackAddress() const volatile {
         MOZ_ASSERT(!isJs());
         return spOrScript;
     }
     JSScript *script() const volatile {
         MOZ_ASSERT(isJs());
@@ -135,17 +136,17 @@ class ProfileEntry
     // The offset of a pc into a script's code can actually be 0, so to
     // signify a nullptr pc, use a -1 index. This is checked against in
     // pc() and setPC() to set/get the right pc.
     static const int32_t NullPCOffset = -1;
 
     static size_t offsetOfLabel() { return offsetof(ProfileEntry, string); }
     static size_t offsetOfSpOrScript() { return offsetof(ProfileEntry, spOrScript); }
     static size_t offsetOfLineOrPc() { return offsetof(ProfileEntry, lineOrPc); }
-    static size_t offsetOfFlags() { return offsetof(ProfileEntry, flags); }
+    static size_t offsetOfFlags() { return offsetof(ProfileEntry, flags_); }
 };
 
 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/vm/SPSProfiler.cpp
+++ b/js/src/vm/SPSProfiler.cpp
@@ -210,16 +210,17 @@ SPSProfiler::enterNative(const char *str
     volatile ProfileEntry *stack = stack_;
     volatile uint32_t *size = size_;
     uint32_t current = *size;
 
     JS_ASSERT(enabled());
     if (current < max_) {
         stack[current].setLabel(string);
         stack[current].setCppFrame(sp, 0);
+        JS_ASSERT(stack[current].flags() == js::ProfileEntry::IS_CPP_ENTRY);
     }
     *size = current + 1;
 }
 
 void
 SPSProfiler::push(const char *string, void *sp, JSScript *script, jsbytecode *pc, bool copy)
 {
     JS_ASSERT_IF(sp != nullptr, script == nullptr && pc == nullptr);
@@ -230,20 +231,24 @@ SPSProfiler::push(const char *string, vo
     volatile uint32_t *size = size_;
     uint32_t current = *size;
 
     JS_ASSERT(installed());
     if (current < max_) {
         volatile ProfileEntry &entry = stack[current];
         entry.setLabel(string);
 
-        if (sp != nullptr)
+        if (sp != nullptr) {
             entry.setCppFrame(sp, 0);
-        else
+            JS_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY);
+        }
+        else {
             entry.setJsFrame(script, pc);
+            JS_ASSERT(entry.flags() == 0);
+        }
 
         // Track if mLabel needs a copy.
         if (copy)
             entry.setFlag(js::ProfileEntry::FRAME_LABEL_COPY);
         else
             entry.unsetFlag(js::ProfileEntry::FRAME_LABEL_COPY);
     }
     *size = current + 1;
--- a/tools/profiler/PseudoStack.h
+++ b/tools/profiler/PseudoStack.h
@@ -357,16 +357,17 @@ public:
     }
 
     volatile StackEntry &entry = mStack[mStackPointer];
 
     // Make sure we increment the pointer after the name has
     // been written such that mStack is always consistent.
     entry.setLabel(aName);
     entry.setCppFrame(aStackAddress, line);
+    MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY);
 
     uint32_t uint_category = static_cast<uint32_t>(aCategory);
     MOZ_ASSERT(
       uint_category >= static_cast<uint32_t>(js::ProfileEntry::Category::FIRST) &&
       uint_category <= static_cast<uint32_t>(js::ProfileEntry::Category::LAST));
 
     entry.setFlag(uint_category);