429252 - trap changes decompilation of "{ let x }"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> function f() { { let x } }
js> f
function f() {
    {
        let x;
    }
}

js> trap(f, 0, "")
js> f
function f() {
    {
        let x;
    }
    let x;
}

Comment 1

[Brian Crowder]

This is fixed by my WIP patch from bug 422137.

Comment 2

[Bob Clary [:bc] (inactive)]

Attachment: js1_7/decompilation/regress-429252.js

I'll check in when the tree reopens

Comment 3

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_7/decompilation/regress-429252.js,v  <--  regress-429252.js
initial revision: 1.1

This test fails on mac os x, leopard at least. 

debug: Assertion failure: ATOM_IS_STRING(atom), at jsinterp.c:4946
opt:   bus error

Note to self: the automation is not collecting the proper error messages.

Comment 4

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/public-failures.txt,v  <--  public-failures.txt
new revision: 1.70; previous revision: 1.69

Comment 5

[Bob Clary [:bc] (inactive)]

centos5 x86_64:
nada

fedora6/centos i686 32bit:
Assertion failure: (script)->objectsOffset != 0, at jsinterp.c:6489

fedora8 32bit:
Assertion failure: (uint32)((atoms - script->atomMap.vector + ((uintN)(((regs.pc + 0)[1] << 8) | (regs.pc + 0)[2])))) < objects_->length

crowder, do you want a new bug or reopen this one?

Comment 6

[Brian Crowder]

Let's keep it here.

Comment 7

[Brian Crowder]

Attachment: undoing my silliness from the previous patch

I really don't know what I was thinking with the previous patch.  This logic must occur in DecompileCode (which is not recursive).  Otherwise, we end up doing pointer math on the untraped "code", using an unmodified pc.  This causes us to go out-of-bounds on the code array we're untrapping.

This bug is very serious and should block.

Comment 8

[Brian Crowder]

I'll create a follow-up bug to introduce some assertions in jsdbgapi.c:js_UntrapScriptCode which would have saved us from this.  I'd rather not do that here.

Comment 9

[Bob Clary [:bc] (inactive)]

also js1_5/extensions/regress-429264.js

Comment 10

[Mike Shaver (:shaver emeritus)]

Comment on attachment 318295 [details] [diff] [review]
undoing my silliness from the previous patch

r+a=shaver, thanks for the diff showing the net-of-previous changes.

Comment 11

[Brian Crowder]

jsopcode.c: 3.313

Comment 12

[Bob Clary [:bc] (inactive)]

v 1.9.0

Comment 13

[Brian Crowder]

Clearing my blocking request from ages ago.