Bug 239785

Summary: JSC shouldn't crash when we run out of structure address space but throw OOM
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: NEW    
Severity: Normal CC: ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch msaboff: review+

Keith Miller
Reported 2022-04-26 13:17:27 PDT
JSC shouldn't crash when we run out of structure address space but throw OOM
Attachments
Patch (10.68 KB, patch)
2022-04-26 13:21 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (12.61 KB, patch)
2022-04-26 19:26 PDT, Keith Miller 		msaboff: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2022-04-26 13:21:23 PDT
Created attachment 458392 [details] Patch
Yusuke Suzuki
Comment 2 2022-04-26 13:33:47 PDT
Comment on attachment 458392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458392&action=review > Source/JavaScriptCore/runtime/JSObjectInlines.h:421 > + if (!newStructure) { Currently, caller of putDirect etc. does not check exception. So I don’t think we can throw it safely without changing all the callers of putDirect.
Mark Lam
Comment 3 2022-04-26 13:38:44 PDT
Comment on attachment 458392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458392&action=review > Source/JavaScriptCore/runtime/JSObjectInlines.h:422 > + auto scope = DECLARE_THROW_SCOPE(vm); The proper idiom would be to put this decl at the top of the function. Otherwise, callers won't always be informed that this function may throw.
Keith Miller
Comment 4 2022-04-26 19:26:37 PDT
Created attachment 458412 [details] Patch
Michael Saboff
Comment 5 2022-04-27 08:31:32 PDT
Comment on attachment 458412 [details] Patch r=me
Mark Lam
Comment 6 2022-04-27 10:46:33 PDT
Comment on attachment 458412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458412&action=review > Source/JavaScriptCore/runtime/JSObjectInlines.h:423 > + auto scope = DECLARE_THROW_SCOPE(vm); This is still in the wrong place. This means there will be fall out from unchecked exceptions that go undetected.
Saam Barati
Comment 7 2022-04-27 11:08:47 PDT
Comment on attachment 458412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458412&action=review >> Source/JavaScriptCore/runtime/JSObjectInlines.h:423 >> + auto scope = DECLARE_THROW_SCOPE(vm); > > This is still in the wrong place. This means there will be fall out from unchecked exceptions that go undetected. yes, agreed.
Yusuke Suzuki
Comment 8 2022-04-27 11:10:26 PDT
There are many places we cannot fail. One example is JSGlobalObject initialization, which will create bunch of Structures. I wonder if throwing OOM will cause fallout in various places.
Radar WebKit Bug Importer
Comment 9 2022-05-03 13:18:13 PDT
<rdar://problem/92688938>
Note You need to log in before you can comment on or make changes to this bug.