"let (y = 3)" in "case" expression decompiles as "let (3 = 3)"

VERIFIED FIXED in mozilla1.8.1

Status

()

Core
JavaScript Engine
P2
normal
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, {testcase, verified1.8.1})

Trunk
mozilla1.8.1
All
AIX
testcase, verified1.8.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
js> function() { switch(0) { case let(y = 3) 6: } }

function () {
    switch (0) {
      case let (3 = 3) 6:
      default:;
    }
}
(Assignee)

Comment 1

11 years ago
Created attachment 237276 [details] [diff] [review]
fix
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #237276 - Flags: review?(mrbkap)

Updated

11 years ago
Attachment #237276 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 2

11 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 3

11 years ago
Comment on attachment 237276 [details] [diff] [review]
fix

This is a safe fix for a failure to model the stack accurately.  It causes the model to have fewer items on it than expected, which could lead to bad memory conditions.  In the specific testcase given in this bug, the only result is a mis-decompilation.  But I would rather not take changes, and the fix correctly matches the code generator's stack model.

/be
Attachment #237276 - Flags: approval1.8.1?
(Assignee)

Comment 4

11 years ago
Comment on attachment 237276 [details] [diff] [review]
fix

Something's bad, this patch leads to decompiler string stack overflow.

/be
Attachment #237276 - Attachment is obsolete: true
Attachment #237276 - Flags: review+
Attachment #237276 - Flags: approval1.8.1?
(Assignee)

Updated

11 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

11 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 5

11 years ago
Created attachment 237297 [details] [diff] [review]
fix, v2

Two bugs:
1.  EmitSwitch failed to link the SRC_PCDELTA of the last case to the implicit JSOP_DEFAULT it generated after all the cases.  This made switch(x){case y:z} decompile with default: labeling z along with case y: !

2.  Fixed DecompileSwitch to manage the stack model correctly for the case label / statements order in which it processes bytecode, which does not match the code genartion order (case labels before any case statements -- the JSOP_CASE stack balance of -1 for mismatch instead of -2 for match favors the code gen order, relieving it of the need to fiddle with cg->stackDepth -- the decompiler loses).

/be
Attachment #237297 - Flags: review?(mrbkap)
(Assignee)

Updated

11 years ago
OS: Mac OS X 10.4 → AIX
Priority: -- → P2
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.8.1

Updated

11 years ago
Attachment #237297 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 6

11 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

11 years ago
Comment on attachment 237297 [details] [diff] [review]
fix, v2

Straight-up fixes, local to their containing functions except for the good effects of the EmitSwitch fix on the decompilation of switch statements that use variables in case labels and have no explicit default case.

/be
Attachment #237297 - Flags: approval1.8.1?

Comment 8

11 years ago
Comment on attachment 237297 [details] [diff] [review]
fix, v2

a=schrep for drivers.
Attachment #237297 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 9

11 years ago
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1

Comment 10

11 years ago
Checking in regress-351496.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-351496.js,v  <--  regress-351496.js
initial revision: 1.1
done
Flags: in-testsuite+

Comment 11

11 years ago
verified fixed 1.8 1.9 20060909 windows/mac*/linux
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
You need to log in before you can comment on or make changes to this bug.