Title: [181404] trunk
Revision
181404
Author
rn...@webkit.org
Date
2015-03-11 14:03:24 -0700 (Wed, 11 Mar 2015)

Log Message

Calling super() in a base class results in a crash
https://bugs.webkit.org/show_bug.cgi?id=142563

Reviewed by Filip Pizlo.

Source/_javascript_Core:

The bug was caused by BytecodeGenerator trying to generate "super" _expression_ inside the constructor of a base class.
Disallow that by keeping track of whether "super" has been used in the current scope or not (needsSuperBinding flag)
and then throwing a syntax error in parseFunctionInfo if it was used and the current scope wasn't the constructor of
a derived class.

* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseFunctionInfo): Don't allow super() or super.foo outside the constructor of a derived class.
(JSC::Parser<LexerType>::parseClass): Pass in the constructor kind to parseGetterSetter.
(JSC::Parser<LexerType>::parseGetterSetter): Ditto to parseFunctionInfo.
(JSC::Parser<LexerType>::parseMemberExpression): Set needsSuperBinding flag true on the containing scope.
* parser/Parser.h:
(JSC::Scope::Scope):
(JSC::Scope::needsSuperBinding): Added.
(JSC::Scope::setNeedsSuperBinding): Added.

LayoutTests:

Added more test cases to an existing test.

* js/class-syntax-super-expected.txt:
* js/script-tests/class-syntax-super.js:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (181403 => 181404)


--- trunk/LayoutTests/ChangeLog	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/LayoutTests/ChangeLog	2015-03-11 21:03:24 UTC (rev 181404)
@@ -1,3 +1,15 @@
+2015-03-11  Ryosuke Niwa  <rn...@webkit.org>
+
+        Calling super() in a base class results in a crash
+        https://bugs.webkit.org/show_bug.cgi?id=142563
+
+        Reviewed by Filip Pizlo.
+
+        Added more test cases to an existing test.
+
+        * js/class-syntax-super-expected.txt:
+        * js/script-tests/class-syntax-super.js:
+
 2015-03-11  Said Abou-Hallawa  <sabouhall...@apple.com>
 
         svg/animations/smil-leak-*.svg tests are flaky.

Modified: trunk/LayoutTests/js/class-syntax-super-expected.txt (181403 => 181404)


--- trunk/LayoutTests/js/class-syntax-super-expected.txt	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/LayoutTests/js/class-syntax-super-expected.txt	2015-03-11 21:03:24 UTC (rev 181404)
@@ -35,6 +35,12 @@
 PASS new (class extends null { constructor() { } }) threw exception TypeError: Cannot return a non-object type in the constructor of a derived class..
 PASS new (class extends null { constructor() { return 1; } }) threw exception TypeError: Cannot return a non-object type in the constructor of a derived class..
 PASS new (class extends null { constructor() { super() } }) did not throw exception.
+PASS new (class { constructor() { super() } }) threw exception SyntaxError: Cannot call super() in a base class constructor..
+PASS function x() { super(); } threw exception SyntaxError: Cannot call super() outside of a class constructor..
+PASS new (class extends Object { constructor() { function x() { super() } } }) threw exception SyntaxError: Cannot call super() outside of a class constructor..
+PASS new (class extends Object { constructor() { function x() { super.method } } }) threw exception SyntaxError: super can only be used in a method of a derived class..
+PASS function x() { super.method(); } threw exception SyntaxError: super can only be used in a method of a derived class..
+PASS function x() { super(); } threw exception SyntaxError: Cannot call super() outside of a class constructor..
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/js/script-tests/class-syntax-super.js (181403 => 181404)


--- trunk/LayoutTests/js/script-tests/class-syntax-super.js	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/LayoutTests/js/script-tests/class-syntax-super.js	2015-03-11 21:03:24 UTC (rev 181404)
@@ -61,5 +61,11 @@
 shouldThrow('new (class extends null { constructor() { } })', '"TypeError: Cannot return a non-object type in the constructor of a derived class."');
 shouldThrow('new (class extends null { constructor() { return 1; } })', '"TypeError: Cannot return a non-object type in the constructor of a derived class."');
 shouldNotThrow('new (class extends null { constructor() { super() } })');
+shouldThrow('new (class { constructor() { super() } })', '"SyntaxError: Cannot call super() in a base class constructor."');
+shouldThrow('function x() { super(); }', '"SyntaxError: Cannot call super() outside of a class constructor."');
+shouldThrow('new (class extends Object { constructor() { function x() { super() } } })', '"SyntaxError: Cannot call super() outside of a class constructor."');
+shouldThrow('new (class extends Object { constructor() { function x() { super.method } } })', '"SyntaxError: super can only be used in a method of a derived class."');
+shouldThrow('function x() { super.method(); }', '"SyntaxError: super can only be used in a method of a derived class."');
+shouldThrow('function x() { super(); }', '"SyntaxError: Cannot call super() outside of a class constructor."');
 
 var successfullyParsed = true;

