476079 - trap() on enditer can trigger "Assertion failure: JSVAL_IS_OBJECT(v), at ../jsapi.h"

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> f = function() { return [w for (x in [0])]; }
function () {
    return [w for (x in [0])];
}

js> f();
typein:1: ReferenceError: w is not defined

js> dis(f);
flags: LAMBDA INTERPRETED
main:
00000:  newinit 3
00002:  enterblock depth 1 {x: 0}
00005:  zero
00006:  newarray 1
00010:  iter 1
00012:  goto 24 (12)
00015:  forlocal 1
00018:  name "w"
00021:  arraypush 0
00024:  nextiter
00025:  ifne 15 (-10)
00028:  enditer                <-- trap will go here
00029:  leaveblock 1
00032:  endinit
00033:  return
00034:  stop

Source notes:
  0:    12 [  12] xdelta  
  1:    12 [   0] if-else  else 6 elseif 13

Exception table:
kind      stack    start      end
 iter         4       15       28

js> trap(f, 28, '');
js> f();
Assertion failure: *regs.pc == JSOP_ENDITER, at ../jsinterp.cpp:7027
Trace/BPT trap

Comment 1

[Jason Orendorff [:jorendorff]]

function f() {
  return [w for (x in [0])];
}
trap(f, 28, '');
f();

The patch in bug 463239 fixes this.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #1)
> function f() {
>   return [w for (x in [0])];
> }
> trap(f, 28, '');
> f();
> 
> The patch in bug 463239 fixes this.

No, the patch does not.

On TM tip, this asserts somewhere else:

$ ./js-dbg-tm-intelmac
js> function f() {
  return [w for (x in [0])];
}
js> trap(f, 28, '');
js> f();
Assertion failure: JSVAL_IS_OBJECT(v), at ../jsapi.h:97

and crashes opt TM js shell (-j not needed) near null at js_Interpret. This assertion/crash does not occur in 1.9.0.x, so I think this is a regression.

Comment 3

[Jason Orendorff [:jorendorff]]

Gary, surprisingly enough, this is not a bug.  :)

The bytecode has changed (in this case, the new JSOP_LOOP instruction) and so 28 is no longer one of the bytecode offsets in that function.

js> dis(f)
main:
00000:  newinit 3
00002:  enterblock depth 1 {x: 0}
00005:  zero
00006:  newarray 1
00010:  iter 1
00012:  goto 25 (13)
00015:  loop
00016:  forlocal 1
00019:  name "w"
00022:  arraypush 0
00025:  nextiter
00026:  ifne 15 (-11)
00029:  enditer
00030:  leaveblock 1
00033:  endinit
00034:  return
00035:  stop

So now to test this, you have to trap the instruction at offset 29.  Then it behaves as expected (f() throws a ReferenceError, unless you defined w).

Comment 4

[Jason Orendorff [:jorendorff]]

I think perhaps the right way to write a test case for stuff like this is to make the test call dis() and parse the output to determine all bytecode offsets in the function. Then just trap them all and run the test.

Comment 5

[Jesse Ruderman]

We could steal code from jsfunfuzz for that -- that's how jsfunfuzz found the bug in the first place.

Comment 6

[(not currently active) Ted Mielczarek]

Not blocking per comment 3.

Comment 7

[Jesse Ruderman]

More like WFM per comment 3.