Bug 180790

Summary: logVMFailure should not simulate crash on iOS
Product: WebKit Reporter: Saam Barati <saam>
Component: bmallocAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ddkilzer, fpizlo, ggaren, jfbastien, keith_miller, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218571
Attachments:
Description Flags
patch none

Saam Barati
Reported 2017-12-13 18:10:48 PST
Reasons I don't think we should do this: 1. Gigacage will often fail 2. I think user controlled allocations in JS may bottom out in tryVMAllocate. Since the allocation size is often user controlled, and JS gracefully handles tryVMAllocate failing, and throws an OOM, it seems weird to consider this a crash.
Attachments
patch (1.34 KB, patch)
2017-12-13 19:47 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2017-12-13 19:47:50 PST
Created attachment 329313 [details] patch I will wait for feedback from Dave before landing since we were talking about it offline already. An alternative patch could be to just log if the allocation is under some threshold, say 1MB or 10MB.
JF Bastien
Comment 2 2017-12-14 10:47:51 PST
Comment on attachment 329313 [details] patch r=me
Keith Miller
Comment 3 2017-12-14 10:49:34 PST
Comment on attachment 329313 [details] patch Do we want to log a failure on MacOS still?
Saam Barati
Comment 4 2017-12-14 10:51:22 PST
(In reply to Keith Miller from comment #3) > Comment on attachment 329313 [details] > patch > > Do we want to log a failure on MacOS still? Logging only does things for iOS. Also, I'm not convinced we ever want to log if this is a user controlled input.
WebKit Commit Bot
Comment 5 2017-12-14 11:10:58 PST
Comment on attachment 329313 [details] patch Clearing flags on attachment: 329313 Committed r225912: <https://trac.webkit.org/changeset/225912>
WebKit Commit Bot
Comment 6 2017-12-14 11:10:59 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2017-12-14 11:11:16 PST
<rdar://problem/36052852>
Note You need to log in before you can comment on or make changes to this bug.