Bug 227962

Summary: Check for out of memory in JSC::globalFuncEscape() and JSC::globalFuncUnescape().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. ysuzuki: review+

Description Mark Lam 2021-07-14 11:38:38 PDT
rdar://78392251
Comment 1 Mark Lam 2021-07-14 11:41:41 PDT
Created attachment 433514 [details]
proposed patch.
Comment 2 Yusuke Suzuki 2021-07-14 11:45:14 PDT
Comment on attachment 433514 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=433514&action=review

r=me

> Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616
> +            return vm.smallStrings.emptyString();

Why not returning `{ }`?

> Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:679
> +            return vm.smallStrings.emptyString();

Why not returning `{ }` ?
Comment 3 Mark Lam 2021-07-14 11:54:00 PDT
(In reply to Yusuke Suzuki from comment #2)
> Comment on attachment 433514 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433514&action=review
> 
> r=me

Thanks for the review.

> > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616
> > +            return vm.smallStrings.emptyString();
> 
> Why not returning `{ }`?
> 
> > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:679
> > +            return vm.smallStrings.emptyString();
> 
> Why not returning `{ }` ?

Because the return value is for a callback called from toStringView().  I gave returning { } a try and this is what I get:

./runtime/JSGlobalObjectFunctions.cpp:617:20: error: cannot deduce lambda return type from initializer list
            return { };
                   ^~~
./runtime/JSGlobalObjectFunctions.cpp:580:28: error: no matching function for call to 'toStringView'
    return JSValue::encode(toStringView(globalObject, callFrame->argument(0), [&] (StringView view) {
                           ^~~~~~~~~~~~

I'll just stick with returning the empty string for simplicity.
Comment 4 Mark Lam 2021-07-14 12:05:47 PDT
(In reply to Mark Lam from comment #3)
> > > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:616
> > > +            return vm.smallStrings.emptyString();
> > 
> > Why not returning `{ }`?
...
> Because the return value is for a callback called from toStringView().  I
> gave returning { } a try and this is what I get:
> 
> ./runtime/JSGlobalObjectFunctions.cpp:617:20: error: cannot deduce lambda
> return type from initializer list
>             return { };
>                    ^~~

On second thought, I can explicitly declare the lambdas as returning JSString*.  It will result in a roundabout way of returning an empty JSValue() on 64-bit and a null cell JSValue on 32-bit ... which is kind of inconsistent, but we already apply this treatment when throwing exceptions elsewhere in the code.

I'll just do that.
Comment 5 Mark Lam 2021-07-14 12:16:43 PDT
Landed in r279915: <http://trac.webkit.org/r279915>.