Unify the JavaScript parsing recursion checks

We only need to check in one central location and we can allow for more
recursion. 4k recursions seem tolerable. A common default for stack
sizes is 8MB. Each recursion step takes up to 1k stack space in debug
mode. So, exhausting this would burn about half of the available stack
size. We don't report the exact source location in this case as finding
the source location may itself trigger a deep recursion.

Fixes: QTBUG-74087
Change-Id: I43e6e20b322f6035c7136a6f381230ec285c30ae
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

11 files changed, 100 insertions, 75 deletions

diff --git a/src/qml/compiler/qqmlirbuilder_p.h b/src/qml/compiler/qqmlirbuilder_p.h
index 1a3ca4163e..6affca3ca6 100644
--- a/src/qml/compiler/qqmlirbuilder_p.h
+++ b/src/qml/compiler/qqmlirbuilder_p.h
@@ -498,6 +498,12 @@ public:
     bool visit(QQmlJS::AST::UiScriptBinding *ast) override;
     bool visit(QQmlJS::AST::UiSourceElement *ast) override;
 
+    void throwRecursionDepthError() override
+    {
+        recordError(AST::SourceLocation(),
+                    QStringLiteral("Maximum statement or expression depth exceeded"));
+    }
+
     void accept(QQmlJS::AST::Node *node);
 
     // returns index in _objects
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index b4da05eb9a..0ca452b93e 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -376,7 +376,6 @@ void Codegen::addCJump()
 
 void Codegen::statement(Statement *ast)
 {
-    RecursionDepthCheck depthCheck(this, ast->lastSourceLocation());
     RegisterScope scope(this);
 
     bytecodeGenerator->setLocation(ast->firstSourceLocation());
@@ -392,7 +391,6 @@ void Codegen::statement(ExpressionNode *ast)
     if (! ast) {
         return;
     } else {
-        RecursionDepthCheck depthCheck(this, ast->lastSourceLocation());
         RegisterScope scope(this);
 
         pushExpr(Result(nx));
@@ -420,7 +418,6 @@ void Codegen::condition(ExpressionNode *ast, const BytecodeGenerator::Label *ift
     if (!ast)
         return;
 
-    RecursionDepthCheck depthCheck(this, ast->lastSourceLocation());
     pushExpr(Result(iftrue, iffalse, trueBlockFollowsCondition));
     accept(ast);
     Result r = popExpr();
@@ -3825,8 +3822,14 @@ QQmlRefPointer<CompiledData::CompilationUnit> Codegen::createUnitForLoading()
 class Codegen::VolatileMemoryLocationScanner: protected QQmlJS::AST::Visitor
 {
     VolatileMemoryLocations locs;
+    Codegen *parent;
 
 public:
+    VolatileMemoryLocationScanner(Codegen *parent) :
+        QQmlJS::AST::Visitor(parent->recursionDepth()),
+        parent(parent)
+    {}
+
     Codegen::VolatileMemoryLocations scan(AST::Node *s)
     {
         s->accept(this);
@@ -3891,25 +3894,41 @@ public:
         }
     }
 
+    void throwRecursionDepthError() override
+    {
+        parent->throwRecursionDepthError();
+    }
+
 private:
-    void collectIdentifiers(QVector<QStringView> &ids, AST::Node *node) const {
+    void collectIdentifiers(QVector<QStringView> &ids, AST::Node *node) {
         class Collector: public QQmlJS::AST::Visitor {
+        private:
             QVector<QStringView> &ids;
+            VolatileMemoryLocationScanner *parent;
+
         public:
-            Collector(QVector<QStringView> &ids): ids(ids) {}
-            virtual bool visit(IdentifierExpression *ie) {
+            Collector(QVector<QStringView> &ids, VolatileMemoryLocationScanner *parent) :
+                QQmlJS::AST::Visitor(parent->recursionDepth()), ids(ids), parent(parent)
+            {}
+
+            bool visit(IdentifierExpression *ie) final {
                 ids.append(ie->name);
                 return false;
             }
+
+            void throwRecursionDepthError() final
+            {
+                parent->throwRecursionDepthError();
+            }
         };
-        Collector collector(ids);
+        Collector collector(ids, this);
         node->accept(&collector);
     }
 };
 
-Codegen::VolatileMemoryLocations Codegen::scanVolatileMemoryLocations(AST::Node *ast) const
+Codegen::VolatileMemoryLocations Codegen::scanVolatileMemoryLocations(AST::Node *ast)
 {
-    VolatileMemoryLocationScanner scanner;
+    VolatileMemoryLocationScanner scanner(this);
     return scanner.scan(ast);
 }
 
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index c1063bc0d0..a6355bf93a 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -571,7 +571,6 @@ protected:
         if (!ast || hasError)
             return Reference();
 
-        RecursionDepthCheck depthCheck(this, ast->lastSourceLocation());
         pushExpr();
         ast->accept(this);
         return popResult();
@@ -705,6 +704,11 @@ protected:
     bool throwSyntaxErrorOnEvalOrArgumentsInStrictMode(const Reference &r, const AST::SourceLocation &loc);
     virtual void throwSyntaxError(const AST::SourceLocation &loc, const QString &detail);
     virtual void throwReferenceError(const AST::SourceLocation &loc, const QString &detail);
+    void throwRecursionDepthError() override
+    {
+        throwSyntaxError(AST::SourceLocation(),
+                         QStringLiteral("Maximum statement or expression depth exceeded"));
+    }
 
 public:
     QList<DiagnosticMessage> errors() const;
@@ -831,33 +835,8 @@ protected:
         bool _onoff;
     };
 
-    class RecursionDepthCheck {
-    public:
-        RecursionDepthCheck(Codegen *cg, const AST::SourceLocation &loc)
-            : _cg(cg)
-        {
-#ifdef QT_NO_DEBUG
-            const int depthLimit = 4000; // limit to ~1000 deep
-#else
-            const int depthLimit = 1000; // limit to ~250 deep
-#endif // QT_NO_DEBUG
-
-            ++_cg->_recursionDepth;
-            if (_cg->_recursionDepth > depthLimit)
-                _cg->throwSyntaxError(loc, QStringLiteral("Maximum statement or expression depth exceeded"));
-        }
-
-        ~RecursionDepthCheck()
-        { --_cg->_recursionDepth; }
-
-    private:
-        Codegen *_cg;
-    };
-    int _recursionDepth = 0;
-    friend class RecursionDepthCheck;
-
 private:
-    VolatileMemoryLocations scanVolatileMemoryLocations(AST::Node *ast) const;
+    VolatileMemoryLocations scanVolatileMemoryLocations(AST::Node *ast);
     void handleConstruct(const Reference &base, AST::ArgumentList *args);
 };
 
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index e0eaa8867b..04593f202a 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -57,7 +57,8 @@ using namespace QV4::Compiler;
 using namespace QQmlJS::AST;
 
 ScanFunctions::ScanFunctions(Codegen *cg, const QString &sourceCode, ContextType defaultProgramType)
-    : _cg(cg)
+    : QQmlJS::AST::Visitor(cg->recursionDepth())
+    , _cg(cg)
     , _sourceCode(sourceCode)
     , _context(nullptr)
     , _allowFuncDecls(true)
@@ -96,25 +97,6 @@ void ScanFunctions::leaveEnvironment()
     _context = _contextStack.isEmpty() ? nullptr : _contextStack.top();
 }
 
-bool ScanFunctions::preVisit(Node *ast)
-{
-    if (_cg->hasError)
-        return false;
-    ++_recursionDepth;
-
-    if (_recursionDepth > 1000) {
-        _cg->throwSyntaxError(ast->lastSourceLocation(), QStringLiteral("Maximum statement or expression depth exceeded"));
-        return false;
-    }
-
-    return true;
-}
-
-void ScanFunctions::postVisit(Node *)
-{
-    --_recursionDepth;
-}
-
 void ScanFunctions::checkDirectivePrologue(StatementList *ast)
 {
     for (StatementList *it = ast; it; it = it->next) {
@@ -893,3 +875,8 @@ void ScanFunctions::calcEscapingVariables()
         }
     }
 }
