Bug 162351 – Turn on exception scope verification for JSC tests.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

This patch takes the number of test failures down from around 13000+ down to 2000+.  Perf appears to be neutral.  Will upload the patch shortly.

[Mark Lam]

Created attachment 289468 [details]
x86_64 benchmark results.

The benchmark results shows that perf is neutral.  All the "definitely" differences appear to be noise.  They don't reproduce when I do another round of benchmarking.  The only one that persisted is the int-or-other-abs-then-get-by-val.js microbenchmark, and I've fixed that (in the patch that I'm preparing for upload).

Archiving these results for reference.

[Mark Lam]

Created attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

[WebKit Commit Bot]

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


ERROR: Source/JavaScriptCore/runtime/JSONObject.cpp:829:  Missing spaces around |  [whitespace/operators] [3]
Total errors found: 1 in 95 files


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

[Build Bot]

Comment on attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

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

New failing tests:
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
plugins/npruntime/object-from-destroyed-plugin.html
fast/custom-elements/CustomElementRegistry.html
http/tests/plugins/cross-frame-object-access.html

[Build Bot]

Created attachment 289479 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

Attachment 289476 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2120645

New failing tests:
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
plugins/npruntime/object-from-destroyed-plugin.html
fast/custom-elements/CustomElementRegistry.html
http/tests/plugins/cross-frame-object-access.html

[Build Bot]

Created attachment 289482 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

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

New failing tests:
plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
http/tests/security/cross-frame-access-object-prototype.html
plugins/npruntime/object-from-destroyed-plugin.html
fast/custom-elements/CustomElementRegistry.html
http/tests/plugins/cross-frame-object-access.html
sputnik/Conformance/15_Native_Objects/15.10_RegExp/15.10.6/15.10.6.2_RegExp.prototype.exec/S15.10.6.2_A4_T11.html

[Build Bot]

Created attachment 289483 [details]
Archive of layout-test-results from ews114 for mac-yosemite

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

[Build Bot]

Comment on attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

Attachment 289476 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2120642

New failing tests:
fast/custom-elements/CustomElementRegistry.html

[Build Bot]

Created attachment 289484 [details]
Archive of layout-test-results from ews124 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Geoffrey Garen]

Is it practical to separate out the a patch that adds new exception checks from the patch that adds lots more validation?

I think it would be helpful to see just the things that needed exception checks, as distinct from the extra validation.

[Geoffrey Garen]

Comment on attachment 289476 [details]
work in progress: has new JSC test failures that need to be resolved.

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

> Source/JavaScriptCore/ChangeLog:15
> +        1. Deleted Error.cppâs hasErrorInfo() which is not used anywhere.

ASCII please

> Source/JavaScriptCore/runtime/ExceptionScope.h:83
> +#define RETURN_IF_EXCEPTION(scope__) do { \
> +        if (UNLIKELY((scope__).exception())) \
> +            return; \
> +    } while (false)
> +
> +#define RETURN_VALUE_IF_EXCEPTION(scope__, value__) do { \
> +        if (UNLIKELY((scope__).exception())) \
> +            return (value__); \
> +    } while (false)
> +

I think you can simplify and join these macros. Functions that return void can "RETURN_VALUE_IF_EXCEPTION(scope, void)". Then you can shorten "RETURN_VALUE_IF_EXCEPTION" TO "RETURN_IF_EXCEPTION" everywhere.

[Mark Lam]

Created attachment 295296 [details]
proposed patch.

[Mark Lam]

We won't land this patch until all the dependent bugs to fix missing exception checks have landed.

[Mark Lam]

Created attachment 295297 [details]
proposed patch.

[Mark Lam]

<rdar://problem/29563911>

[Mark Lam]

Created attachment 320447 [details]
proposed patch.

[Mark Lam]

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

[Ryan Haddad]

Reverted r221868 for reason:

Rolling out this change while we investigate test262 failures.

Committed r222037: <http://trac.webkit.org/changeset/222037>

[Mark Lam]

I've relanded this patch in r222618: <http://trac.webkit.org/r222618>.