Closed
Bug 584650
Opened 13 years ago
Closed 13 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•13 years ago
|
Blocks: compartments
![]() |
Reporter | |
Comment 2•13 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•13 years ago
|
||
Its a backout. Try to dig a bit further.
![]() |
Reporter | |
Comment 4•13 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•13 years ago
|
||
Awesome find! The problem is that AutoValueVector/AutoIdVector don't make elements added by resize() GC-safe.
![]() |
Reporter | |
Updated•13 years ago
|
No longer blocks: compartments
Comment 6•13 years ago
|
||
All hail the Fuzzer! Oh and Gary too ;-). /be
Updated•13 years ago
|
blocking2.0: ? → beta4+
Updated•13 years ago
|
Comment 7•13 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•13 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•13 years ago
|
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?][ccbr] fixed-in-tracemonkey [critsmash:patch]
Comment 9•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/0714e6bd36be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Group: core-security
Comment 10•11 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•10 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
•