+
+void ScanFunctions::throwRecursionDepthError()
+{
+    _cg->throwRecursionDepthError();
+}
diff --git a/src/qml/compiler/qv4compilerscanfunctions_p.h b/src/qml/compiler/qv4compilerscanfunctions_p.h
index 28ad846bcd..0f7bf1818a 100644
--- a/src/qml/compiler/qv4compilerscanfunctions_p.h
+++ b/src/qml/compiler/qv4compilerscanfunctions_p.h
@@ -96,9 +96,6 @@ protected:
     using Visitor::visit;
     using Visitor::endVisit;
 
-    bool preVisit(AST::Node *ast) override;
-    void postVisit(AST::Node *) override;
-
     void checkDirectivePrologue(AST::StatementList *ast);
 
     void checkName(const QStringRef &name, const AST::SourceLocation &loc);
@@ -160,6 +157,8 @@ protected:
     bool visit(AST::WithStatement *ast) override;
     void endVisit(AST::WithStatement *ast) override;
 
+    void throwRecursionDepthError() override;
+
 protected:
     bool enterFunction(AST::Node *ast, const QString &name, AST::FormalParameterList *formals, AST::StatementList *body, bool enterName);
 
@@ -173,8 +172,6 @@ protected:
     bool _allowFuncDecls;
     ContextType defaultProgramType;
 
-    unsigned _recursionDepth = 0;
-
 private:
     static constexpr AST::Node *astNodeForGlobalEnvironment = nullptr;
 };
