Bug 185755 - We don't throw SyntaxErrors for runtime generated regular expressions with errors
Summary: We don't throw SyntaxErrors for runtime generated regular expressions with er...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-17 16:57 PDT by Michael Saboff
Modified: 2018-05-17 19:59 PDT (History)
6 users (show)

See Also:

Attachments
Patch (8.61 KB, patch)
2018-05-17 17:13 PDT, Michael Saboff 		keith_miller: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.31 MB, application/zip)
2018-05-17 18:28 PDT, EWS Watchlist 		no flags Details
Updated patch with new results for stack-overflow-regexp.js (17.35 KB, patch)
2018-05-17 18:32 PDT, Michael Saboff 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2018-05-17 16:57:06 PDT 
We properly handle and report syntax errors for statically declared regular expressions.  When we need to compile a regular expression as part of executing methods that take a regular we sometime don't report errors.
Comment 1 Michael Saboff 2018-05-17 16:58:50 PDT 
<rdar://problem/40303478>
Comment 2 Michael Saboff 2018-05-17 17:13:33 PDT 
Created attachment 340668 [details]
Patch
Comment 3 Keith Miller 2018-05-17 17:20:03 PDT 
Comment on attachment 340668 [details]
Patch

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

r=me.

> Source/JavaScriptCore/runtime/RegExpConstructor.cpp:286
> +                throwException(exec, scope, regExp->errorToThrow(exec));

shouldn't there be a return here? Why create an invalid RegExp?
Comment 4 Michael Saboff 2018-05-17 17:37:53 PDT 
(In reply to Keith Miller from comment #3)
> Comment on attachment 340668 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340668&action=review
> 
> r=me.
> 
> > Source/JavaScriptCore/runtime/RegExpConstructor.cpp:286
> > +                throwException(exec, scope, regExp->errorToThrow(exec));
> 
> shouldn't there be a return here? Why create an invalid RegExp?

Sure, I'll return the exception object.
Comment 5 EWS Watchlist 2018-05-17 18:28:51 PDT 
Comment on attachment 340668 [details]
Patch

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

New failing tests:
js/stack-overflow-regexp.html
Comment 6 EWS Watchlist 2018-05-17 18:28:52 PDT 
Created attachment 340669 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-05-17 18:29:48 PDT 
Comment on attachment 340668 [details]
Patch

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

New failing tests:
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-no-cjit
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-dfg-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-no-llint
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-ftl-no-cjit
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-no-ftl
jsc-layout-tests.yaml/js/script-tests/stack-overflow-regexp.js.layout-ftl-eager-no-cjit
apiTests
Comment 8 Michael Saboff 2018-05-17 18:32:36 PDT 
Created attachment 340670 [details]
Updated patch with new results for stack-overflow-regexp.js
Comment 9 Keith Miller 2018-05-17 18:34:33 PDT 
Comment on attachment 340670 [details]
Updated patch with new results for stack-overflow-regexp.js

r=me.
Comment 10 WebKit Commit Bot 2018-05-17 19:59:37 PDT 
Comment on attachment 340670 [details]
Updated patch with new results for stack-overflow-regexp.js

Clearing flags on attachment: 340670

Committed r231939: <https://trac.webkit.org/changeset/231939>
Comment 11 WebKit Commit Bot 2018-05-17 19:59:38 PDT 
All reviewed patches have been landed.  Closing bug.