Bug 163996

Summary: String.prototype.replace() should throw an OutOfMemoryError when using too much memory.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, fpizlo, ggaren, jfbastien, keith_miller, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 164125    
Bug Blocks:    
Attachments:
Description Flags
proposed patch.
none
proposed patch. ggaren: review+

Mark Lam
Reported 2016-10-25 16:57:04 PDT
Currently, it just crashes with an allocation failure, which is correct behavior but not too friendly.
Attachments
proposed patch. (11.92 KB, patch)
2016-10-25 17:07 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
proposed patch. (12.23 KB, patch)
2016-10-25 17:25 PDT, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-10-25 16:58:12 PDT
<rdar://problem/28263117>
Mark Lam
Comment 2 2016-10-25 17:07:26 PDT
Created attachment 292852 [details] proposed patch.
Geoffrey Garen
Comment 3 2016-10-25 17:17:51 PDT
Comment on attachment 292852 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=292852&action=review > Source/JavaScriptCore/runtime/StringPrototype.cpp:462 > + THROW_AND_RETURN_IF_FAIL(exec, scope, sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)); I don't really like this idiom because it buries the real code inside the error handling macro. How about this: if (!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)) OUT_OF_MEMORY(exec, scope);
Mark Lam
Comment 4 2016-10-25 17:19:06 PDT
(In reply to comment #3) > Comment on attachment 292852 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=292852&action=review > > > Source/JavaScriptCore/runtime/StringPrototype.cpp:462 > > + THROW_AND_RETURN_IF_FAIL(exec, scope, sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)); > > I don't really like this idiom because it buries the real code inside the > error handling macro. > > How about this: > > if (!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)) > OUT_OF_MEMORY(exec, scope); Sure. I'll make the change.
Mark Lam
Comment 5 2016-10-25 17:25:38 PDT
Created attachment 292853 [details] proposed patch.
Geoffrey Garen
Comment 6 2016-10-25 17:32:54 PDT
Comment on attachment 292853 [details] proposed patch. r=me
Mark Lam
Comment 7 2016-10-25 18:19:17 PDT
Thanks for the review. Landed in r207861: <http://trac.webkit.org/r207861>.
Note You need to log in before you can comment on or make changes to this bug.