diff --git a/src/qml/jsruntime/qv4runtimecodegen_p.h b/src/qml/jsruntime/qv4runtimecodegen_p.h
index be66dc57ca..006a6a3cde 100644
--- a/src/qml/jsruntime/qv4runtimecodegen_p.h
+++ b/src/qml/jsruntime/qv4runtimecodegen_p.h
@@ -71,6 +71,7 @@ public:
 
     void throwSyntaxError(const AST::SourceLocation &loc, const QString &detail) override;
     void throwReferenceError(const AST::SourceLocation &loc, const QString &detail) override;
+
 private:
     ExecutionEngine *engine;
 };
diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index b86dba6daa..8ae51a795f 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -614,16 +614,8 @@ bool Parser::parse(int startToken)
     program = 0;
 
     do {
-        if (++tos == stack_size) {
+        if (++tos == stack_size)
             reallocateStack();
-            if (stack_size > 10000) {
-                // We're now in some serious right-recursive stuff, which will probably result in
-                // an AST that's so deep that recursively visiting it will run out of stack space.
-                const QString msg = QCoreApplication::translate("QQmlParser", "Maximum statement or expression depth exceeded");
-                diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, token_buffer[0].loc, msg));
-                return false;
-            }
-        }
 
         state_stack[tos] = action;
 
diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h
index 0978ab523a..e84c62af2f 100644
--- a/src/qml/parser/qqmljsast_p.h
+++ b/src/qml/parser/qqmljsast_p.h
@@ -273,9 +273,14 @@ public:
 
     inline void accept(Visitor *visitor)
     {
-        if (visitor->preVisit(this))
-            accept0(visitor);
-        visitor->postVisit(this);
+        Visitor::RecursionDepthCheck recursionCheck(visitor);
+        if (recursionCheck()) {
+            if (visitor->preVisit(this))
+                accept0(visitor);
+            visitor->postVisit(this);
+        } else {
+            visitor->throwRecursionDepthError();
+        }
     }
 
     inline static void accept(Node *node, Visitor *visitor)
diff --git a/src/qml/parser/qqmljsastvisitor.cpp b/src/qml/parser/qqmljsastvisitor.cpp
index eec151298e..666623eecc 100644
--- a/src/qml/parser/qqmljsastvisitor.cpp
+++ b/src/qml/parser/qqmljsastvisitor.cpp
@@ -43,7 +43,7 @@ QT_QML_BEGIN_NAMESPACE
 
 namespace QQmlJS { namespace AST {
 
-Visitor::Visitor()
+Visitor::Visitor(quint16 parentRecursionDepth) : m_recursionDepth(parentRecursionDepth)
 {
 }
 
diff --git a/src/qml/parser/qqmljsastvisitor_p.h b/src/qml/parser/qqmljsastvisitor_p.h
index c925096de6..9c69f88e0c 100644
--- a/src/qml/parser/qqmljsastvisitor_p.h
+++ b/src/qml/parser/qqmljsastvisitor_p.h
@@ -61,7 +61,33 @@ namespace QQmlJS { namespace AST {
 class QML_PARSER_EXPORT Visitor
 {
 public:
-    Visitor();
+    class RecursionDepthCheck
+    {
+        Q_DISABLE_COPY(RecursionDepthCheck)
+    public:
+        RecursionDepthCheck(RecursionDepthCheck &&) = delete;
+        RecursionDepthCheck &operator=(RecursionDepthCheck &&) = delete;
+
+        RecursionDepthCheck(Visitor *visitor) : m_visitor(visitor)
+        {
+            ++(m_visitor->m_recursionDepth);
+        }
+
+        ~RecursionDepthCheck()
+        {
+            --(m_visitor->m_recursionDepth);
+        }
+
+        bool operator()() const {
+            return m_visitor->m_recursionDepth < s_recursionLimit;
+        }
+
+    private:
+        static const quint16 s_recursionLimit = 4096;
+        Visitor *m_visitor;
+    };
+
+    Visitor(quint16 parentRecursionDepth = 0);
     virtual ~Visitor();
 
     virtual bool preVisit(Node *) { return true; }
@@ -374,6 +400,14 @@ public:
 
     virtual bool visit(DebuggerStatement *) { return true; }
     virtual void endVisit(DebuggerStatement *) {}
+
+    virtual void throwRecursionDepthError() = 0;
+
+    quint16 recursionDepth() const { return m_recursionDepth; }
+
+protected:
+    quint16 m_recursionDepth = 0;
+    friend class RecursionDepthCheck;
 };
 
 } } // namespace AST
diff --git a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
index c2c73935c0..71dd900073 100644
--- a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
+++ b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
@@ -105,6 +105,11 @@ public:
     {
         nodeStack.removeLast();
     }
+
+    void throwRecursionDepthError() final
+    {
+        QFAIL("Maximum statement or expression depth exceeded");
+    }
 };
 
 }