Fix #12847 Stack overflow in setParentExprId() with huge array

danmar · Jun 15, 2024 · c9f2bbc · c9f2bbc
1 parent b2fa1ef
commit c9f2bbc
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 67 deletions.
diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp
@@ -1564,83 +1564,91 @@ namespace {
         }
     };
     using ExprIdMap = std::map<ExprIdKey, nonneg int>;
-    void setParentExprId(Token* tok, ExprIdMap& exprIdMap, nonneg int &id) {
-        if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
-            return;
-        const Token* op1 = tok->astParent()->astOperand1();
-        if (op1 && op1->exprId() == 0 && !Token::Match(op1, "[{[]"))
-            return;
-        const Token* op2 = tok->astParent()->astOperand2();
-        if (op2 && op2->exprId() == 0 &&
-            !((tok->astParent()->astParent() && tok->astParent()->isAssignmentOp() && tok->astParent()->astParent()->isAssignmentOp()) ||
-              isLambdaCaptureList(op2) ||
-              (op2->str() == "(" && isLambdaCaptureList(op2->astOperand1())) ||
-              Token::simpleMatch(op2, "{ }")))
-            return;
+    void setParentExprId(Token* tokStart, ExprIdMap& exprIdMap, nonneg int &id) {
+        std::stack<Token*> tokens;
+        tokens.push(tokStart);
+        while (!tokens.empty()) {
+            Token* tok = tokens.top();
+            tokens.pop();
+            if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
+                continue;
+            const Token* op1 = tok->astParent()->astOperand1();
+            if (op1 && op1->exprId() == 0 && !Token::Match(op1, "[{[]"))
+                continue;
+            const Token* op2 = tok->astParent()->astOperand2();
+            if (op2 && op2->exprId() == 0 &&
+                !((tok->astParent()->astParent() && tok->astParent()->isAssignmentOp() && tok->astParent()->astParent()->isAssignmentOp()) ||
+                    isLambdaCaptureList(op2) ||
+                    (op2->str() == "(" && isLambdaCaptureList(op2->astOperand1())) ||
+                    Token::simpleMatch(op2, "{ }")))
+                continue;
 
-        if (tok->astParent()->isExpandedMacro() || Token::Match(tok->astParent(), "++|--")) {
-            tok->astParent()->exprId(id);
-            ++id;
-            setParentExprId(tok->astParent(), exprIdMap, id);
-            return;
-        }
+            if (tok->astParent()->isExpandedMacro() || Token::Match(tok->astParent(), "++|--")) {
+                tok->astParent()->exprId(id);
+                ++id;
+                tokens.push(tok->astParent());
+                continue;
+            }
 
-        ExprIdKey key;
-        key.parentOp = tok->astParent()->str();
-        key.operand1 = op1 ? op1->exprId() : 0;
-        key.operand2 = op2 ? op2->exprId() : 0;
+            ExprIdKey key;
+            key.parentOp = tok->astParent()->str();
+            key.operand1 = op1 ? op1->exprId() : 0;
+            key.operand2 = op2 ? op2->exprId() : 0;
 
-        if (tok->astParent()->isCast() && tok->astParent()->str() == "(") {
-            const Token* typeStartToken;
-            const Token* typeEndToken;
-            if (tok->astParent()->astOperand2()) {
-                typeStartToken = tok->astParent()->astOperand1();
-                typeEndToken = tok;
-            } else {
-                typeStartToken = tok->astParent()->next();
-                typeEndToken = tok->astParent()->link();
-            }
-            std::string type;
-            for (const Token* t = typeStartToken; t != typeEndToken; t = t->next()) {
-                type += " " + t->str();
+            if (tok->astParent()->isCast() && tok->astParent()->str() == "(") {
+                const Token* typeStartToken;
+                const Token* typeEndToken;
+                if (tok->astParent()->astOperand2()) {
+                    typeStartToken = tok->astParent()->astOperand1();
+                    typeEndToken = tok;
+                }
+                else {
+                    typeStartToken = tok->astParent()->next();
+                    typeEndToken = tok->astParent()->link();
+                }
+                std::string type;
+                for (const Token* t = typeStartToken; t != typeEndToken; t = t->next()) {
+                    type += " " + t->str();
+                }
+                key.parentOp += type;
             }
-            key.parentOp += type;
-        }
 
-        for (const auto& ref: followAllReferences(op1)) {
-            if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
-                key.operand1 = ref.token->exprId();
-                break;
+            for (const auto& ref : followAllReferences(op1)) {
+                if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
+                    key.operand1 = ref.token->exprId();
+                    break;
+                }
             }
-        }
-        for (const auto& ref: followAllReferences(op2)) {
-            if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
-                key.operand2 = ref.token->exprId();
-                break;
+            for (const auto& ref : followAllReferences(op2)) {
+                if (ref.token->exprId() != 0) { // cppcheck-suppress useStlAlgorithm
+                    key.operand2 = ref.token->exprId();
+                    break;
+                }
             }
-        }
 
-        if (key.operand1 > key.operand2 && key.operand2 &&
-            Token::Match(tok->astParent(), "%or%|%oror%|+|*|&|&&|^|==|!=")) {
-            // In C++ the order of operands of + might matter
-            if (!tok->isCpp() ||
-                key.parentOp != "+" ||
-                !tok->astParent()->valueType() ||
-                tok->astParent()->valueType()->isIntegral() ||
-                tok->astParent()->valueType()->isFloat() ||
-                tok->astParent()->valueType()->pointer > 0)
-                std::swap(key.operand1, key.operand2);
-        }
+            if (key.operand1 > key.operand2 && key.operand2 &&
+                Token::Match(tok->astParent(), "%or%|%oror%|+|*|&|&&|^|==|!=")) {
+                // In C++ the order of operands of + might matter
+                if (!tok->isCpp() ||
+                    key.parentOp != "+" ||
+                    !tok->astParent()->valueType() ||
+                    tok->astParent()->valueType()->isIntegral() ||
+                    tok->astParent()->valueType()->isFloat() ||
+                    tok->astParent()->valueType()->pointer > 0)
+                    std::swap(key.operand1, key.operand2);
+            }
 
-        const auto it = exprIdMap.find(key);
-        if (it == exprIdMap.end()) {
-            exprIdMap[key] = id;
-            tok->astParent()->exprId(id);
-            ++id;
-        } else {
-            tok->astParent()->exprId(it->second);
+            const auto it = exprIdMap.find(key);
+            if (it == exprIdMap.end()) {
+                exprIdMap[key] = id;
+                tok->astParent()->exprId(id);
+                ++id;
+            }
+            else {
+                tok->astParent()->exprId(it->second);
+            }
+            tokens.push(tok->astParent());
         }
-        setParentExprId(tok->astParent(), exprIdMap, id);
     }
 }
 

diff --git a/test/cli/performance_test.py b/test/cli/performance_test.py
@@ -219,3 +219,23 @@ def test_slow_many_scopes(tmpdir):
                     return EXIT_SUCCESS;
                 }""")
     cppcheck([filename]) # should not take more than ~1 second
+
+@pytest.mark.timeout(20)
+def test_crash_array_in_namespace(tmpdir):
+    # 12847
+    filename = os.path.join(tmpdir, 'hang.c')
+    with open(filename, 'wt') as f:
+        f.write(r"""
+                #define ROW A, A, A, A, A, A, A, A,
+                #define ROW8 ROW ROW ROW ROW ROW ROW ROW ROW
+                #define ROW64 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8 ROW8
+                #define ROW256 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64 ROW64
+                #define ROW2048 ROW256 ROW256 ROW256 ROW256 ROW256 ROW256 ROW256 ROW256
+                namespace N {
+                    static const char A = 'a';
+                    const char a[] = {
+                        ROW2048 ROW2048 ROW2048 ROW2048
+                    };
+                }""")
+    cppcheck([filename]) # should not take more than ~5 seconds
+