Bug 165025 – Fix exception scope verification failures in JSONObject.cpp.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 295328 [details]
proposed patch.

[Saam Barati]

Comment on attachment 295328 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/JSONObject.cpp:233
> +    if (UNLIKELY(scope.exception()))

Why not RETURN_IF_EXCEPTION

> Source/JavaScriptCore/runtime/JSONObject.cpp:243
> +        if (UNLIKELY(scope.exception()))

Ditto

> Source/JavaScriptCore/runtime/JSONObject.cpp:278
> +    if ((stringifyResult != StringifySucceeded))

Style: Extra parens

> Source/JavaScriptCore/runtime/JSONObject.cpp:861
> +    auto throwScope = DECLARE_THROW_SCOPE(vm);

Could this be a catch scope instead?

[Mark Lam]

Comment on attachment 295328 [details]
proposed patch.

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

>> Source/JavaScriptCore/runtime/JSONObject.cpp:233
>> +    if (UNLIKELY(scope.exception()))
> 
> Why not RETURN_IF_EXCEPTION

Because this is in a constructor, and constructors do not like "return void()".

>> Source/JavaScriptCore/runtime/JSONObject.cpp:243
>> +        if (UNLIKELY(scope.exception()))
> 
> Ditto

Same.  In a constructor.

>> Source/JavaScriptCore/runtime/JSONObject.cpp:278
>> +    if ((stringifyResult != StringifySucceeded))
> 
> Style: Extra parens

Missing an "UNLIKELY" before the inner ().  Will fix.

>> Source/JavaScriptCore/runtime/JSONObject.cpp:861
>> +    auto throwScope = DECLARE_THROW_SCOPE(vm);
> 
> Could this be a catch scope instead?

I'm not sure.  But at least one client of JSONStringify() (i.e. JSValueCreateJSONString) is expecting it to be able to throw an exception.  I'll leave it with a ThrowScope for now.

[Mark Lam]

Thanks for the review.  Landed in r208966: <http://trac.webkit.org/r208966>.