Assertion failure: ~x > x (can't round up -- will overflow!), at dist/include/mozilla/MathAlgorithms.h

VERIFIED FIXED in Firefox 25

Status

()

defect
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Assigned: Waldo)

Tracking

(Blocks 1 bug, {assertion, regression, testcase})

Trunk
mozilla25
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox24 unaffected, firefox25 fixed)

Details

Attachments

(3 attachments)

Posted file stack
Object.defineProperty(this, "p0", {
    get: function() {
        return p2.partition(1);
    }
});
p2 = new ParallelArray([13266], function(x0) {
    return (gcPreserveCode());
});
p1 = p0.filter(function(e, i, s) {
    return i % 2;
});
uneval(p1);

asserts js debug shell on m-c changeset 0d0263a58f06 without any CLI arguments at Assertion failure: ~x > x (can't round up -- will overflow!), at dist/include/mozilla/MathAlgorithms.h

Setting s-s because this seems to involve gc via gcPreserveCode.

This reproduced with a 32-bit threadsafe shell on Linux (I think both deterministic / non-deterministic), but possibly not 64-bit.

Will be running bisection - assuming ParallelArrays are involved for now.
Flags: needinfo?(shu)
Like the other GC fuzz test, I can't reproduce this. 32bit or 64bit, --enable-debug --enable-optimize, --enable-threadsafe. :/
Flags: needinfo?(shu)
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/021fd4e03439
user:        Jeff Walden
date:        Wed Jul 03 15:46:51 2013 -0700
summary:     Bug 891177 - Move leading/trailing-zero-bit counting functions, ceiling/floor log2 functions, and round-up-pow2 functions into MathAlgorithms.h.  r=terrence

Waldo, do you think you could take a look and see if bug 891177 is related?
Blocks: 891177
Flags: needinfo?(jwalden+bmo)
See Also: → 895864
The failing assertion is a new one, added in that bug, to track a requirement that wasn't tested before.  The problem is that the Vector code implicated by that stack isn't using RoundUpPow2 correctly.  I'm pretty sure some sort of 32-bit Vector unit test could reproduce the assertion pretty easily.  I'll try to take a look into this soon-ish, but no guarantees, as this is a bit off my current path now.  I don't know whether a failure there has any security implications.  It might, or it might not -- I'd have to investigate the Vector code a bit more before I could tell.
Assignee: general → jwalden+bmo
Flags: needinfo?(jwalden+bmo)
Not security-sensitive, just a mistaken assertion written by blindly transcribing an incorrect precondition given in a nearby comment -- the actual code works correctly if you fall through the assertion.
Group: core-security
Component: JavaScript Engine → MFBT
Comment on attachment 781910 [details] [diff] [review]
Fix RoundUpPow2's required precondition to not be wrong

Review of attachment 781910 [details] [diff] [review]:
-----------------------------------------------------------------

Righteous!
r=me
Attachment #781910 - Flags: review?(terrence) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a126050ecdb
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla25
https://hg.mozilla.org/mozilla-central/rev/4a126050ecdb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 895864
Verified the fix on Ubuntu 13.04 by running the test from the bug description on Spidermonkey built from the latest mozilla-beta-5916e01e5a0a source: no assertion occurs but instead, I get the Reference Error: ParallelArray is not defined. Is this expected?
Flags: needinfo?(jwalden+bmo)
(In reply to Simona B [QA] from comment #10)
> Verified the fix on Ubuntu 13.04 by running the test from the bug
> description on Spidermonkey built from the latest mozilla-beta-5916e01e5a0a
> source: no assertion occurs but instead, I get the Reference Error:
> ParallelArray is not defined. Is this expected?

ParallelArrays, as far as I know, are only enabled on Nightly(?) for now, but I'm not sure so I'd defer to Shu-yu.
Flags: needinfo?(jwalden+bmo) → needinfo?(shu)
(In reply to Gary Kwong [:gkw] [:nth10sd] (still catching up on bugmail) from comment #11)
> (In reply to Simona B [QA] from comment #10)
> > Verified the fix on Ubuntu 13.04 by running the test from the bug
> > description on Spidermonkey built from the latest mozilla-beta-5916e01e5a0a
> > source: no assertion occurs but instead, I get the Reference Error:
> > ParallelArray is not defined. Is this expected?
> 
> ParallelArrays, as far as I know, are only enabled on Nightly(?) for now,
> but I'm not sure so I'd defer to Shu-yu.

That's correct, PJS (the now defunct by unremoved ParallelArray API, Array.prototype.*Par methods, and the upcoming Par methods on typed objects) are ifdef'd in only in Nightly.
Flags: needinfo?(shu)
now defunct but unremoved*
Simona, you might have to retest on a later build from mozilla-central, since Parallel JS is only enabled by ifdef in Nightly for now.
Flags: needinfo?(simona.marcu)
(In reply to Gary Kwong [:gkw] [:nth10sd] (still catching up on bugmail) from comment #14)
> Simona, you might have to retest on a later build from mozilla-central,
> since Parallel JS is only enabled by ifdef in Nightly for now.

On the latest Nightly I don't get the same reference error as I do on the latest beta but, instead I get: "uncaught exception: out of memory".
Any thoughts?
Flags: needinfo?(simona.marcu)
I think that means it's fixed. I suppose I'm right, Waldo?
Flags: needinfo?(jwalden+bmo)
I don't especially understand the original testcase enough to say what it should or shouldn't do.  But given there was an automated test in the original patch, that would have detected the original issue, and this wasn't a security issue, I don't think this benefits from manual QA resources, or worrying about the expected behavior of the original testcase.
Flags: needinfo?(jwalden+bmo)
Marking VERIFIED per comment 17 as well as the fact that a test was landed.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.