Last Comment Bug 308085 - JavaScript switch statement going to wrong case
: JavaScript switch statement going to wrong case
Status: RESOLVED FIXED
: fixed1.8, js1.5, js1.6, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 1.8 Branch
: All All
: P1 critical (vote)
: mozilla1.8beta5
Assigned To: Brendan Eich [:brendan]
:
:
Mentors:
Depends on: 309695 310456
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-11 18:43 PDT by Henry Cejtin
Modified: 2006-03-12 18:54 PST (History)
5 users (show)
brendan: blocking1.8b5+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Here is the html file that actually demonstrates the bug (48.11 KB, text/html)
2005-09-11 18:44 PDT, Henry Cejtin
no flags Details
Simpler test case for the shell (34.80 KB, text/plain)
2005-09-20 03:03 PDT, Igor Bukanov
no flags Details
fix the regression (6.79 KB, patch)
2005-09-21 15:54 PDT, Brendan Eich [:brendan]
mrbkap: review+
shaver: superreview+
Details | Diff | Splinter Review
fix checked in, good for 1.8 branch too (6.87 KB, patch)
2005-09-21 20:28 PDT, Brendan Eich [:brendan]
brendan: review+
brendan: superreview+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Henry Cejtin 2005-09-11 18:43:16 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050825 Firefox/1.0.4 (Debian package 1.0.4-2sarge3)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

The attached html file, when run, first puts up an alert box showing 43 (which
is correct) but then, instead of putting up an alert showing CORRECT, it puts
up one showing BUG.  This indicates that the big switch statement, which has
cases for 0 through 244 as well as default, is going to the wrong place.

Note, as should be clear, the JavaScript code is machine generated.  It doesn't
take much of a change to cause the bug to go away (or atleast hide).

I marked this bug critical because clearly the JavaScript could do any thing,
including causing data to be lost.

Reproducible: Always

Steps to Reproduce:
1. load the html file
2. click ok on the first alert (showing 43)
3. observe the alert showing BUG

Actual Results:  
You see the BUG alert

Expected Results:  
You should see the CORRECT alert
Comment 1 Henry Cejtin 2005-09-11 18:44:27 PDT
Created attachment 195696 [details]
Here is the html file that actually demonstrates the bug
Comment 2 Bill Gianopoulos [:WG9s] 2005-09-11 18:57:24 PDT
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
Gecko/20050911 Firefox/1.6a1 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20050830 SeaMonkey/1.1a.

JavaScrpit looks OK to me.  Works as user expected in IE and Opera.
Comment 3 OstGote! 2005-09-12 11:01:21 PDT
Works also with Mozilla 1.7.11. 
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-09-19 10:16:04 PDT
Seeing as this is a regression from the 1.7 branch... blocking1.8b5?
Comment 5 Brendan Eich [:brendan] 2005-09-19 12:14:12 PDT
I won't have time to look at this till later today, so mrbkap or shaver, feel
free to steal.

/be
Comment 6 Igor Bukanov 2005-09-20 03:03:24 PDT
Created attachment 196769 [details]
Simpler test case for the shell

If one remove single line of 1; or add "default:" after "case 1:" in the test
case, then it printsexpected "CORRECT" instead of "BUG".
Comment 7 Igor Bukanov 2005-09-20 03:59:51 PDT
The regression is caused by the fix for the bug 302439. Without attachment
190798 [details] [diff] [review] it works.
Comment 8 Brendan Eich [:brendan] 2005-09-21 15:54:36 PDT
Created attachment 196961 [details] [diff] [review]
fix the regression

This patch also fixes the bug that causes the code generator to emit bytecode
for all those useless '1;' statements.

/be
Comment 9 Brendan Eich [:brendan] 2005-09-21 16:00:15 PDT
(In reply to comment #8)
> This patch also fixes the bug that causes the code generator to emit bytecode
> for all those useless '1;' statements.

But only when the switch is in a function.  At top-level, the useless expression
eliminator is still lame, for fear of eliminating a wanted result communicated
to JS_EvaluateScript's caller via the *rval out param.

/be
Comment 10 Blake Kaplan (:mrbkap) 2005-09-21 16:08:24 PDT
Comment on attachment 196961 [details] [diff] [review]
fix the regression

Moving the null defense to the right place so we actually emit good bytecodes
sounds good to me!

r=mrbkap
Comment 11 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-09-21 20:13:56 PDT
Comment on attachment 196961 [details] [diff] [review]
fix the regression

sr=shaver
Comment 12 Brendan Eich [:brendan] 2005-09-21 20:28:52 PDT
Created attachment 196998 [details] [diff] [review]
fix checked in, good for 1.8 branch too

This is a regression in 1.8b4, new since Firefox 1.0 / Mozilla 1.7.

/be
Comment 13 Brendan Eich [:brendan] 2005-09-21 20:30:39 PDT
Fixed on the trunk.

/be
Comment 14 Brendan Eich [:brendan] 2005-09-22 23:24:52 PDT
This bug's fix caused bug 309695.

/be
Comment 15 Brendan Eich [:brendan] 2005-09-22 23:30:16 PDT
Fixed on the 1.8 branch, along with 309695.

/be
Comment 16 Bob Clary [:bc:] 2005-10-09 19:55:12 PDT
Checking in regress-308085.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-308085.js,v  <--  regress-308085.js
initial revision: 1.1
done

Note You need to log in before you can comment on or make changes to this bug.