JavaScript switch statement going to wrong case

RESOLVED FIXED in mozilla1.8beta5



JavaScript Engine
12 years ago
12 years ago


(Reporter: Henry Cejtin, Assigned: brendan)


(5 keywords)

1.8 Branch
fixed1.8, js1.5, js1.6, regression, testcase
Dependency tree / graph
Bug Flags:
blocking1.8b5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)



(4 attachments)



12 years ago
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

12 years ago
Created attachment 195696 [details]
Here is the html file that actually demonstrates the bug
Assignee: nobody → general
Component: General → JavaScript Engine
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
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.
Hardware: PC → All

Comment 3

12 years ago
Works also with Mozilla 1.7.11. 
Keywords: testcase
Seeing as this is a regression from the 1.7 branch... blocking1.8b5?
Flags: blocking1.8b5?
Keywords: regression
Version: Trunk → 1.8 Branch

Comment 5

12 years ago
I won't have time to look at this till later today, so mrbkap or shaver, feel
free to steal.

Assignee: general → brendan
Flags: blocking1.8b5? → blocking1.8b5+
Keywords: js1.5, js1.6
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5

Comment 6

12 years ago
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

12 years ago
The regression is caused by the fix for the bug 302439. Without attachment
190798 [details] [diff] [review] it works.

Comment 8

12 years ago
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.

Attachment #196961 - Flags: superreview?(shaver)
Attachment #196961 - Flags: review?(mrbkap)


12 years ago

Comment 9

12 years ago
(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.

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!

Attachment #196961 - Flags: review?(mrbkap) → review+
Comment on attachment 196961 [details] [diff] [review]
fix the regression

Attachment #196961 - Flags: superreview?(shaver) → superreview+

Comment 12

12 years ago
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.

Attachment #196998 - Flags: superreview+
Attachment #196998 - Flags: review+
Attachment #196998 - Flags: approval1.8b5?

Comment 13

12 years ago
Fixed on the trunk.

Last Resolved: 12 years ago
Resolution: --- → FIXED


12 years ago
Attachment #196998 - Flags: approval1.8b5? → approval1.8b5+


12 years ago
Blocks: 309695

Comment 14

12 years ago
This bug's fix caused bug 309695.


Comment 15

12 years ago
Fixed on the 1.8 branch, along with 309695.

No longer blocks: 309695
Depends on: 309695
Keywords: fixed1.8
Depends on: 310456

Comment 16

12 years ago
Checking in regress-308085.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-308085.js,v  <--  regress-308085.js
initial revision: 1.1
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.