350670 - Incorrect decompilation of "for(z() in x)"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

> function() { for(z() in x) { } }
function () { for (z()[()] in x) { } }

> function() { for(z(12345678) in x) { } }
function () { for (z(12345678)[8)] in x) { } }

I'm guessing the fact that "for (z() in x)" compiles at all has something to do with JS_HAS_LVALUE_RETURN.

Comment 1

[Brendan Eich [:brendan]]

Regression from 1.8.0 due to first big wave of js1.7 changes.

Great to have the decompiler fuzzer, for coverage!

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

The decompiler has always required saving strings popped from the sprinter's buffer in the malloc heap, unless an all-in-one Sprint is extending the buffer on top of the popped strings (in which case, Sprint's internal buffer is used to hold the output string as it's formatted, saving the input strings from being overwritten).  The js1.7 changes neglected to save xval at the top of the do_forinhead: code.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: better fix

Thanks to mrbkap for prodding.

/be

Comment 4

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 236132 [details] [diff] [review]
better fix

This is a safe spot fix, local variables and short live ranges are the only things to analyze.  The diff has two hunks, but they are adjacent and the only early returns are in the atom-not-null case, which is disjoint from the xval case as the if-else-if structure shows.

/be

Comment 6

[Bob Clary [:bc] (inactive)]

Checking in regress-350670.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-350670.js,v  <--  regress-350670.js
initial revision: 1.1

Comment 7

[Bob Clary [:bc] (inactive)]

I screwed up the test this morning.
Checking in regress-350670.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-350670.js,v  <--  regress-350670.js
new revision: 1.2; previous revision: 1.1

verified fixed 1.9 20060831 window/mac*/linux

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 236132 [details] [diff] [review]
better fix

a=beltzner on behalf of 181drivers

Comment 9

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 10

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 2006090118 windows/mac*/linux