Bug 142863 – SyntaxChecker assertion is trapped with computed property name and getter

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

The following input crashes with SyntaxChecker assersion.

(function () {
    var object = {
        [Symbol.unscopables]: {
            Cocoa: true,
            Cappuccino: true
        },

        get Cocoa() {
            throw new Error("bad trap");
        }
        Cappuccino: null
    };
}());

And dump is the following.

ASSERTION FAILED: property.name
../../../Source/JavaScriptCore/parser/SyntaxChecker.h(277) : const JSC::Identifier* JSC::SyntaxChecker::getName(const JSC::SyntaxChecker::Property&) const
1   0x7f69dc2b99a3 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7f69dc2b99a3]
2   0x7f69dc003530 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZNK3JSC13SyntaxChecker7getNameERKNS0_8PropertyE+0x40) [0x7f69dc003530]
3   0x7f69dc07cac6 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE24parseStrictObjectLiteralINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x250) [0x7f69dc07cac6]
4   0x7f69dc07d567 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE18parseObjectLiteralINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x3b9) [0x7f69dc07d567]
5   0x7f69dc079b37 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE22parsePrimaryExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x323) [0x7f69dc079b37]
6   0x7f69dc07426e /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseMemberExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x140) [0x7f69dc07426e]
7   0x7f69dc07fe31 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE20parseUnaryExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x369) [0x7f69dc07fe31]
8   0x7f69dc07c4c3 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseBinaryExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0xad) [0x7f69dc07c4c3]
9   0x7f69dc079527 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE26parseConditionalExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x41) [0x7f69dc079527]
10  0x7f69dc071be0 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE25parseAssignmentExpressionINS_13SyntaxCheckerEEENT_10ExpressionERS6_+0x1b8) [0x7f69dc071be0]
11  0x7f69dc068a91 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE23parseVarDeclarationListINS_13SyntaxCheckerEEENT_10ExpressionERS6_RiRNS6_21DeconstructionPatternERS7_RNS_14JSTextPositionESE_SE_NS3_25VarDeclarationListContextE+0x3a1) [0x7f69dc068a91]
12  0x7f69dc05856f /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE19parseVarDeclarationINS_13SyntaxCheckerEEENT_9StatementERS6_+0xd3) [0x7f69dc05856f]
13  0x7f69dc052b7f /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE14parseStatementINS_13SyntaxCheckerEEENT_9StatementERS6_RPKNS_10IdentifierEPj+0x12b) [0x7f69dc052b7f]
14  0x7f69dc04ae8a /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE19parseSourceElementsINS_13SyntaxCheckerEEENT_14SourceElementsERS6_NS_18SourceElementsModeE+0x6c) [0x7f69dc04ae8a]
15  0x7f69dc03dd56 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE17parseFunctionBodyINS_10ASTBuilderEEENT_12FunctionBodyERS6_NS_15ConstructorKindE+0x176) [0x7f69dc03dd56]
16  0x7f69dc028660 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE17parseFunctionInfoINS_10ASTBuilderEEEbRT_NS_20FunctionRequirementsENS_17FunctionParseModeEbNS_15ConstructorKindERNS_18ParserFunctionInfoIS6_EE+0xfd8) [0x7f69dc028660]
17  0x7f69dc049764 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE22parsePrimaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x1f0) [0x7f69dc049764]
18  0x7f69dc03bbd5 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseMemberExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x13f) [0x7f69dc03bbd5]
19  0x7f69dc055e31 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE20parseUnaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x32f) [0x7f69dc055e31]
20  0x7f69dc0502d6 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseBinaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0xba) [0x7f69dc0502d6]
21  0x7f69dc046b6f /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE26parseConditionalExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x41) [0x7f69dc046b6f]
22  0x7f69dc0380ea /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE25parseAssignmentExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x250) [0x7f69dc0380ea]
23  0x7f69dc02936b /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE15parseExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x9b) [0x7f69dc02936b]
24  0x7f69dc0498e9 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE22parsePrimaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x375) [0x7f69dc0498e9]
25  0x7f69dc03bbd5 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseMemberExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x13f) [0x7f69dc03bbd5]
26  0x7f69dc055e31 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE20parseUnaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x32f) [0x7f69dc055e31]
27  0x7f69dc0502d6 /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE21parseBinaryExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0xba) [0x7f69dc0502d6]
28  0x7f69dc046b6f /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE26parseConditionalExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x41) [0x7f69dc046b6f]
29  0x7f69dc0380ea /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE25parseAssignmentExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x250) [0x7f69dc0380ea]
30  0x7f69dc02936b /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE15parseExpressionINS_10ASTBuilderEEENT_10ExpressionERS6_+0x9b) [0x7f69dc02936b]
31  0x7f69dc01d76f /home/yusuke/dev/WebKit/WebKitBuild/unscopable/Debug/lib/libjavascriptcoregtk-4.0.so.18(_ZN3JSC6ParserINS_5LexerIhEEE24parseExpressionStatementINS_10ASTBuilderEEENT_9StatementERS6_+0x5f) [0x7f69dc01d76f]
[1]    8376 segmentation fault (core dumped)  WebKitBuild/unscopable/Debug/bin/jsc tmp/t10.js

