search

keeleysam / tenfourfox

Automatically exported from code.google.com/p/tenfourfox
0 stars 0 forks source link

Conformance regression in JavaScript interpreter #212

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
http://code.google.com/p/aurorafox/issues/detail?id=34

Starting program: /Volumes/BruceDeuce/src/mozilla-20b/obj-ff-dbg/dist/bin/js -f 
js/src/tests/shell.js -f js/src/tests/js1_6/extensions/regress-352392.js
[...]
BUGNUMBER: 352392
STATUS: Do not hang/crash |for each| over object with getter set to map
 FAILED! [reported from test()] Do not hang/crash |for each| over object with getter set to map : Expected value 'SyntaxError: invalid for each loop', Actual value 'SyntaxError: missing ( after for' 

This worked in 17.

Original issue reported on code.google.com by classi...@floodgap.com on 7 Mar 2013 at 4:13

GoogleCodeExporter commented 9 years ago 
bruce:/home/spectre/src/bruce/mozilla-20b/js/src/% grep after.for *
js.msg:MSG_DEF(JSMSG_PAREN_AFTER_FORMAL,      71, 0, JSEXN_SYNTAXERR, "missing 
) after formal parameters")
js.msg:MSG_DEF(JSMSG_PAREN_AFTER_FOR,         85, 0, JSEXN_SYNTAXERR, "missing 
( after for")
js.msg:MSG_DEF(JSMSG_SEMI_AFTER_FOR_INIT,     86, 0, JSEXN_SYNTAXERR, "missing 
; after for-loop initializer")
js.msg:MSG_DEF(JSMSG_SEMI_AFTER_FOR_COND,     87, 0, JSEXN_SYNTAXERR, "missing 
; after for-loop condition")
js.msg:MSG_DEF(JSMSG_PAREN_AFTER_FOR_CTRL,    88, 0, JSEXN_SYNTAXERR, "missing 
) after for-loop control")
js.msg:MSG_DEF(JSMSG_NAME_AFTER_FOR_PAREN,   210, 0, JSEXN_SYNTAXERR, "missing 
name after for (")
js.msg:MSG_DEF(JSMSG_IN_AFTER_FOR_NAME,      211, 0, JSEXN_SYNTAXERR, "missing 
'in' or 'of' after for")

Both the wrong error and the right error are reported out of 
frontend/Parser.cpp.

Original comment by classi...@floodgap.com on 7 Mar 2013 at 4:17

GoogleCodeExporter commented 9 years ago 
js.msg:MSG_DEF(JSMSG_BAD_FOR_EACH_LOOP,      193, 0, JSEXN_SYNTAXERR, "invalid 
for each loop")

Original comment by classi...@floodgap.com on 7 Mar 2013 at 4:37

GoogleCodeExporter commented 9 years ago 
JSMSG_PAREN_AFTER_FOR only occurs in two places. Both of them look like this:

    if (allowsForEachIn() && tokenStream.matchToken(TOK_NAME)) { << X
        if (tokenStream.currentToken().name() == context->names().each) << Y
            pn->pn_iflags = JSITER_FOREACH;
        else
            tokenStream.ungetToken();
    }

    [...]
    MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_AFTER_FOR);

Since the each doesn't get accepted, then it hits the AFTER_FOR test. In 17, it 
looked like this:

    if (tokenStream.matchToken(TOK_NAME)) {
        if (tokenStream.currentToken().name() == context->runtime->atomState.eac
hAtom)
            pn->pn_iflags = JSITER_FOREACH;
        else
            tokenStream.ungetToken();
    }

Blame log says change X was 
https://bugzilla.mozilla.org/show_bug.cgi?id=804834. This is probably not the 
causing bug.

However, change Y was https://bugzilla.mozilla.org/show_bug.cgi?id=790349 and 
this might have some endian problems.

Original comment by classi...@floodgap.com on 7 Mar 2013 at 4:54

GoogleCodeExporter commented 9 years ago 
I'm leaning towards this being an issue with handles (a la issue 165).

Original comment by classi...@floodgap.com on 7 Mar 2013 at 5:10

GoogleCodeExporter commented 9 years ago 
Still, something must happen to whack it, because this will work:

js> var k = ["a", "b", "c"];
js> for each (i in k) { print(i); }
a
b
c
js> (function() { for each (i in k) { print(i); }})();
a
b
c

Original comment by classi...@floodgap.com on 7 Mar 2013 at 5:13

GoogleCodeExporter commented 9 years ago 
Browsing through https://bugzilla.mozilla.org/show_bug.cgi?id=790349 I found 
one thing that might be dangerous:

Patch 4 
(https://bugzilla.mozilla.org/attachment.cgi?id=660190&action=diff#a/js/src/jsat
ominlines.h_sec3) makes me suspect that there may be some assumptions about the 
size of class FixedHeapPtr from Patch 3 
(https://bugzilla.mozilla.org/attachment.cgi?id=660188&action=diff#a/js/src/gc/B
arrier.h_sec3). To me it seems that an object of type FixedHeapPtr is assumed 
to be the same size as a simple pointer. However my experience with Leopard 
WebKit (although it's still built using gcc 4.2) tells me that class objects in 
the OS X PowerPC ABI frequently don't get compiled to the same size as for 
other ABIs. Maybe putting some sort of "pragma pack(push/pop)" around the class 
declaration is needed plus a compile time assert to verify that the object 
really gets compiled to the needed size.

Original comment by Tobias.N...@gmail.com on 7 Mar 2013 at 8:37

GoogleCodeExporter commented 9 years ago 
It looks like a problem actually with the token stream. Given these debug 
changes in both places:

fprintf(stderr, "allows: %i ", allowsForEachIn());
    if (allowsForEachIn() && tokenStream.matchToken(TOK_NAME)) {
fprintf(stderr, "cur_tok %i each %i ",
        tokenStream.currentToken().name(), context->names().each);
        if (tokenStream.currentToken().name() == context->names().each)
            pn->pn_iflags = JSITER_FOREACH;
        else
            tokenStream.ungetToken();
    }
fprintf(stderr, "pn_iflags %i\n", pn->pn_iflags);

js> var k = ["a", "b", "c"]; for each (i in k) { print(i); }
allows: 1 cur_tok 43065920 each 43065920 pn_iflags 2
allows: 1 cur_tok 43065920 each 43065920 pn_iflags 2
a
b
c
js> 
bruce:/Volumes/BruceDeuce/src/mozilla-20b/% 
/Volumes/BruceDeuce/src/mozilla-20b/obj-ff-dbg/dist/bin/js -f 
js/src/tests/shell.js -f js/src/tests/js1_6/extensions/regress-352392.js
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
BUGNUMBER: 352392
STATUS: Do not hang/crash |for each| over object with getter set to map
allows: 0 pn_iflags 0
 FAILED! [reported from test()] Do not hang/crash |for each| over object with getter set to map : Expected value 'SyntaxError: invalid for each loop', Actual value 'SyntaxError: missing ( after for' 

Notice that the token appears fine in the shell example but not in the test. 
Therefore, matchToken must be failing.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 2:03

GoogleCodeExporter commented 9 years ago 
It's something about shell.js. On y = {}; (y.a = [2 for each (p in [])])(); 
(js1_6/Regress/regress-350417.js):

% obj-ff-dbg/dist/bin/js
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
js> y = {}; (y.a = [2 for each (p in [])])();
allows: 1 cur_tok 43065920 each 43065920 pn_iflags -65536
allows: 1 cur_tok 43065920 each 43065920 pn_iflags -65536
typein:1:9 TypeError: [] is not a function
js> 
bruce:/Volumes/BruceDeuce/src/mozilla-20b/% obj-ff-dbg/dist/bin/js -f 
js/src/tests/shell.js -f js/src/tests/!$
obj-ff-dbg/dist/bin/js -f js/src/tests/shell.js -f 
js/src/tests/js1_6/Regress/regress-350417.js
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 0 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
allows: 1 pn_iflags 0
BUGNUMBER: 350417
STATUS: Do not crash decompiling "is not function" msg
allows: 0 pn_iflags -65536
 FAILED! [reported from test()] Do not crash decompiling "is not function" msg : Expected value 'TypeError: [] is not a function', Actual value 'SyntaxError: missing ( after for' 

So it works fine if run by itself, but the test harness shell actually pushes 
it over the edge.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 2:39

GoogleCodeExporter commented 9 years ago 
That test actually passes in 19. Therefore, it must be bug 819509, 808286, 
821470, 821103, 822283 or 823310.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 2:53

GoogleCodeExporter commented 9 years ago 
Not 821103
Not 823310
Not 822283
Not 821470
Not 808286

It's got to be something in 819509 which is of course the biggest one.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 3:09

GoogleCodeExporter commented 9 years ago 
Not https://hg.mozilla.org/mozilla-central/rev/ee6cd137eb24
Not https://hg.mozilla.org/mozilla-central/rev/a55201f7b07b
Not https://hg.mozilla.org/mozilla-central/rev/ef91166a4405
Not https://hg.mozilla.org/mozilla-central/rev/3a07784f694d
Not https://hg.mozilla.org/mozilla-central/rev/d29ea23dde5f
Not https://hg.mozilla.org/mozilla-central/rev/72f55526999a
Not https://hg.mozilla.org/mozilla-central/rev/d2c8c4d0e9c8

Maybe https://hg.mozilla.org/mozilla-central/rev/135f6317a5d1
Maybe https://hg.mozilla.org/mozilla-central/rev/fa10b335dd65

Original comment by classi...@floodgap.com on 10 Mar 2013 at 3:22

GoogleCodeExporter commented 9 years ago 
Nope. It's none of those. It's M804834 after all. If I remove the 
allowsForEachIn(), it works in all cases. And the reason is ...

if (typeof version != 'undefined')
{
  version(0);
}

in shell.js. Comment out version(0) (instead of disabling allowsForEachIn), and 
the tests pass. Maybe we're not handling the test harness correctly, but it 
seems to work properly when the right version of JS is specified.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 3:55

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 10 Mar 2013 at 11:54

GoogleCodeExporter commented 9 years ago 
Dropping to low since this hasn't been an issue for testing so far.

Original comment by classi...@floodgap.com on 12 Feb 2014 at 3:08