Bug 144954

Created attachment 253037 [details] Patch

Comment on attachment 253037 [details] Patch Small refactoring. Landing of this page allow to move to the base implemention of the arrow function.

Comment on attachment 253037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253037&action=review > Source/JavaScriptCore/ChangeLog:2 > + Small refactoring of the parcer.cpp file. It is necessary to increase readability of the patch Add a blank line here. > Source/JavaScriptCore/parser/Parser.cpp:1333 > - > + Revert.

Comment on attachment 253037 [details] Patch Rejecting attachment 253037 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 253037, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/4708680951922688

Created attachment 253058 [details] Patch

Created attachment 253059 [details] Patch

Comment on attachment 253059 [details] Patch Clearing flags on attachment: 253059 Committed r184313: <http://trac.webkit.org/changeset/184313>

All reviewed patches have been landed. Closing bug.

Looks like this patch introduced a crash on 32-bit builds: https://build.webkit.org/builders/Apple%20Yosemite%2032-bit%20JSC%20%28BuildAndTest%29/builds/3967/steps/webkit-32bit-jsc-test/logs/stdio https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/10038/steps/webkit-32bit-jsc-test/logs/stdio jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: PASS ({get a([x]){}}) threw exception SyntaxError: Unexpected token '['. getter functions must have no parameters.. jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: ASSERTION FAILED: !hasError() jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: /Volumes/Data/slave/yosemite-32bitJSC-debug/build/Source/JavaScriptCore/parser/Parser.h(823) : JSC::Parser::SavePoint JSC::Parser<JSC::Lexer<unsigned char> >::createSavePoint() [LexerType = JSC::Lexer<unsigned char>] jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 1 0xb1619d WTFCrash jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 2 0x8f7bf2 JSC::Parser<JSC::Lexer<unsigned char> >::createSavePoint() jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 3 0x9853b1 JSC::SyntaxChecker::SourceElements JSC::Parser<JSC::Lexer<unsigned char> >::parseSourceElements<JSC::SyntaxChecker>(JSC::SyntaxChecker&, JSC::SourceElementsMode) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 4 0x984cad JSC::ASTBuilder::FunctionBody JSC::Parser<JSC::Lexer<unsigned char> >::parseFunctionBody<JSC::ASTBuilder>(JSC::ASTBuilder&, int, int, int, JSC::ConstructorKind) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 5 0x979b8f bool JSC::Parser<JSC::Lexer<unsigned char> >::parseFunctionInfo<JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::FunctionRequirements, JSC::FunctionParseMode, bool, JSC::ConstructorKind, JSC::SuperBinding, int, JSC::ParserFunctionInfo<JSC::ASTBuilder>&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 6 0x982077 JSC::ASTBuilder::Property JSC::Parser<JSC::Lexer<unsigned char> >::parseGetterSetter<JSC::ASTBuilder>(JSC::ASTBuilder&, bool, JSC::PropertyNode::Type, unsigned int, JSC::ConstructorKind, JSC::SuperBinding) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 7 0x980834 JSC::ASTBuilder::Property JSC::Parser<JSC::Lexer<unsigned char> >::parseProperty<JSC::ASTBuilder>(JSC::ASTBuilder&, bool) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 8 0x97d562 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseObjectLiteral<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 9 0x976d8f JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parsePrimaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 10 0x975400 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 11 0x9740fe JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 12 0x9731eb JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 13 0x9726df JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 14 0x971acb JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseAssignmentExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 15 0x97109d JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 16 0x976df0 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parsePrimaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 17 0x975400 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 18 0x9740fe JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 19 0x9731eb JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 20 0x9726df JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 21 0x971acb JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseAssignmentExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 22 0x97109d JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseExpression<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 23 0x970da7 JSC::ASTBuilder::Statement JSC::Parser<JSC::Lexer<unsigned char> >::parseExpressionStatement<JSC::ASTBuilder>(JSC::ASTBuilder&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 24 0x965fe9 JSC::ASTBuilder::Statement JSC::Parser<JSC::Lexer<unsigned char> >::parseStatement<JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::Identifier const*&, unsigned int*) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 25 0x8f47a8 JSC::ASTBuilder::SourceElements JSC::Parser<JSC::Lexer<unsigned char> >::parseSourceElements<JSC::ASTBuilder>(JSC::ASTBuilder&, JSC::SourceElementsMode) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 26 0x8f3f0e JSC::Parser<JSC::Lexer<unsigned char> >::parseInner() jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 27 0x1dce9c std::__1::unique_ptr<JSC::EvalNode, std::__1::default_delete<JSC::EvalNode> > JSC::Parser<JSC::Lexer<unsigned char> >::parse<JSC::EvalNode>(JSC::ParserError&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 28 0x1dbdda std::__1::unique_ptr<JSC::EvalNode, std::__1::default_delete<JSC::EvalNode> > JSC::parse<JSC::EvalNode>(JSC::VM*, JSC::SourceCode const&, JSC::FunctionParameters*, JSC::Identifier const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::JSParserCodeType, JSC::ParserError&, JSC::JSTextPosition*, JSC::ConstructorKind, JSC::ThisTDZMode) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 29 0x1dab4c JSC::UnlinkedEvalCodeBlock* JSC::CodeCache::getGlobalCodeBlock<JSC::UnlinkedEvalCodeBlock, JSC::EvalExecutable>(JSC::VM&, JSC::EvalExecutable*, JSC::SourceCode const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::ThisTDZMode, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 30 0x1d918f JSC::CodeCache::getEvalCodeBlock(JSC::VM&, JSC::EvalExecutable*, JSC::SourceCode const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::ThisTDZMode, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: 31 0x76d821 JSC::JSGlobalObject::createEvalCodeBlock(JSC::ExecState*, JSC::EvalExecutable*, JSC::ThisTDZMode) jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: test_script_18968: line 2: 4296 Segmentation fault: 11 "$@" ../../../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --enableFunctionDotArguments\=true resources/standalone-pre.js object-literal-syntax.js resources/standalone-post.js jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout: ERROR: Unexpected exit code: 139 FAIL: jsc-layout-tests.yaml/js/script-tests/object-literal-syntax.js.layout

