Bug 203230 - Fix incorrect assertion in operationRegExpExecNonGlobalOrSticky().
Summary: Fix incorrect assertion in operationRegExpExecNonGlobalOrSticky().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-21 18:19 PDT by Mark Lam
Modified: 2019-10-22 13:28 PDT (History)
8 users (show)

See Also:

Attachments
proposed patch. (3.47 KB, patch)
2019-10-21 18:46 PDT, Mark Lam 		rmorisset: review+
Details | Formatted Diff | Diff
proposed patch. (3.60 KB, patch)
2019-10-21 22:57 PDT, Mark Lam 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews212 for win-future (13.72 MB, application/zip)
2019-10-22 00:41 PDT, EWS Watchlist 		no flags Details
Show Obsolete (1) 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 Mark Lam 2019-10-21 18:19:15 PDT 
operationRegExpExecNonGlobalOrSticky() was asserting no exception when createRegExpMatchesArray() returns null.  createRegExpMatchesArray() only returns null when RegExp::matchInline() returns -1.  The only way RegExp::matchInline() can return -1 is via a throwError() helper which throws an exception.  The other return path in RegExp::matchInline() explicitly ASSERT(result >= -1).

Hence, the assertion in operationRegExpExecNonGlobalOrSticky() is wrong.

<rdar://problem/56460749>
Comment 1 Mark Lam 2019-10-21 18:46:06 PDT 
Created attachment 381497 [details]
proposed patch.
Comment 2 Robin Morisset 2019-10-21 18:47:13 PDT 
Comment on attachment 381497 [details]
proposed patch.

r=me
Comment 3 Mark Lam 2019-10-21 18:56:37 PDT 
Thanks for the review.  Landed in r251411: <http://trac.webkit.org/r251411>.
Comment 4 Mark Lam 2019-10-21 22:45:08 PDT 
The fix is incorrect and regressing some JSC tests.  Rolled out in r251418: <http://trac.webkit.org/r251418>.
Comment 5 Mark Lam 2019-10-21 22:57:05 PDT 
Created attachment 381512 [details]
proposed patch.
Comment 6 EWS Watchlist 2019-10-22 00:41:50 PDT 
Comment on attachment 381512 [details]
proposed patch.

Attachment 381512 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13161246

New failing tests:
fast/repaint/backgroundSizeRepaint.html
Comment 7 EWS Watchlist 2019-10-22 00:41:52 PDT 
Created attachment 381519 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 8 EWS Watchlist 2019-10-22 02:37:11 PDT 
Comment on attachment 381512 [details]
proposed patch.

Attachment 381512 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/13161354

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-no-ftl
Comment 9 Mark Lam 2019-10-22 09:15:06 PDT 
(In reply to Build Bot from comment #6)
> New failing tests:
> fast/repaint/backgroundSizeRepaint.html

This is failing due to slight differences in rendered pixels. It cannot possibly be due to this patch.


(In reply to Build Bot from comment #8)
> New failing tests:
> mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-no-ftl

This is the test flaking out again.  It's not related to this patch.
Comment 10 Robin Morisset 2019-10-22 12:22:51 PDT 
Comment on attachment 381512 [details]
proposed patch.

r=me
Comment 11 Mark Lam 2019-10-22 12:24:57 PDT 
Comment on attachment 381512 [details]
proposed patch.

Thanks for the review.
Comment 12 WebKit Commit Bot 2019-10-22 13:28:00 PDT 
Comment on attachment 381512 [details]
proposed patch.

Clearing flags on attachment: 381512

Committed r251447: <https://trac.webkit.org/changeset/251447>
Comment 13 WebKit Commit Bot 2019-10-22 13:28:02 PDT 
All reviewed patches have been landed.  Closing bug.