Modified: trunk/Source/_javascript_Core/ChangeLog (181403 => 181404)


--- trunk/Source/_javascript_Core/ChangeLog	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-03-11 21:03:24 UTC (rev 181404)
@@ -1,3 +1,25 @@
+2015-03-11  Ryosuke Niwa  <rn...@webkit.org>
+
+        Calling super() in a base class results in a crash
+        https://bugs.webkit.org/show_bug.cgi?id=142563
+
+        Reviewed by Filip Pizlo.
+
+        The bug was caused by BytecodeGenerator trying to generate "super" _expression_ inside the constructor of a base class.
+        Disallow that by keeping track of whether "super" has been used in the current scope or not (needsSuperBinding flag)
+        and then throwing a syntax error in parseFunctionInfo if it was used and the current scope wasn't the constructor of
+        a derived class.
+
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseFunctionInfo): Don't allow super() or super.foo outside the constructor of a derived class.
+        (JSC::Parser<LexerType>::parseClass): Pass in the constructor kind to parseGetterSetter.
+        (JSC::Parser<LexerType>::parseGetterSetter): Ditto to parseFunctionInfo.
+        (JSC::Parser<LexerType>::parseMemberExpression): Set needsSuperBinding flag true on the containing scope.
+        * parser/Parser.h:
+        (JSC::Scope::Scope):
+        (JSC::Scope::needsSuperBinding): Added.
+        (JSC::Scope::setNeedsSuperBinding): Added.
+
 2015-03-10  Darin Adler  <da...@apple.com>
 
         Some event handler fixes

Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (181403 => 181404)


--- trunk/Source/_javascript_Core/parser/Parser.cpp	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp	2015-03-11 21:03:24 UTC (rev 181404)
@@ -1377,9 +1377,13 @@
         semanticFailIfTrue(m_vm->propertyNames->eval == *info.name, "'", info.name->impl(), "' is not a valid function name in strict mode");
     }
     if (functionScope->hasDirectSuper()) {
-        bool nameIsConstructor = info.name && *info.name == m_vm->propertyNames->constructor;
-        semanticFailIfTrue(mode != MethodMode || !nameIsConstructor, "Cannot call super() outside of a class constructor");
+        bool isClassConstructor = mode == MethodMode && info.name && *info.name == m_vm->propertyNames->constructor;
+        semanticFailIfTrue(!isClassConstructor, "Cannot call super() outside of a class constructor");
+        semanticFailIfTrue(constructorKind == ConstructorKind::Base, "Cannot call super() in a base class constructor");
     }
+    if (functionScope->needsSuperBinding())
+        semanticFailIfTrue(constructorKind == ConstructorKind::Base, "super can only be used in a method of a derived class");
+
     info.closeBraceOffset = m_token.m_data.offset;
     unsigned closeBraceLine = m_token.m_data.line;
     unsigned closeBraceLineStartOffset = m_token.m_data.lineStartOffset;
@@ -1503,7 +1507,7 @@
         if (isGetter || isSetter) {
             semanticFailIfTrue(isStaticMethod, "Cannot declare a static", stringForFunctionMode(isGetter ? GetterMode : SetterMode));
             nextExpectIdentifier(LexerFlagsIgnoreReservedWords);
-            property = parseGetterSetter(context, alwaysStrictInsideClass, isGetter ? PropertyNode::Getter : PropertyNode::Setter, methodStart, SuperBinding::Needed);
+            property = parseGetterSetter(context, alwaysStrictInsideClass, isGetter ? PropertyNode::Getter : PropertyNode::Setter, methodStart, constructorKind, SuperBinding::Needed);
             failIfFalse(property, "Cannot parse this method");
         } else {
             ParserFunctionInfo<TreeBuilder> methodInfo;
@@ -2018,7 +2022,8 @@
 }
 
 template <typename LexerType>