Actually, it's nothing to do with 32-bit-ness. It's an assertion failure.

Re-opened since this is blocked by bug 144974

More crash traces: https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r184313%20(4304)/results.html https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK1%20(Tests)/r184313%20(4641)/results.html

Comment on attachment 253059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253059&action=review > Source/JavaScriptCore/parser/Parser.cpp:1314 > { It's also weird that we we're returning bools and ints from this function. I think there is probably some case that isn't being thought through. > Source/JavaScriptCore/parser/Parser.cpp:1363 > + int parametersStart = parseFunctionParamters(context, requirements, mode, nameIsInContainingScope, functionScope, info); You might need to propagate errors here.

Created attachment 253119 [details] Patch

Comment on attachment 253059 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253059&action=review >> Source/JavaScriptCore/parser/Parser.cpp:1314 >> { > > It's also weird that we we're returning bools and ints from this function. > I think there is probably some case that isn't being thought through. I'm quite new in webkit, so I tried to follow exist examples. Could you please suggest what I should change in this function? >> Source/JavaScriptCore/parser/Parser.cpp:1363 >> + int parametersStart = parseFunctionParamters(context, requirements, mode, nameIsInContainingScope, functionScope, info); > > You might need to propagate errors here. Done in new patch

(In reply to comment #10) > Actually, it's nothing to do with 32-bit-ness. It's an assertion failure. I see, my fault. I always run tests in release mode, because in debug mode it is run so slowly on my Laptop. Will check in debug mode also before load patch.

Comment on attachment 253119 [details] Patch Clearing flags on attachment: 253119 Committed r184349: <http://trac.webkit.org/changeset/184349>

All reviewed patches have been landed. Closing bug.