Closed
Bug 514999
Opened 16 years ago
Closed 16 years ago
TM: "Assertion failure: thing, at ../jsgc.cpp"
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
| Tracking | Status | |
|---|---|---|
| status1.9.2 | --- | beta1-fixed |
| blocking1.9.1 | --- | .6+ |
| status1.9.1 | --- | .6-fixed |
| fennec | 1.0+ | --- |
People
(Reporter: gkw, Assigned: gal)
References
Details
(5 keywords, Whiteboard: fixed-in-tracemonkey [sg:critical])
Attachments
(2 files, 1 obsolete file)
|
11.70 KB,
patch
|
mrbkap
:
review+
dveditz
:
approval1.9.1.6+
|
Details | Diff | Splinter Review |
|
9.15 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(function () {
(eval("\
(function () {\
for (var y = 0; y < 16; ++y) {\
if (y % 3 == 2) {\
gczeal(1);\
} else {\
print(0 / 0);\
}\
}\
});\
"))()
})();
asserts js debug shell with -j at Assertion failure: thing, at ../jsgc.cpp:2610
Brendan says (and I confirmed) that this is most probably related to bug 513981. Security-sensitive since bug 513981 is locked too.
Flags: blocking1.9.2?
| Reporter | ||
Comment 1•16 years ago
|
||
TM branch tip too.
| Assignee | ||
Comment 2•16 years ago
|
||
Awesome fuzzer work Gary. This is really bad news. Its very likely 513981 as you suspected.
| Assignee | ||
Comment 3•16 years ago
|
||
It doesn't crash if I use TMFLAGS=full. This is going to be fun.
Comment 4•16 years ago
|
||
(In reply to comment #2)
> Awesome fuzzer work Gary. This is really bad news. Its very likely 513981 as
> you suspected.
As *I* suspect :-P. See bug 514819 comment 28.
/be
| Reporter | ||
Comment 5•16 years ago
|
||
(In reply to comment #0)
> Brendan says (and I confirmed) that this is most probably related to bug
> 513981. Security-sensitive since bug 513981 is locked too.
(In reply to comment #4)
> (In reply to comment #2)
> > Awesome fuzzer work Gary. This is really bad news. Its very likely 513981 as
> > you suspected.
>
> As *I* suspect :-P. See bug 514819 comment 28.
>
> /be
Yeah, I (or autoBisect) only get partial credit, full credit goes out to Brendan. ;-)
| Assignee | ||
Comment 6•16 years ago
|
||
Assignee: general → gal
Attachment #399034 -
Flags: review?(mrbkap)
| Assignee | ||
Comment 7•16 years ago
|
||
I concur with blocking and this is also needed for the 3.5 branch.
| Assignee | ||
Updated•16 years ago
|
tracking-fennec: --- → ?
| Assignee | ||
Updated•16 years ago
|
Attachment #399034 -
Attachment is patch: true
Attachment #399034 -
Attachment mime type: application/octet-stream → text/plain
Attachment #399034 -
Flags: approval1.9.1.4?
| Assignee | ||
Comment 8•16 years ago
|
||
sayrer, this and the other patch should go on trunk to make asap (assuming I get review for this and tryserver doesn't hate me)
| Assignee | ||
Comment 9•16 years ago
|
||
bake, not make
Updated•16 years ago
|
blocking1.9.1: --- → ?
Updated•16 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Updated•16 years ago
|
Attachment #399034 -
Flags: review?(mrbkap) → review+
| Assignee | ||
Comment 10•16 years ago
|
||
I am getting a trace-test failure with the patch. Investigating.
Comment 11•16 years ago
|
||
This is not a security issue per se tho right? Trying to figure out sg: rating.
Comment 12•16 years ago
|
||
I'd rate this as sg:critical. At the very least, if this particular bug is checked in as-is, it leaves a sg:critical bug in its wake.
| Assignee | ||
Comment 13•16 years ago
|
||
We can re-enter the interpreter while an outer use of nativeVp is still active, so we have to move nativeVp from cx to InterpState (debugged by Blake).
Attachment #399034 -
Attachment is obsolete: true
Attachment #399034 -
Flags: approval1.9.1.4?
| Assignee | ||
Updated•16 years ago
|
Attachment #399641 -
Flags: review?(mrbkap)
Updated•16 years ago
|
Attachment #399641 -
Flags: review?(mrbkap) → review+
Comment 14•16 years ago
|
||
Comment on attachment 399641 [details] [diff] [review]
patch
Looks good.
| Assignee | ||
Comment 15•16 years ago
|
||
Whiteboard: fixed-in-tracemonkey
| Assignee | ||
Updated•16 years ago
|
Attachment #399641 -
Flags: approval1.9.1.4?
Updated•16 years ago
|
tracking-fennec: ? → 1.0+
Comment 16•16 years ago
|
||
Busted trace tests, gczeal is not defined in opt build. Fixing.
Comment 17•16 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/dad8ab8cb1dd should have fixed it, back in a bit to confirm.
| Assignee | ||
Comment 18•16 years ago
|
||
Thanks graydon.
Comment 19•16 years ago
|
||
We'll switch this to blocking the next specific 1.9.1.x after it lands on trunk
and 1.9.2 successfully.
blocking1.9.1: ? → needed
status1.9.1:
--- → wanted
| Assignee | ||
Updated•16 years ago
|
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey [sg:critical]
Comment 20•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 21•16 years ago
|
||
Updated•16 years ago
|
Attachment #399641 -
Flags: approval1.9.1.4? → approval1.9.1.5?
Updated•16 years ago
|
Priority: -- → P1
Comment 22•16 years ago
|
||
status1.9.2:
--- → beta1-fixed
Updated•16 years ago
|
blocking1.9.1: needed → .5+
Comment 23•16 years ago
|
||
js/src/trace-test/tests/basic/testNativeArgsRooting.js
Flags: in-testsuite+
Updated•16 years ago
|
Flags: wanted1.9.0.x-
Comment 25•16 years ago
|
||
Comment on attachment 399641 [details] [diff] [review]
patch
Approved for 1.9.1.5, a=dveditz for release-drivers
Attachment #399641 -
Flags: approval1.9.1.5? → approval1.9.1.5+
Comment 26•15 years ago
|
||
andreas, this doesn't apply to 1.9.1, can you refresh it?
| Assignee | ||
Comment 27•15 years ago
|
||
Looking.
| Assignee | ||
Comment 28•15 years ago
|
||
I am going to have to drop parts of the patch that fix the problem for getters/setters (since thats not on 191). If we ever take the getters/setters patch on top of this patch, we will lose those changes, create a gc hazard, and shoot ourselves in the foot. Fair warning.
| Assignee | ||
Comment 29•15 years ago
|
||
Actually 1.9.1 looks quite a bit different than what we tested with. I am not sure I am comfortable dropping this into a release based on my manual rebasing only.
| Assignee | ||
Comment 30•15 years ago
|
||
Ok no wonder this doesn't work. 513981 is missing in between.
| Assignee | ||
Updated•15 years ago
|
| Assignee | ||
Comment 31•15 years ago
|
||
Ok on top of 513981 this looks manageable. Warning about getter/setter patch still applies. Patch in a sec.
| Assignee | ||
Comment 32•15 years ago
|
||
Attachment #410351 -
Flags: review?(mrbkap)
Updated•15 years ago
|
Attachment #410351 -
Flags: review?(mrbkap) → review+
| Assignee | ||
Comment 33•15 years ago
|
||
Blake, could you land this for me?
Comment 34•15 years ago
|
||
Comment 35•15 years ago
|
||
Bob, can you verify this on the 1.9.1 nightly?
Comment 36•15 years ago
|
||
v 1.9.1, testcase in comment 0 does not assert on 1.9.1 mac debug shell.
Keywords: verified1.9.1
Updated•15 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•