Bug 63951 - ASSERT_NOT_REACHED running test 262
Summary: ASSERT_NOT_REACHED running test 262
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Juan C. Montemayor
URL: http://test262.ecmascript.org/
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-05 13:19 PDT by Gavin Barraclough
Modified: 2011-07-06 21:20 PDT (History)
3 users (show)

See Also:


Attachments
patch (1.34 KB, patch)
2011-07-06 11:02 PDT, Juan C. Montemayor
no flags Details | Formatted Diff | Diff
updated patch (27.85 KB, patch)
2011-07-06 15:23 PDT, Juan C. Montemayor
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.46 MB, application/zip)
2011-07-06 16:41 PDT, WebKit Review Bot
no flags Details
updated patch (29.61 KB, patch)
2011-07-06 17:12 PDT, Juan C. Montemayor
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2011-07-05 13:19:33 PDT
This is in JSParser::updateErrorMessageSpecialCase.
Comment 1 Juan C. Montemayor 2011-07-05 14:04:05 PDT
Do you know which specific test(s) fail because of the assert?
Comment 2 Juan C. Montemayor 2011-07-06 11:02:54 PDT
Created attachment 99851 [details]
patch
Comment 3 Gavin Barraclough 2011-07-06 11:05:17 PDT
Comment on attachment 99851 [details]
patch

Ah! I should have guessed it was this!  Cheers for the fix JC.  Do you have commit privileges? - or shall I cq+?
Comment 4 Juan C. Montemayor 2011-07-06 11:08:17 PDT
(In reply to comment #3)
> (From update of attachment 99851 [details])
> Ah! I should have guessed it was this!  Cheers for the fix JC.  Do you have commit privileges? - or shall I cq+?

Not yet ;) so you should cq+
Comment 5 Juan C. Montemayor 2011-07-06 11:11:31 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 99851 [details] [details])
> > Ah! I should have guessed it was this!  Cheers for the fix JC.  Do you have commit privileges? - or shall I cq+?
> 
> Not yet ;) so you should cq+

...but wait until I make sure this patch wont fail any tests... I was focused on 262 so I forgot about the webkit tests.
Comment 6 Gavin Barraclough 2011-07-06 11:44:12 PDT
Comment on attachment 99851 [details]
patch

Actually, I gave a bad review here.  We really should have a regression test covering this, if possible.  Could you look into this?  We should be able to add something to a regression test to check this syntax error is thrown correctly.  Could you try to produce a simple test that triggers the bug, and add to a layout test?  (LayoutTests/fast/js/reserved-words might be a candidate if this is reserved-words specific, or there may be a existing syntax-errors related test, or you can add a new test case).  Hope that sounds reasonable.
Comment 7 Juan C. Montemayor 2011-07-06 11:46:39 PDT
(In reply to comment #6)
> (From update of attachment 99851 [details])
> Actually, I gave a bad review here.  We really should have a regression test covering this, if possible.  Could you look into this?  We should be able to add something to a regression test to check this syntax error is thrown correctly.  Could you try to produce a simple test that triggers the bug, and add to a layout test?  (LayoutTests/fast/js/reserved-words might be a candidate if this is reserved-words specific, or there may be a existing syntax-errors related test, or you can add a new test case).  Hope that sounds reasonable.

On it!
Comment 8 Juan C. Montemayor 2011-07-06 15:23:25 PDT
Created attachment 99892 [details]
updated patch

Layout test actually exposed a logic bug that is fixed with this patch.
Comment 9 WebKit Review Bot 2011-07-06 16:25:03 PDT
Attachment 99892 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 9

Updating OpenSource
RA layer request failed: OPTIONS of 'http://svn.webkit.org/repository/webkit': timed out waiting for server (http://svn.webkit.org) at /usr/lib/git-core/git-svn line 2295

Died at Tools/Scripts/update-webkit line 146.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Juan C. Montemayor 2011-07-06 16:34:11 PDT
(In reply to comment #9)
> Attachment 99892 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 9
> 

I ran check-webkit-style locally and it passed with no errors.
Comment 11 WebKit Review Bot 2011-07-06 16:41:21 PDT
Comment on attachment 99892 [details]
updated patch

Attachment 99892 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8991356

New failing tests:
fast/js/reserved-words-strict.html
Comment 12 WebKit Review Bot 2011-07-06 16:41:26 PDT
Created attachment 99899 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 13 Juan C. Montemayor 2011-07-06 17:12:23 PDT
Created attachment 99903 [details]
updated patch

Hopefully will work with chromium now
Comment 14 Gavin Barraclough 2011-07-06 20:35:25 PDT
Comment on attachment 99903 [details]
updated patch

Looks good to me, cheers JC.
Comment 15 WebKit Review Bot 2011-07-06 21:20:18 PDT
Comment on attachment 99903 [details]
updated patch

Clearing flags on attachment: 99903

Committed r90535: <http://trac.webkit.org/changeset/90535>
Comment 16 WebKit Review Bot 2011-07-06 21:20:26 PDT
All reviewed patches have been landed.  Closing bug.