Closed
Bug 846984
Opened 11 years ago
Closed 11 years ago
Crash [@ js_NewStringCopyN] with a threadsafe js shell
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla22
Tracking | Status | |
---|---|---|
firefox19 | --- | unaffected |
firefox20 | --- | unaffected |
firefox21 | + | fixed |
firefox22 | + | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | + | affected |
b2g18-v1.0.0 | --- | wontfix |
b2g18-v1.0.1 | --- | affected |
People
(Reporter: gkw, Assigned: Benjamin)
References
Details
(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:])
Crash Data
Attachments
(2 files)
32.71 KB,
text/plain
|
Details | |
1.10 KB,
patch
|
jorendorff
:
review+
gkw
:
feedback+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
for (var y = 0; y < 999999; y++) { evalcx("print(x);function x(...x){}", newGlobal('')) } crashes js debug and opt shell on m-c changeset 3362afba690e without any CLI arguments at js_NewStringCopyN
Reporter | ||
Comment 1•11 years ago
|
||
I just verified that this seems to only occur with threadsafe js shells.
Summary: Crash [@ js_NewStringCopyN] → Crash [@ js_NewStringCopyN] with a threadsafe js shell
Reporter | ||
Comment 2•11 years ago
|
||
And I also verified that 32-bit and 64-bit Windows shells are affected, but Linux and Mac shells do not seem to be affected.
Hardware: x86 → All
Summary: Crash [@ js_NewStringCopyN] with a threadsafe js shell → Crash [@ js_NewStringCopyN] with a threadsafe js shell on Windows
Updated•11 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Comment 3•11 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Reporter | ||
Comment 4•11 years ago
|
||
A faster testcase: for (var y = 0; y < 9999; y++) { evalcx("print(x);function x(...x){}", newGlobal('')) }
Reporter | ||
Comment 5•11 years ago
|
||
Even faster one: for (var y = 0; y < 999; y++) { evalcx("print(x);function x(...x){}", newGlobal('')) } autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 121521:79ebe1a5d8f4 user: Ms2ger date: Tue Feb 12 11:14:01 2013 +0100 summary: Bug 837176 - Simplify code flow in CheckSideEffects; r=jorendorff Ms2ger, is bug 837176 a likely regressor?
Blocks: 837176
status-b2g18:
--- → unaffected
status-b2g18-v1.0.0:
--- → unaffected
status-b2g18-v1.0.1:
--- → unaffected
status-firefox19:
--- → unaffected
status-firefox20:
--- → unaffected
status-firefox21:
--- → affected
status-firefox22:
--- → affected
status-firefox-esr17:
--- → unaffected
tracking-firefox21:
--- → ?
tracking-firefox22:
--- → ?
Comment 6•11 years ago
|
||
I would have hoped not... Jason?
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to :Ms2ger from comment #6) > I would have hoped not... Jason? I'm not so sure anymore either. :-/ It is an unstable testcase. I changed the for loop to 99999: for (var y = 0; y < 99999; y++) { evalcx("print(x);function x(...x){}", newGlobal('')) } and have restarted autoBisect. More tomorrow.
Reporter | ||
Comment 8•11 years ago
|
||
I have managed to get a second possible regressor from the testcase in comment 7: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 121159:71f14579265f user: Benjamin Peterson date: Thu Feb 07 09:29:22 2013 -0500 summary: Bug 836515 - Allow source compression to run while executing the script. r=jorendorff jorendorff, you're probably the best person to evaluate the regression ranges.
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 9•11 years ago
|
||
It's more likely related to 71f14579265f. Unfortunately, I don't have Windows, so I probably won't be able to debug this.
Flags: needinfo?(jorendorff)
Updated•11 years ago
|
Flags: needinfo?(jorendorff)
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 10•11 years ago
|
||
Assuming sec-critical until otherwise shown. (there are some indications from the stack that it might just be a null deref, or not.. so let's have someone look at this first)
Keywords: sec-critical
Updated•11 years ago
|
Assignee: general → jorendorff
Updated•11 years ago
|
status-b2g18-v1.0.0:
unaffected → ---
status-b2g18-v1.0.1:
unaffected → ---
status-firefox19:
unaffected → ---
Reporter | ||
Comment 11•11 years ago
|
||
Adjusting flags and nominating for tracking on b2g, apparently bug 836515 landed on b2g as a performance improvement.
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → affected
status-firefox19:
--- → unaffected
tracking-b2g18:
--- → ?
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #724238 -
Flags: review?(jorendorff)
Attachment #724238 -
Flags: feedback?(gary)
Updated•11 years ago
|
Reporter | ||
Comment 13•11 years ago
|
||
Comment on attachment 724238 [details] [diff] [review] tweak some defines This seems to fix the crash for me.
Attachment #724238 -
Flags: feedback?(gary) → feedback+
Comment 14•11 years ago
|
||
Comment on attachment 724238 [details] [diff] [review] tweak some defines Review of attachment 724238 [details] [diff] [review]: ----------------------------------------------------------------- This is fine, but if you think it's less error-prone to make all the compression *and* threading code conditional on having both USE_ZLIB and JS_THREADSAFE (thus supporting only two variants rather than four), please do that instead.
Attachment #724238 -
Flags: review?(jorendorff) → review+
Flags: needinfo?(jorendorff)
Reporter | ||
Comment 16•11 years ago
|
||
Post-developer analysis, I guess this no longer needs to land on b2g, but this will be nice to have on the Aurora 21 branch though.
Flags: needinfo?(lsblakk)
Assignee | ||
Comment 17•11 years ago
|
||
I don't think it needs to land on anything except central unless it blocks fuzzing. The patch is simple, but the bug doesn't afflict the browser.
Reporter | ||
Comment 18•11 years ago
|
||
(In reply to :Benjamin Peterson from comment #17) > I don't think it needs to land on anything except central unless it blocks > fuzzing. The patch is simple, but the bug doesn't afflict the browser. Yes, this blocks fuzzing on Windows (which is how I found this bug in the first place) on the Aurora 21 branch. I think they accept fuzzing blockers when requesting for landing approval.
Reporter | ||
Comment 19•11 years ago
|
||
I could make this trigger (via another unreliably large testcase) on Linux as well, but it was a lot more unreliable. I can somewhat confirm that the patch fixes this issue, too, on Linux.
Reporter | ||
Updated•11 years ago
|
Summary: Crash [@ js_NewStringCopyN] with a threadsafe js shell on Windows → Crash [@ js_NewStringCopyN] with a threadsafe js shell
Assignee | ||
Comment 20•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd8615bc2be5
Assignee | ||
Comment 21•11 years ago
|
||
Comment on attachment 724238 [details] [diff] [review] tweak some defines [Approval Request Comment] Blocks fuzzing on auroa. Very simple patch, which doesn't change browser.
Attachment #724238 -
Flags: approval-mozilla-aurora?
Comment 22•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bd8615bc2be5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•11 years ago
|
Attachment #724238 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 23•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/a18006dab9f4
Flags: needinfo?(lsblakk)
Updated•11 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•