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: 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, Build Bot
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

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 Build Bot 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 Build Bot 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 Build Bot 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.