-template <class TreeBuilder> TreeProperty Parser<LexerType>::parseGetterSetter(TreeBuilder& context, bool strict, PropertyNode::Type type, unsigned getterOrSetterStartOffset, SuperBinding superBinding)
+template <class TreeBuilder> TreeProperty Parser<LexerType>::parseGetterSetter(TreeBuilder& context, bool strict, PropertyNode::Type type, unsigned getterOrSetterStartOffset,
+    ConstructorKind constructorKind, SuperBinding superBinding)
 {
     const Identifier* stringPropertyName = 0;
     double numericPropertyName = 0;
@@ -2033,10 +2038,10 @@
     ParserFunctionInfo<TreeBuilder> info;
     if (type == PropertyNode::Getter) {
         failIfFalse(match(OPENPAREN), "Expected a parameter list for getter definition");
-        failIfFalse((parseFunctionInfo(context, FunctionNoRequirements, GetterMode, false, ConstructorKind::Base, info)), "Cannot parse getter definition");
+        failIfFalse((parseFunctionInfo(context, FunctionNoRequirements, GetterMode, false, constructorKind, info)), "Cannot parse getter definition");
     } else {
         failIfFalse(match(OPENPAREN), "Expected a parameter list for setter definition");
-        failIfFalse((parseFunctionInfo(context, FunctionNoRequirements, SetterMode, false, ConstructorKind::Base, info)), "Cannot parse setter definition");
+        failIfFalse((parseFunctionInfo(context, FunctionNoRequirements, SetterMode, false, constructorKind, info)), "Cannot parse setter definition");
     }
     if (stringPropertyName)
         return context.createGetterOrSetterProperty(location, type, strict, stringPropertyName, info, getterOrSetterStartOffset, superBinding);
@@ -2384,6 +2389,7 @@
     if (baseIsSuper) {
         base = context.superExpr(location);
         next();
+        currentScope()->setNeedsSuperBinding();
     } else
         base = parsePrimaryExpression(context);
 

Modified: trunk/Source/_javascript_Core/parser/Parser.h (181403 => 181404)


--- trunk/Source/_javascript_Core/parser/Parser.h	2015-03-11 20:27:42 UTC (rev 181403)
+++ trunk/Source/_javascript_Core/parser/Parser.h	2015-03-11 21:03:24 UTC (rev 181404)
@@ -114,6 +114,7 @@
         , m_usesEval(false)
         , m_needsFullActivation(false)
         , m_hasDirectSuper(false)
+        , m_needsSuperBinding(false)
         , m_allowsNewDecls(true)
         , m_strictMode(strictMode)
         , m_isFunction(isFunction)
@@ -130,6 +131,7 @@
         , m_usesEval(rhs.m_usesEval)
         , m_needsFullActivation(rhs.m_needsFullActivation)
         , m_hasDirectSuper(rhs.m_hasDirectSuper)
+        , m_needsSuperBinding(rhs.m_needsSuperBinding)
         , m_allowsNewDecls(rhs.m_allowsNewDecls)
         , m_strictMode(rhs.m_strictMode)
         , m_isFunction(rhs.m_isFunction)
@@ -272,6 +274,13 @@
 #endif
     void setHasDirectSuper() { m_hasDirectSuper = true; }
 
+#if ENABLE(ES6_CLASS_SYNTAX)
+    bool needsSuperBinding() { return m_needsSuperBinding; }
+#else
+    bool needsSuperBinding() { return false; }
+#endif
+    void setNeedsSuperBinding() { m_needsSuperBinding = true; }
+
     bool collectFreeVariables(Scope* nestedScope, bool shouldTrackClosedVariables)
     {
         if (nestedScope->m_usesEval)
@@ -366,6 +375,7 @@
     bool m_usesEval : 1;
     bool m_needsFullActivation : 1;
     bool m_hasDirectSuper : 1;
+    bool m_needsSuperBinding : 1;
     bool m_allowsNewDecls : 1;
     bool m_strictMode : 1;
     bool m_isFunction : 1;
@@ -748,7 +758,7 @@
     template <class TreeBuilder> ALWAYS_INLINE TreeArguments parseArguments(TreeBuilder&, SpreadMode);
     template <class TreeBuilder> TreeProperty parseProperty(TreeBuilder&, bool strict);
     template <class TreeBuilder> TreeExpression parsePropertyMethod(TreeBuilder& context, const Identifier* methodName);
-    template <class TreeBuilder> TreeProperty parseGetterSetter(TreeBuilder&, bool strict, PropertyNode::Type, unsigned getterOrSetterStartOffset, SuperBinding = SuperBinding::NotNeeded);
+    template <class TreeBuilder> TreeProperty parseGetterSetter(TreeBuilder&, bool strict, PropertyNode::Type, unsigned getterOrSetterStartOffset, ConstructorKind = ConstructorKind::Base, SuperBinding = SuperBinding::NotNeeded);
     template <class TreeBuilder> ALWAYS_INLINE TreeFunctionBody parseFunctionBody(TreeBuilder&, ConstructorKind);
     template <class TreeBuilder> ALWAYS_INLINE TreeFormalParameterList parseFormalParameters(TreeBuilder&);
     enum VarDeclarationListContext { ForLoopContext, VarDeclarationContext };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to