620750 - "arguments;" sneaks into decompilation (eval in strict mode, for, genexp)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

function f()
{
  'use strict';
  for (; false; (0 for each (t in eval("")))) { }
}
fs = "" + f;
print(fs);
eval("(" + fs + ")");


Result:
typein:11: SyntaxError: missing ) after for-loop control:
typein:11:     for (; false;    arguments;
typein:11: ..............................^


The first bad revision is:
changeset:   bf52361e6fd0
user:        Jeff Walden
date:        Wed Aug 11 23:27:33 2010 -0700
summary:     Bug 516255 - Eagerly copy initial parameter values into the arguments object when a function's parameters might be mutated, and rely on normal resolution behavior in the remaining cases when parameters are never modified.  r=dmandelin

Comment 1

[Jeff Walden [:Waldo]]

Generator expressions don't have arguments, so creating an arguments object early isn't necessary.  That might be an easy fix to make to js_EmitFunctionScript.

But what the heck is |arguments| supposed to mean inside a generator expression anyway?  Looks like we bind it to the generator expression right now!

js> function g() { return (0 for each (t in arguments)); }
js> var gen = g(1, 2, 3)
js> gen.next()
uncaught exception: [object StopIteration]

Comment 2

[Brendan Eich [:brendan]]

A generator expression desugars to a generator lambda applied immediately:

js> dis('-r', function g() { return (0 for each (t in arguments)); })
flags: LAMBDA NULL_CLOSURE
main:
00000:  lambda (function () {for each (t in arguments) {yield 0;}})
00003:  nullblockchain
00004:  push
00005:  call 0
00008:  return
00009:  stop

Source notes:
  0:     0 [   0] if      
  1:     5 [   5] pcbase   offset 5

flags: LAMBDA NULL_CLOSURE
00000:  generator
main:
00001:  enterblock depth 0 {t: 0}
00004:  arguments
00005:  iter 3
00007:  goto 19 (12)
00010:  trace 0
00013:  forlocal 0
00016:  zero
00017:  yield
00018:  pop
00019:  moreiter
00020:  ifne 10 (-10)
00023:  enditer
00024:  leaveblock 1
00029:  stop

Source notes:
  0:     7 [   7] if-else  else 9 elseif 13
  3:    10 [   3] while    offset 10
  5:    17 [   7] hidden  

Exception table:
kind      stack    start      end
 iter         2        9       22

The desugaring means arguments is for the inner generator lambda, which indeed is called with 0 arguments. That is easy to fix:

js> function g() { var a = arguments; return (0 for each (t in a)); }  
js> dis('-r', g)
flags: NULL_CLOSURE
main:
00000:  arguments
00001:  setlocal 0
00004:  pop
00005:  lambda_fc (function () {for each (t in a) {yield 0;}})
00008:  nullblockchain
00009:  push
00010:  call 0
00013:  return
00014:  stop

Source notes:
  0:     1 [   1] decl     offset 0
  2:     5 [   4] if      
  3:    10 [   5] pcbase   offset 5

flags: LAMBDA FLAT_CLOSURE
upvars: {
  a: {skip:1, slot:0},
}
00000:  generator
main:
00001:  enterblock depth 0 {t: 0}
00004:  getfcslot 0
00007:  iter 3
00009:  goto 21 (12)
00012:  trace 0
00015:  forlocal 0
00018:  zero
00019:  yield
00020:  pop
00021:  moreiter
00022:  ifne 12 (-10)
00025:  enditer
00026:  leaveblock 1
00031:  stop

Source notes:
  0:     9 [   9] xdelta  
  1:     9 [   0] if-else  else 9 elseif 13
  4:    12 [   3] while    offset 10
  6:    19 [   7] hidden  

Exception table:
kind      stack    start      end
 iter         2       11       24

To make the demo nicer, let's comprehend t not 0:

js> function g() { var a = arguments; return (t for each (t in a)); }
js> i = g(1,2,3)                                                      
({})
js> i.next()
1
js> i.next()
2
js> i.next()
3
js> i.next()
uncaught exception: [object StopIteration]

Now to the bug. This is indeed a regression from js1.8.0/fx3. Here's a shell from that era running the testcase:

js> function f()
{
  'use strict';
  for (; false; (0 for each (t in eval("")))) { }
}
js> fs = "" + f;
function f() {
    for (; false; (0 for each (t in eval("")))) {
    }
}
js> print(fs);
function f() {
    for (; false; (0 for each (t in eval("")))) {
    }
}
js> eval("(" + fs + ")");
function f() {
    for (; false; (0 for each (t in eval("")))) {
    }
}

The genexp decompiler does not expect prolog opcodes to have stack effects. Luke knows about this change potentially rocking the boat. Here's one rock. I can fix quickly.

/be

Comment 3

[Brendan Eich [:brendan]]

In the fuzzer's face, wanted.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: proposed fix

We can't make the JSOP_ARGUMENTS;JSOP_POP hidden by annotating the JSOP_POP with SRC_HIDDEN, because that means the compiler's model stack budget is not updated for that pop. Plus, a srcnote costs enough to avoid with a bit more just-so code for decompiling genexps.

/be

Comment 5

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/13ab993875f2

/be

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/13ab993875f2