Skip nativeStackAddrs that conflict with pseudoStackAddrs and jsStackAddrs in mergeStacksIntoProfile

RESOLVED FIXED in Firefox 39

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: djvj, Assigned: djvj)

Tracking

unspecified
Firefox 39
x86_64
Windows 8.1
Points:
---

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Sometimes, in mergeStacksIntoProfile in TableTicker.cpp, we can get native stack addrs that are the same as a pseudstack addr, or a js stack addr.

Currently, we assert that this does not happen, but it can happen in rare cases.  We should handle this case instead of just asserting it, and give preference to pseudostack or jsstack interpretations over the nativestack interpretation.
(Assignee)

Updated

3 years ago
Blocks: 1141712
QA Contact: kvijayan
(Assignee)

Comment 1

3 years ago
Created attachment 8580251 [details] [diff] [review]
skip-conflicting-native-stackaddrs.patch
Attachment #8580251 - Flags: review?(mstange)
(Assignee)

Comment 2

3 years ago
Created attachment 8580253 [details] [diff] [review]
skip-conflicting-native-stackaddrs.patch

Updated patch with comment.
Attachment #8580251 - Attachment is obsolete: true
Attachment #8580251 - Flags: review?(mstange)
Attachment #8580253 - Flags: review?(mstange)
Attachment #8580253 - Flags: review?(mstange) → review+
Duplicate of this bug: 1136384
Even with that in place, I've seen occasional assertion failures further down,
here:

    // If we reach here, there must be a native stack entry and it must be the
    // greatest entry.
    MOZ_ASSERT(nativeStackAddr);  <----------------- HERE
    MOZ_ASSERT(nativeIndex >= 0);
    aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
    nativeIndex--;

I suspect it is possible to get to this point with nativeStackAddr,
jsStackAddr, pseudoStackAddr all being zero.  Does that sound
plausible?  If we have all 3 being zero, then this doesn't apply

    // Check to see if pseudoStack frame is top-most.
    if (pseudoStackAddr > jsStackAddr && pseudoStackAddr > nativeStackAddr) {

and neither does this

    // Check to see if JS jit stack frame is top-most
    if (jsStackAddr > nativeStackAddr) {

so we wind up failing here:

    MOZ_ASSERT(nativeStackAddr);

It's very difficult to reproduce, though -- I've only seen it about
three or four times.  So I can't yet confirm the all-3-values-zero
hypothesis.
(In reply to Julian Seward [:jseward] from comment #4)
> I suspect it is possible to get to this point with nativeStackAddr,
> jsStackAddr, pseudoStackAddr all being zero.

Verified:

    // If we reach here, there must be a native stack entry and it must be the
    // greatest entry.
if (!nativeStackAddr)
fprintf(stderr, "XXXXXXXXXXXXXXXXXXXXX %p %p %p\n", nativeStackAddr, jsStackAddr, pseudoStackAddr);
    MOZ_ASSERT(nativeStackAddr);

produces

XXXXXXXXXXXXXXXXXXXXX (nil) (nil) (nil)
Assertion failure: nativeStackAddr, at /home/sewardj/MOZ/MC-PROF/tools/profiler/TableTicker.cpp:626
Any opinions about this as a fix?

     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
-    MOZ_ASSERT(nativeStackAddr);
-    MOZ_ASSERT(nativeIndex >= 0);
-    aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
-    nativeIndex--;
+    if (nativeStackAddr) {
+      MOZ_ASSERT(nativeIndex >= 0);
+      aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
+    }
+    if (nativeIndex >= 0) {
+      nativeIndex--;
+    }
https://hg.mozilla.org/mozilla-central/rev/3abdf236d4f2
Assignee: nobody → kvijayan
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
You need to log in before you can comment on or make changes to this bug.