[Joseph Pecoraro]

Potentially interesting is that the snippet is indeed invalid (missing comma between the getter and Cappuccino property.

[Joseph Pecoraro]

Interesting, it looks like this code was added in r157724 with computed properties, and  was meant to handle a property without a known identifier name (computed properties) but still ASSERTs if that is the case.

I'll look into it. I believe this is actually correct behavior without the ASSERT, so this might just be an overzealous ASSERT, or we should handle the "possibly null" case differently.

[Joseph Pecoraro]

Further reduction. Include any computed property with another property:

<script>
(function() {
    "use strict";
    var object = {
        ["x"]: 1,
        x: 2,
    }
})();
</script>

In strict mode, property redefinition (duplicate property names) is disallowed:

  js> ({x:1, x:1})
  SyntaxError: Attempted to redefine property 'x'.

So this we are trying to add the property name for the computed property to the list, and we, as expected, don't have the property name yet. I believe it is safe to just skip computed property names here, but I want to verify that.

[Joseph Pecoraro]

Err, it seems the duplicate property check can be removed for object literals and classes.

ES6 seems to have changed here:
<https://people.mozilla.org/~jorendorff/es6-draft.html#sec-additions-and-changes-that-introduce-incompatibilities-with-prior-editions>

> Annex E: Additions and Changes That Introduce Incompatibilities with Prior Editions
> 12.2.5.1: In Edition 6, it is no longer an early error to have duplicate property names in Object Initializers.

> July 18, 2014 Draft Rev 26
> Eliminated duplicate property name restrictions on object literals and class definitions

> Deleted:
> ObjectLiteral : { PropertyDefinitionList }
> ObjectLiteral : { PropertyDefinitionList , }
> 
> <#>It is a Syntax Error if PropertyNameList of PropertyDefinitionList contains any duplicate entries, unless one of the following conditions aretrueforeachduplicateentry: 
> <#>The source code corresponding to PropertyDefinitionList is not strict code and all occurrences in the list of the duplicated entry were obtained from productions of the form PropertyDefinition : PropertyName : AssignmentExpression.
> <#>The duplicated entry occurs exactly twice in the list and one occurrence was obtained from a get accessor MethodDefinition and the other occurrence was obtained from a set accessor MethodDefinition.

For starters, I'll remove the ASSERT, then look at removing the duplicate property thing.

[Joseph Pecoraro]

Yeah, I tried to remove the duplicate properties check, and it turns out that just exposes a new can of worms due to assumptions PropertyListNode::emitBytecode makes. I'll file a separate bug on that, and just ensure the tests for this actually work and remove the invalid ASSERT.

[Joseph Pecoraro]

Created attachment 249090 [details]
[PATCH] Proposed Fix

It took an embarrassingly long time to understand why the existing test wasn't already asserting. That was because the ASSERT was in the SyntaxChecker path, which is different from regular parsing which did not have this ASSERT. All this, just to remove the assert!

[WebKit Commit Bot]

Comment on attachment 249090 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 249090

Committed r181807: <http://trac.webkit.org/changeset/181807>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

Great!