320032 - Parenthesization dereferences ECMA Reference type, incorrectly

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

See bug 41864, where assignment's dereferencing of RHS Reference was incorrectly fixed by dereferencing under the new-in-that-bug's-fix JSOP_GROUP bytecode.  This may be ugly to fix, since SpiderMonkey does not use a Reference type internally, rather it compiles out all cases that need to store through a Reference using "lvalue"-specialized bytecodes.

/be

Comment 1

[Brendan Eich [:brendan]]

(RHS == Right Hand Side, i.e., "rvalue")

Comment 2

[Bob Clary [:bc] (inactive)]

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

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed fix

Passes the testsuite using the js shell.

/be

Comment 4

[Brendan Eich [:brendan]]

Minimal testcase, interactive session witnessing:

js> function f(){return this}
js> o = {}
[object Object]
js> (o.m = f)()
[object global]
js> (o.m)()
[object Object]

With this bug unpatched, the last line would be [object global] too.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 206876 [details] [diff] [review]
proposed fix

r=mrbkap

Comment 6

[Brendan Eich [:brendan]]

Checked into trunk for baking.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

verified windows/linux/mac trunk

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 206876 [details] [diff] [review]
proposed fix

sr=shaver, though I wish I were more confident that I had checked all the places that now want a nulling of obj.

Comment 9

[Brendan Eich [:brendan]]

shaver: it's easy, just grep for JOF_ASSIGNING in jsopcode.tbl to get this set:

setconst setprop setelem setarg setvar bindname setname enumelem setcall setgvar bindxmlname setxmlname

and eliminate all ops that do not result in an expression value (setconst bindname enumelem bindxmlname).  Note also that read/write ops such as JSOP_FORPROP and JSOP_INCPROP, which flip JSFRAME_ASSIGNING, either do not result in an expression value (the JSOP_FOR* ops) or are guaranteed to result in a non-callable value (the increment ops).

It might be beneficial to refactor bytecode into read- and write-only ops, and get rid of JSFRAME_ASSIGNING.  This is on my radar, I'll wiki about it and other steps toward a better VM.

/be

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 206876 [details] [diff] [review]
proposed fix

a=dveditz for drivers

Comment 11

[Brendan Eich [:brendan]]

Fixed on branches.

/be

Comment 12

[Bob Clary [:bc] (inactive)]

v 2006-01-11 1.8.0.1, 1.8.1, trunk on windows/linux/mac