375794 - Incorrect decompilation with array comprehension in catchguard

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

"c" becomes "[]":

js> function() { try { } catch(a if [b for each (c in [])]) { } } 
function () {
    try {
    } catch (a if [b for each ([] in [])]) {
    }
}

I think this also causes
Assertion failure: (uintN)i < ss->top, at jsopcode.c:2483

Comment 1

[Brendan Eich [:brendan]]

Did the fix for bug 351102 cause this bug? In any event, the decompiler stack is not being managed to include the necessary slot for the exception.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Should be self-explanatory. Variation on the case shows the need for conditioning in JSOP_LEAVEBLOCK:

function() { try { } catch(a if [b for each (c in [])]) { print(a) } catch (e) { print(e) }}

/be

Comment 3

[Igor Bukanov]

Comment on attachment 263378 [details] [diff] [review]
fix

>Index: jsopcode.c
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jsopcode.c,v
>retrieving revision 3.228
>diff -p -u -8 -r3.228 jsopcode.c
>--- jsopcode.c	1 May 2007 18:07:44 -0000	3.228
>+++ jsopcode.c	1 May 2007 19:08:09 -0000
>@@ -2426,44 +2426,57 @@ Decompile(SprintStack *ss, jsbytecode *p
>                   case SRC_CATCH:
...
>                     if (*pc == JSOP_DUP) {
>                         sn2 = js_GetSrcNote(jp->script, pc);
>                         if (sn2 && SN_TYPE(sn2) == SRC_HIDDEN) {
>                             /*
>                              * This is a hidden dup to save the exception for
>-                             * later. It must exist only when the catch has
>-                             * an exception guard.
>+                             * later. It is emitted only when the catch has an
>+                             * exception guard.
>                              */

As I understand, the decompiler has to model the stack precisely as the interpreter does for asserts and stack calculations based on jsopcode.tbl to work. So this dup is not hidden, but rather very visible! Thus I suggest to remove the hidden node from the guarded catch dup and rather annotate the following one with SRC_DESTRUCT.

Otherwise it seems OK.

Comment 4

[Brendan Eich [:brendan]]

Attachment: fix, v2

Good point, thanks.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk:

js/src/jsemit.c 3.246
js/src/jsopcode.c 3.229

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: patch I committed

Fixed a recent unpicked whitespace nit in jsopcode.c.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

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

Comment 8

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 linux/mac* 2007-05-05 shell.