I think this bug is related with https://bugs.webkit.org/show_bug.cgi?id=135545 variable declarations with no initializer, something like 'var {A};' or 'var [A];', make crash. these declarations also need AST nodes.
found with afl-fuzz
Created attachment 245698 [details] Patch maybe enable profiling would be needed.. I don't know.
Created attachment 245699 [details] Patch
(In reply to comment #0) > I think this bug is related with > https://bugs.webkit.org/show_bug.cgi?id=135545 > variable declarations with no initializer, something like 'var {A};' or 'var > [A];', make crash. > these declarations also need AST nodes. Is it valid ES6 syntax to do something like: var {foo}; foo = 20; ? I'm not sure what the spec says for things like this. Looking it up now.
(In reply to comment #0) > I think this bug is related with > https://bugs.webkit.org/show_bug.cgi?id=135545 > variable declarations with no initializer, something like 'var {A};' or 'var > [A];', make crash. > these declarations also need AST nodes. From reading the spec, I don't think this is valid. If you refer to section 13.2.2, it says that all BindingPattern nodes inside a variable declaration must have an accompanying initializer. That said, JSC should definitely not crash on this. We should raise a parsing error. I think this can be fixed inside Parser::parseVarDeclarationList on line 489+ by raising a parse error for any deconstruction pattern node that does not contain a rhs.
(In reply to comment #5) > (In reply to comment #0) > > I think this bug is related with > > https://bugs.webkit.org/show_bug.cgi?id=135545 > > variable declarations with no initializer, something like 'var {A};' or 'var > > [A];', make crash. > > these declarations also need AST nodes. > > From reading the spec, I don't think this is valid. If you refer to section > 13.2.2, it says that all BindingPattern nodes inside a variable declaration > must have an accompanying initializer. That said, JSC should definitely not > crash on this. We should raise a parsing error. > > I think this can be fixed inside Parser::parseVarDeclarationList on line > 489+ by raising a parse error for any deconstruction pattern node that does > not contain a rhs. It's actually not this easy. parseVarDeclarationList is used both in for statements and in variable declaration statements. This is somewhat unfortunate, anyways, it should be legal to do: for (var {x} of arr) ... but not: var {x};
Created attachment 245734 [details] patch A patch like this should do the trick. It's a bit ugly though passing a flag into parseVarDeclarationList
Comment on attachment 245734 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=245734&action=review > Source/JavaScriptCore/parser/Parser.cpp:381 > + TreeExpression varDecls = parseVarDeclarationList(context, scratch, scratch1, scratch2, scratch3, scratch3, scratch3, false); When we write code like this, we either use an enumeration or we explicitly assign the boolean value to a named local variable. Otherwise, it's hard to know what the parameter means.
(In reply to comment #8) > Comment on attachment 245734 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245734&action=review > > > Source/JavaScriptCore/parser/Parser.cpp:381 > > + TreeExpression varDecls = parseVarDeclarationList(context, scratch, scratch1, scratch2, scratch3, scratch3, scratch3, false); > > When we write code like this, we either use an enumeration or we explicitly > assign the boolean value to a named local variable. Otherwise, it's hard to > know what the parameter means. Yeah, agreed. I'll make it into an enum, it seems cleanest that way.
I forgot to link to the spec section I was referring to: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-variable-statement
Created attachment 245787 [details] patch
Comment on attachment 245787 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=245787&action=review > Source/JavaScriptCore/ChangeLog:10 > + statement, the assignment must have also right hand side value. Typo: *the assignment must also have a right hand side value*
Comment on attachment 245787 [details] patch r=me with your ChangeLog change.
landed in: http://trac.webkit.org/changeset/179682