Closed
Bug 596988
Opened 14 years ago
Closed 14 years ago
Crash [@ js_ConcatStrings]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta8+ |
People
(Reporter: martijn.martijn, Assigned: alangpierce)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: fixed-in-tracemonkey)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
While testing, I keep getting hit by these kind of crashes.
http://crash-stats.mozilla.com/report/index/bp-f207ec22-5176-4fc1-b135-a931e2100914
0 xul.dll js_ConcatStrings js/src/jsstr.cpp:382
1 xul.dll js::mjit::stubs::Add js/src/methodjit/StubCalls.cpp:1216
It would be nice if it could get fixed.
Updated•14 years ago
|
blocking2.0: --- → ?
Updated•14 years ago
|
Blocks: JaegerBrowser
Do you know when it started?
blocking2.0: ? → final+
Reporter | ||
Comment 2•14 years ago
|
||
Probably when tracemonkey landed or something.
Assignee | ||
Comment 3•14 years ago
|
||
This patch probably fixes this issue. It fixes a stupid OOM-propagation bug. I'm flagging gal for review, since he reviewed the original ropes patch.
Here's the story to explain the line number (the line is a call to ObtainRopeBuffer, which is forced to be inlined, so the line number info just says that the crash happened in ObtainRopeBuffer), and why this would occur relatively rarely:
-Concat strings is called where at least one argument is already a rope, and its underlying buffer is not big enough, so a new one needs to be malloced.
-malloc inside ObtainRopeBuffer fails, so ObtainRopeBuffer returns NULL.
-The NULL is sent to FinishConcat (instead of being propagated, as it should be), so we return a rope that is otherwise completely valid, except that the underlying buffer of the top node is instead NULL.
-There are no (failed) mallocs until the next call to js_ConcatStrings.
-The next call to js_ConcatStrings calls ObtainRopeBuffer, which tries to read from the NULL buffer, causing a crash.
Attachment #476666 -
Flags: review?(gal)
Updated•14 years ago
|
Attachment #476666 -
Flags: review?(gal) → review+
Assignee | ||
Comment 4•14 years ago
|
||
I don't have commit access, so someone else needs to commit this.
Do we really think that we're in OOM when this hits? I would hope that'd be rarer than this bug seems to be...
(See also bug 597886.)
Reporter | ||
Comment 7•14 years ago
|
||
I was hitting this often while testing bug 581539.
Blocks: crossfuzz
Updated•14 years ago
|
Keywords: checkin-needed
Comment 9•14 years ago
|
||
Don't code else after return. Whoever lands this can fix with rs=me unless Alan wants to do a new patch.
/be
Comment 10•14 years ago
|
||
I'm seeing a crash in js_ConcatStrings when I'm trying to download large files (Go-OO ~180Mb) in the latest nightly on Win XP.
Crash report: http://crash-stats.mozilla.com/report/index/bp-c4440b0a-240b-431a-ac4f-fc6ad2100928 (there's a bunch more, but they all look the same to me)
Assignee: general → alangpierce
Comment 11•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/aa2ca63a88d8
Refactored code to eliminate else-after-return. OK'd by Alan.
Whiteboard: fixed-in-tracemonkey
Updated•14 years ago
|
blocking2.0: final+ → beta8+
Updated•14 years ago
|
Keywords: checkin-needed
OS: Windows 7 → Windows XP
Comment 12•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Crash Signature: [@ js_ConcatStrings]
You need to log in
before you can comment on or make changes to this bug.
Description
•