Bug 176662 – Fix all ExceptionScope verification failures in JavaScriptCore.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

This is in preparation to turn on exception scope verification for JSC tests.

[Radar WebKit Bug Importer]

<rdar://problem/34352085>

[Mark Lam]

Created attachment 320365 [details]
proposed patch.

[Mark Lam]

Created attachment 320368 [details]
proposed patch.

[Build Bot]

Attachment 320368 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:91:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 71 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Created attachment 320369 [details]
proposed patch.

[Build Bot]

Attachment 320369 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:91:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 71 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 320369 [details]
proposed patch.

R=me with build fixes.

[Mark Lam]

Created attachment 320376 [details]
patch for landing.

Thanks for the review.  Here's the patch for landing: added a #include in JSDOMMapLike.cpp to fix the build, and a scope.release() in jsc.cpp's functionDollarAgentReceiveBroadcast() to fix an intermittent failure in the stress/lars-sab-workers.js test.

[Build Bot]

Attachment 320376 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:91:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 71 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

*** Bug 165035 has been marked as a duplicate of this bug. ***

[Build Bot]

Comment on attachment 320376 [details]
patch for landing.

Attachment 320376 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4501545

New failing tests:
imported/w3c/web-platform-tests/fetch/api/headers/headers-record.html
js/dom/webidl-type-mapping.html

[Build Bot]

Created attachment 320382 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Mark Lam]

(In reply to Build Bot from comment #11)
> New failing tests:
> imported/w3c/web-platform-tests/fetch/api/headers/headers-record.html
> js/dom/webidl-type-mapping.html

I'm investigating these.

[Mark Lam]

Created attachment 320384 [details]
patch for landing (again).

I applied the same patch on a different workspace (same revision) and cannot reproduce these 2 crashes.  I suspect the issue isn't due to my patch.  Let's try re-uploading it and trying on the EWS again.

[Build Bot]

Attachment 320384 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:91:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 71 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 320384 [details]
patch for landing (again).

Attachment 320384 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4502530

New failing tests:
imported/w3c/web-platform-tests/fetch/api/headers/headers-record.html
js/dom/webidl-type-mapping.html

[Build Bot]

Created attachment 320390 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Mark Lam]

Created attachment 320411 [details]
patch for landing (w/ an exception check fix in JSDOMConvertRecord.h's convert().

Let's try this on the EWS again.

[Build Bot]

Attachment 320411 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:91:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

The EWS bots and my local testing says that tests are passing now with the latest patch.

Landed in r221849: <http://trac.webkit.org/r221849>.

[Saam Barati]

Comment on attachment 320411 [details]
patch for landing (w/ an exception check fix in JSDOMConvertRecord.h's convert().

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

> Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:788
> +    CLEAR_AND_RETURN_IF_EXCEPTION(catchScope, encodedJSValue());

These look wrong. You're returning JSValue() to a JS caller *without* an exception.

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:86
> +    CLEAR_AND_RETURN_IF_EXCEPTION(scope, encodedJSValue());

ditto

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:187
> +    CLEAR_AND_RETURN_IF_EXCEPTION(scope, encodedJSValue());

ditto

> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:194
> +        CLEAR_AND_RETURN_IF_EXCEPTION(scope, encodedJSValue());

ditto