We should only append ParserArenaDeletable pointers to ParserArena::m_deletableObjects.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jan 2018 21:14:17 +0000 (21:14 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jan 2018 21:14:17 +0000 (21:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182180
<rdar://problem/36460697>

Reviewed by Michael Saboff.

Some parser Node subclasses extend ParserArenaDeletable via multiple inheritance,
but not as the Node's first base class.  ParserArena::m_deletableObjects is
expecting pointers to objects of the shape of ParserArenaDeletable.  We ensure
this by allocating the Node subclass, and casting it to ParserArenaDeletable to
get the correct pointer to append to ParserArena::m_deletableObjects.

To simplify things, we introduce a JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED
(analogous to WTF_MAKE_FAST_ALLOCATED) for use in Node subclasses that extends
ParserArenaDeletable.

* parser/NodeConstructors.h:
(JSC::ParserArenaDeletable::operator new):
* parser/Nodes.h:
* parser/ParserArena.h:
(JSC::ParserArena::allocateDeletable):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227692 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/NodeConstructors.h
Source/JavaScriptCore/parser/Nodes.h
Source/JavaScriptCore/parser/ParserArena.h

index b7d18de..2951bee 100644 (file)
@@ -1,3 +1,27 @@
+2018-01-26  Mark Lam  <mark.lam@apple.com>
+
+        We should only append ParserArenaDeletable pointers to ParserArena::m_deletableObjects.
+        https://bugs.webkit.org/show_bug.cgi?id=182180
+        <rdar://problem/36460697>
+
+        Reviewed by Michael Saboff.
+
+        Some parser Node subclasses extend ParserArenaDeletable via multiple inheritance,
+        but not as the Node's first base class.  ParserArena::m_deletableObjects is
+        expecting pointers to objects of the shape of ParserArenaDeletable.  We ensure
+        this by allocating the Node subclass, and casting it to ParserArenaDeletable to
+        get the correct pointer to append to ParserArena::m_deletableObjects.
+
+        To simplify things, we introduce a JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED 
+        (analogous to WTF_MAKE_FAST_ALLOCATED) for use in Node subclasses that extends
+        ParserArenaDeletable.
+
+        * parser/NodeConstructors.h:
+        (JSC::ParserArenaDeletable::operator new):
+        * parser/Nodes.h:
+        * parser/ParserArena.h:
+        (JSC::ParserArena::allocateDeletable):
+
 2018-01-26  Joseph Pecoraro  <pecoraro@apple.com>
 
         JavaScriptCore builtins should be partially minified in Release builds not Debug builds
index 6f6af97..0a0797e 100644 (file)
@@ -30,9 +30,10 @@ namespace JSC {
         return parserArena.allocateFreeable(size);
     }
 
+    template<typename T>
     inline void* ParserArenaDeletable::operator new(size_t size, ParserArena& parserArena)
     {
-        return parserArena.allocateDeletable(size);
+        return parserArena.allocateDeletable<T>(size);
     }
 
     inline ParserArenaRoot::ParserArenaRoot(ParserArena& parserArena)
index cb51b5e..32a60ea 100644 (file)
@@ -113,9 +113,21 @@ namespace JSC {
 
         // ParserArenaDeletable objects are deleted when the arena is deleted.
         // Clients must not call delete directly on such objects.
-        void* operator new(size_t, ParserArena&);
+        template<typename T> void* operator new(size_t, ParserArena&);
     };
 
+#define JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED_IMPL(__classToNew) \
+        void* operator new(size_t size, ParserArena& parserArena) \
+        { \
+            return ParserArenaDeletable::operator new<__classToNew>(size, parserArena); \
+        }
+
+#define JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(__classToNew) \
+    public: \
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED_IMPL(__classToNew) \
+    private: \
+        typedef int __thisIsHereToForceASemicolonAfterThisMacro
+
     class ParserArenaRoot {
         WTF_MAKE_FAST_ALLOCATED;
     protected:
@@ -238,6 +250,7 @@ namespace JSC {
     };
 
     class VariableEnvironmentNode : public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(VariableEnvironmentNode);
     public:
         typedef DeclarationStacks::FunctionStack FunctionStack;
 
@@ -1413,9 +1426,8 @@ namespace JSC {
     };
 
     class BlockNode : public StatementNode, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(BlockNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         BlockNode(const JSTokenLocation&, SourceElements*, VariableEnvironment&, FunctionStack&&);
 
         StatementNode* singleStatement() const;
@@ -1536,9 +1548,8 @@ namespace JSC {
     };
 
     class ForNode : public StatementNode, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ForNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         ForNode(const JSTokenLocation&, ExpressionNode* expr1, ExpressionNode* expr2, ExpressionNode* expr3, StatementNode*, VariableEnvironment&);
 
     private:
@@ -1553,9 +1564,8 @@ namespace JSC {
     class DestructuringPatternNode;
     
     class EnumerationNode : public StatementNode, public ThrowableExpressionData, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(EnumerationNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         EnumerationNode(const JSTokenLocation&, ExpressionNode*, ExpressionNode*, StatementNode*, VariableEnvironment&);
 
         ExpressionNode* lexpr() const { return m_lexpr; }
@@ -1568,6 +1578,7 @@ namespace JSC {
     };
     
     class ForInNode : public EnumerationNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ForInNode);
     public:
         ForInNode(const JSTokenLocation&, ExpressionNode*, ExpressionNode*, StatementNode*, VariableEnvironment&);
 
@@ -1579,6 +1590,7 @@ namespace JSC {
     };
     
     class ForOfNode : public EnumerationNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ForOfNode);
     public:
         ForOfNode(bool, const JSTokenLocation&, ExpressionNode*, ExpressionNode*, StatementNode*, VariableEnvironment&);
         bool isForOfNode() const override { return true; }
@@ -1668,9 +1680,8 @@ namespace JSC {
     };
 
     class TryNode : public StatementNode, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(TryNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         TryNode(const JSTokenLocation&, StatementNode* tryBlock, DestructuringPatternNode* catchPattern, StatementNode* catchBlock, VariableEnvironment& catchEnvironment, StatementNode* finallyBlock);
 
     private:
@@ -1684,12 +1695,15 @@ namespace JSC {
 
     class ScopeNode : public StatementNode, public ParserArenaRoot, public VariableEnvironmentNode {
     public:
+        // ScopeNode is never directly instantiate. The life-cycle of its derived classes are
+        // managed using std::unique_ptr. Hence, though ScopeNode extends VariableEnvironmentNode,
+        // which in turn extends ParserArenaDeletable, we don't want to use ParserArenaDeletable's
+        // new for allocation.
+        using ParserArenaRoot::operator new;
 
         ScopeNode(ParserArena&, const JSTokenLocation& start, const JSTokenLocation& end, bool inStrictContext);
         ScopeNode(ParserArena&, const JSTokenLocation& start, const JSTokenLocation& end, const SourceCode&, SourceElements*, VariableEnvironment&, FunctionStack&&, VariableEnvironment&, UniquedStringImplPtrSet&&, CodeFeatures, InnerArrowFunctionCodeFeatures, int numConstants);
 
-        using ParserArenaRoot::operator new;
-
         const SourceCode& source() const { return m_source; }
         const String& sourceURL() const { return m_source.provider()->url(); }
         intptr_t sourceID() const { return m_source.providerID(); }
@@ -1831,6 +1845,7 @@ namespace JSC {
     };
 
     class ImportSpecifierListNode : public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ImportSpecifierListNode);
     public:
         typedef Vector<ImportSpecifierNode*, 3> Specifiers;
 
@@ -1921,6 +1936,7 @@ namespace JSC {
     };
 
     class ExportSpecifierListNode : public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ExportSpecifierListNode);
     public:
         typedef Vector<ExportSpecifierNode*, 3> Specifiers;
 
@@ -1949,9 +1965,8 @@ namespace JSC {
     };
 
     class FunctionMetadataNode final : public Node, public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(FunctionMetadataNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         FunctionMetadataNode(
             ParserArena&, const JSTokenLocation& start, const JSTokenLocation& end, 
             unsigned startColumn, unsigned endColumn, int functionKeywordStart, 
@@ -2120,9 +2135,8 @@ namespace JSC {
     };
 
     class ClassExprNode final : public ExpressionNode, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ClassExprNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         ClassExprNode(const JSTokenLocation&, const Identifier&, const SourceCode& classSource,
             VariableEnvironment& classEnvironment, ExpressionNode* constructorExpresssion,
             ExpressionNode* parentClass, PropertyListNode* instanceMethods, PropertyListNode* staticMethods);
@@ -2164,9 +2178,8 @@ namespace JSC {
     };
 
     class ArrayPatternNode : public DestructuringPatternNode, public ThrowableExpressionData, public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ArrayPatternNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         ArrayPatternNode();
         enum class BindingType {
             Elision,
@@ -2194,9 +2207,8 @@ namespace JSC {
     };
     
     class ObjectPatternNode : public DestructuringPatternNode, public ThrowableExpressionData, public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(ObjectPatternNode);
     public:
-        using ParserArenaDeletable::operator new;
-        
         ObjectPatternNode();
         enum class BindingType {
             Element,
@@ -2312,6 +2324,7 @@ namespace JSC {
     };
 
     class FunctionParameters : public ParserArenaDeletable {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(FunctionParameters);
     public:
         FunctionParameters();
         ALWAYS_INLINE unsigned size() const { return m_patterns.size(); }
@@ -2410,9 +2423,8 @@ namespace JSC {
     };
 
     class SwitchNode : public StatementNode, public VariableEnvironmentNode {
+        JSC_MAKE_PARSER_ARENA_DELETABLE_ALLOCATED(SwitchNode);
     public:
-        using ParserArenaDeletable::operator new;
-
         SwitchNode(const JSTokenLocation&, ExpressionNode*, CaseBlockNode*, VariableEnvironment&, FunctionStack&&);
 
     private:
index 16e065d..85b4855 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -28,6 +28,7 @@
 #include "CommonIdentifiers.h"
 #include "Identifier.h"
 #include <array>
+#include <type_traits>
 #include <wtf/SegmentedVector.h>
 
 namespace JSC {
@@ -153,11 +154,17 @@ namespace JSC {
             return block;
         }
 
+        template<typename T, typename = std::enable_if_t<std::is_base_of<ParserArenaDeletable, T>::value>>
         void* allocateDeletable(size_t size)
         {
-            ParserArenaDeletable* deletable = static_cast<ParserArenaDeletable*>(allocateFreeable(size));
+            // T may extend ParserArenaDeletable via multiple inheritance, but not as T's first
+            // base class. m_deletableObjects is expecting pointers to objects of the shape of
+            // ParserArenaDeletable. We ensure this by allocating T, and casting it to
+            // ParserArenaDeletable to get the correct pointer to append to m_deletableObjects.
+            T* instance = static_cast<T*>(allocateFreeable(size));
+            ParserArenaDeletable* deletable = static_cast<ParserArenaDeletable*>(instance);
             m_deletableObjects.append(deletable);
-            return deletable;
+            return instance;
         }
 
         IdentifierArena& identifierArena()