Closed
Bug 584650
Opened 14 years ago
Closed 14 years ago
TM: "Assertion failure: thing,"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta4+ |
status1.9.2 | --- | unaffected |
status1.9.1 | --- | unaffected |
People
(Reporter: gkw, Assigned: luke)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [sg:critical?][ccbr] fixed-in-tracemonkey [critsmash:patch])
Attachments
(1 file)
5.47 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
x = (evalcx('lazy'))
x.watch("", function () {})
gczeal(1)
for (w in x) {}
asserts js debug shell on JM changeset 6347cf00d3ab with -m at Assertion failure: thing, at ../jsgc.cpp:2177 if passed in as a CLI argument.
This asserts on tm-tip as well.
No longer blocks: JaegerFuzz
Summary: JM: "Assertion failure: thing," → TM: "Assertion failure: thing,"
Updated•14 years ago
|
Blocks: compartments
Reporter | ||
Comment 2•14 years ago
|
||
Locking s-s because gczeal is involved.
I double checked and indeed this seems to be the case, that it's also on TM.
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: 48355:babf32a45349
user: Andreas Gal
date: Wed Jul 28 11:20:19 2010 -0700
summary: Back out bug 580803.
I'm not sure if it's indeed related to the changeset or not.
Group: core-security
blocking2.0: --- → ?
Comment 3•14 years ago
|
||
Its a backout. Try to dig a bit further.
Reporter | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> Its a backout. Try to dig a bit further.
This is probably more plausible:
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: 47546:9c869e64ee26
user: Luke Wagner
date: Wed Jul 14 23:19:36 2010 -0700
summary: Bug 549143 - fatvals
Blocks: fatvals
Assignee | ||
Comment 5•14 years ago
|
||
Awesome find! The problem is that AutoValueVector/AutoIdVector don't make elements added by resize() GC-safe.
Reporter | ||
Updated•14 years ago
|
No longer blocks: compartments
Comment 6•14 years ago
|
||
All hail the Fuzzer! Oh and Gary too ;-).
/be
Updated•14 years ago
|
blocking2.0: ? → beta4+
Updated•14 years ago
|
Comment 7•14 years ago
|
||
Comment on attachment 463589 [details] [diff] [review]
patch
>diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h
>+MakeValueRangeGCSafe(Value *vec, size_t len)
>+MakeValueRangeGCSafe(Value *beg, Value *end)
>+MakeIdRangeGCSafe(jsid *beg, jsid *end)
>+MakeIdRangeGCSafe(jsid *vec, size_t len)
>+SetValueRangeToUndefined(Value *beg, Value *end)
>+SetValueRangeToUndefined(Value *vec, size_t len)
>+SetValueRangeToNull(Value *beg, Value *end)
>+SetValueRangeToNull(Value *vec, size_t len)
These would seem more sensible in jsvalue.h.
>+ /* N.B. Value's default ctor leaves the Value udnefined */
Pervasive typo, and put a period at the end of this complete sentence.
> bool resize(size_t newLength) {
>- return vector.resize(newLength);
>+ size_t oldLength = vector.length();
>+ if (newLength <= oldLength)
>+ vector.shrinkBy(oldLength - newLength);
>+ /* N.B. Value's default ctor leaves the Value udnefined */
>+ if (!vector.growByUninitialized(newLength - oldLength))
>+ return false;
>+ MakeValueRangeGCSafe(vector.begin() + oldLength, vector.end());
>+ return true;
> }
Umm, this looks...wrong. Are you missing a return true in the <= case?
> bool resize(size_t newLength) {
>- return vector.resize(newLength);
>+ size_t oldLength = vector.length();
>+ if (newLength <= oldLength)
>+ vector.shrinkBy(oldLength - newLength);
>+ /* N.B. jsid's default ctor leaves the jsid udnefined */
>+ if (!vector.growByUninitialized(newLength - oldLength))
>+ return false;
>+ MakeIdRangeGCSafe(vector.begin() + oldLength, vector.end());
>+ return true;
> }
Same.
r=me if you agree all that's missing is a return true in the <= cases, otherwise poke me again.
Attachment #463589 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 8•14 years ago
|
||
Thanks for pointing out the thinko!
http://hg.mozilla.org/tracemonkey/rev/bd2a104ad837
and, because I didn't notice all the comments at first
http://hg.mozilla.org/tracemonkey/rev/0714e6bd36be
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Updated•14 years ago
|
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?][ccbr] fixed-in-tracemonkey [critsmash:patch]
Comment 9•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Group: core-security
Comment 10•13 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•12 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug584650.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•