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 186580 d185d33cfce5abbbb03157af9f471570478476c3
parent 186579 b2b3a983b1a901958e4a210bf00adf84711500f6
child 186581 748b4ffb759e97e4495d6f92948c9f8fed585155
push idunknown
push userunknown
push dateunknown
reviewersdjvj
bugs1019182
milestone32.0a1
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);