Closed Bug 1022081 Opened 6 years ago Closed 6 years ago

Assertion failure: containsPC(pc), at jsscript.h

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: gkw, Assigned: h4writer)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker][jsbugmon:update])

Attachments

(2 files)

Attached file stack
+++ This bug was initially created as a clone of Bug #1011730 +++

function f() {
    (function() {
        gc()
    })()
}
enableSPSProfiling()
f()
f()

asserts js debug shell on m-c changeset 62d33e3ba514 with --ion-eager --ion-parallel-compile=off at Assertion failure: containsPC(pc), at jsscript.h

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a0bb2b2da6ef
user:        Hannes Verschore
date:        Mon Jun 02 23:28:44 2014 +0200
summary:     Bug 844779 - Part 1: Remove inline function tracking in ionmonkey, r=djvj

Hannes, is bug 844779 a likely regressor?

Because this involves SPS, setting s-s and assuming sec-high for now. Also, this is flooding jsfunfuzz, so setting [fuzzblocker].
Flags: needinfo?(hv1989)
Yes, this is the likely regressor. Looks like I forgot another edgecase :(.
Looking into it. I should normally have a patch before the end of the weekend.
Flags: needinfo?(hv1989)
I will be so happy when we can just rip this all out.
Understanding this better this must be working by accident before I landed my patch.

Now during "GenerateOOLCode" we adjust the script (possibly to an inlined frame). Really we adjust the stack to contain [inlined frame][OOL Call]. Everything else is discarded. But we don't reset the script to be in the state like before.

When we "Link" the code we call "handleFailure", which does SPSFrame. This checks if the pc is in the top frame. The pc is taken as the last executed pc (in the toplevel script). But now the script has possibly been adjusted to the inlined script. So they don't match.

The fix is to restore the topScript after GenerateOOLCode. That way they correspond again. IIUC the assert is just a sanity check, since the pc isn't used during linking. So the generated code is still the same... (So I think this is not a security bug).
Assignee: nobody → hv1989
Attachment #8436463 - Flags: review?(kvijayan)
Attachment #8436463 - Flags: review?(kvijayan) → review+
(In reply to Hannes Verschore [:h4writer] from comment #3)
> The fix is to restore the topScript after GenerateOOLCode. That way they
> correspond again. IIUC the assert is just a sanity check, since the pc isn't
> used during linking. So the generated code is still the same... (So I think
> this is not a security bug).

Opening up...
Group: core-security, javascript-core-security
Keywords: sec-high
Hannes, is this ready for landing? Also, since this isn't a security bug, please land the testcase as well.
Flags: needinfo?(hv1989)
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf4180bf9f0

Sorry about the delay.
Flags: needinfo?(hv1989)
https://hg.mozilla.org/mozilla-central/rev/fcf4180bf9f0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.