Closed
Bug 831040
Opened 8 years ago
Closed 8 years ago
js_ReportAllocationOverflow should not be able to GC
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: bhackett1024, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
4.02 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
This function gets called in various allocator routines when the allocated space is too large. Examples are JSContext::pod_malloc/pod_calloc. Similar to js_ReportOutOfMemory, it should be impossible for this function to trigger a GC, as otherwise all sorts of unexpected GCs could occur.
Reporter | ||
Comment 1•8 years ago
|
||
Attachment #702552 -
Flags: review?(terrence)
Reporter | ||
Comment 2•8 years ago
|
||
Some error reporting on malformed inputs when inflating/deflating UTF8 strings is also causing issues.
Attachment #702552 -
Attachment is obsolete: true
Attachment #702552 -
Flags: review?(terrence)
Attachment #702892 -
Flags: review?(terrence)
Comment 3•8 years ago
|
||
Comment on attachment 702892 [details] [diff] [review] add a few more AutoSuppressGCs Review of attachment 702892 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay. This is pretty excellent, so lets go ahead and do it. ::: js/src/jsstr.cpp @@ +3856,5 @@ > if (srclen > dstlen) { > for (size_t i = 0; i < dstlen; i++) > dst[i] = (char) src[i]; > if (maybecx) { > + AutoSuppressGC suppress(maybecx); I had planned to eventually change the API to treat a too-short buffer as API misuse and crash appropriately, but given our timeframe, I think this is the better approach. Please wrap this as js_ReportBufferTooSmall in jscntxt.cpp so that these don't get lost when I or someone else moves this all to vm/CharacterEncoding.cpp, where it belongs. @@ +3929,5 @@ > JS_ReportErrorFlagsAndNumber(cx, > JSREPORT_ERROR, > js_GetErrorMessage, NULL, > JSMSG_UTF8_CHAR_TOO_LARGE, > buffer); Likewise, wrap this as js_ReportUTF8CharTooLarge.
Attachment #702892 -
Flags: review?(terrence) → review+
Comment 4•8 years ago
|
||
On second thought, we only want to require suppression for OOM, so this is fine as is.
Reporter | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f719315ea412
Comment 6•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f719315ea412
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•