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 Bugs

Assignee:

Keith Miller <keith_miller>

Status:

NEW ---

Severity:

Normal

CC:

ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	msaboff: review+

Description Keith Miller 2022-04-26 13:17:27 PDT

JSC shouldn't crash when we run out of structure address space but throw OOM

Comment 1 Keith Miller 2022-04-26 13:21:23 PDT

Created attachment 458392 [details]
Patch

Comment 2 Yusuke Suzuki 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.

Comment 3 Mark Lam 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.

Comment 4 Keith Miller 2022-04-26 19:26:37 PDT

Created attachment 458412 [details]
Patch

Comment 5 Michael Saboff 2022-04-27 08:31:32 PDT

Comment on attachment 458412 [details]
Patch

r=me

Comment 6 Mark Lam 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.

Comment 7 Saam Barati 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.

Comment 8 Yusuke Suzuki 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.

Comment 9 Radar WebKit Bug Importer 2022-05-03 13:18:13 PDT

<rdar://problem/92688938>