JavaScript switch statement going to wrong case

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
JavaScript Engine
P1
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Henry Cejtin, Assigned: brendan)

Tracking

(5 keywords)

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

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
(Reporter)

Comment 1

12 years ago
Created attachment 195696 [details]
Here is the html file that actually demonstrates the bug
Assignee: nobody → general
Status: UNCONFIRMED → NEW
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
(Assignee)

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.

/be
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.
(Assignee)

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.

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

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

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.

/be
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
Attachment #196961 - Flags: review?(mrbkap) → review+
Comment on attachment 196961 [details] [diff] [review]
fix the regression

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

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.

/be
Attachment #196998 - Flags: superreview+
Attachment #196998 - Flags: review+
Attachment #196998 - Flags: approval1.8b5?
(Assignee)

Comment 13

12 years ago
Fixed on the trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

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

Updated

12 years ago
Blocks: 309695
(Assignee)

Comment 14

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

/be
(Assignee)

Comment 15

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

/be
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
done
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.