From c9a2e5994e0411747a0796e2d9a9f29b366914ed Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 7 Aug 2024 23:37:09 +0200 Subject: [PATCH 1/7] Fix --- lib/valueflow.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c1816af1a0f..d4899370bc3 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7158,7 +7158,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, value.setImpossible(); valueFlowForward(tok->linkAt(2), containerTok, std::move(value), tokenlist, errorLogger, settings); } - } else if (Token::simpleMatch(tok, "+=") && astIsContainer(tok->astOperand1())) { + } else if (tok->str() == "+=" && astIsContainer(tok->astOperand1())) { const Token* containerTok = tok->astOperand1(); const Token* valueTok = tok->astOperand2(); MathLib::bigint size = valueFlowGetStrLength(valueTok); @@ -7172,6 +7172,27 @@ static void valueFlowContainerSize(const TokenList& tokenlist, if (!next) next = tok->next(); valueFlowForward(next, containerTok, std::move(value), tokenlist, errorLogger, settings); + } else if (tok->str() == "+" && Token::simpleMatch(tok->astParent(), "=")) { // TODO: handle multiple + + const Token* op1 = tok->astOperand1(); + const Token* op2 = tok->astOperand2(); + + const bool op1Str = astIsContainerString(op1); + const bool op2Str = astIsContainerString(op2); + if (!op1Str && !op2Str) + continue; + + const MathLib::bigint size1 = valueFlowGetStrLength(op1), size2 = valueFlowGetStrLength(op2); + const MathLib::bigint size = std::max(size1, size2); + if (size == 0) + continue; + ValueFlow::Value value(size - 1); + value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + value.bound = ValueFlow::Value::Bound::Upper; + value.setImpossible(); + Token* next = nextAfterAstRightmostLeaf(tok); + if (!next) + next = tok->next(); + valueFlowForward(next, tok->astParent()->astOperand1(), std::move(value), tokenlist, errorLogger, settings); } } } From 4d30e11ec80efb940912e2cc7c3642e8b4c04674 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 7 Aug 2024 23:50:40 +0200 Subject: [PATCH 2/7] Tests --- test/testvalueflow.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index d0413682724..41f153002f8 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6942,6 +6942,30 @@ class TestValueFlow : public TestFixture { " return x;\n" "}\n"; ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 1)); + + code = "void f(const std::string& a) {\n" // #12994 + " std::string b = a + \"123\";\n" + " if (b.empty()) {}\n" + "}"; + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 2)); + + code = "void f(const std::string& a) {\n" + " std::string b = \"123\" + a;\n" + " if (b.empty()) {}\n" + "}"; + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 2)); + + code = "void f(const std::string& a, const std::string& b) {\n" + " std::string c = a + b + \"123\";\n" + " if (c.empty()) {}\n" + "}"; + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2)); + + code = "void f(const std::string& a, const std::string& b) {\n" + " std::string c = \"123\" + a + b;\n" + " if (c.empty()) {}\n" + "}"; + TODO_ASSERT_EQUALS("", "values.size():0", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2)); } void valueFlowContainerElement() From 1fb009d7e211c32f16823269ed54dce152aa87c6 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 7 Aug 2024 23:55:29 +0200 Subject: [PATCH 3/7] Format --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3d56b7db408..b175a68ca52 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7178,7 +7178,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, } else if (tok->str() == "+" && Token::simpleMatch(tok->astParent(), "=")) { // TODO: handle multiple + const Token* op1 = tok->astOperand1(); const Token* op2 = tok->astOperand2(); - + const bool op1Str = astIsContainerString(op1); const bool op2Str = astIsContainerString(op2); if (!op1Str && !op2Str) From 83d1ee1592d2f8a11bb6c3ff991420346ddbce51 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Thu, 8 Aug 2024 00:29:53 +0200 Subject: [PATCH 4/7] Fix TODO --- lib/valueflow.cpp | 25 +++++++++++++------------ test/testvalueflow.cpp | 14 +++++++++++++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b175a68ca52..6dee2e1b9b0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7175,19 +7175,20 @@ static void valueFlowContainerSize(const TokenList& tokenlist, if (!next) next = tok->next(); valueFlowForward(next, containerTok, std::move(value), tokenlist, errorLogger, settings); - } else if (tok->str() == "+" && Token::simpleMatch(tok->astParent(), "=")) { // TODO: handle multiple + - const Token* op1 = tok->astOperand1(); - const Token* op2 = tok->astOperand2(); - - const bool op1Str = astIsContainerString(op1); - const bool op2Str = astIsContainerString(op2); - if (!op1Str && !op2Str) + } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+")) { + const Token* tok2 = tok->astOperand2(); + bool haveString = false; + MathLib::bigint size = 0; + while (Token::simpleMatch(tok2, "+")) { + size += valueFlowGetStrLength(tok2->astOperand2()); + haveString = haveString || astIsContainerString(tok2->astOperand2()); + tok2 = tok2->astOperand1(); + } + size += valueFlowGetStrLength(tok2); + haveString = haveString || astIsContainerString(tok2); + if (size == 0 || !haveString) continue; - const MathLib::bigint size1 = valueFlowGetStrLength(op1), size2 = valueFlowGetStrLength(op2); - const MathLib::bigint size = std::max(size1, size2); - if (size == 0) - continue; ValueFlow::Value value(size - 1); value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; value.bound = ValueFlow::Value::Bound::Upper; @@ -7195,7 +7196,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, Token* next = nextAfterAstRightmostLeaf(tok); if (!next) next = tok->next(); - valueFlowForward(next, tok->astParent()->astOperand1(), std::move(value), tokenlist, errorLogger, settings); + valueFlowForward(next, tok->astOperand1(), std::move(value), tokenlist, errorLogger, settings); } } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 82bf309b31d..85a38b7bd33 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6968,11 +6968,23 @@ class TestValueFlow : public TestFixture { "}"; ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2)); + code = "void f(const std::string& a) {\n" + " std::string b = a + \"123\" + \"456\";\n" + " if (b.empty()) {}\n" + "}"; + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 5)); + + code = "void f(const std::string& a) {\n" + " std::string b = \"123\" + a + \"456\";\n" + " if (b.empty()) {}\n" + "}"; + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "b ."), 5)); + code = "void f(const std::string& a, const std::string& b) {\n" " std::string c = \"123\" + a + b;\n" " if (c.empty()) {}\n" "}"; - TODO_ASSERT_EQUALS("", "values.size():0", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2)); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "c ."), 2)); } void valueFlowContainerElement() From 567bef2309b583ba81ae9fb384f107af4e62c94c Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Thu, 8 Aug 2024 00:40:24 +0200 Subject: [PATCH 5/7] Handle unary + --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6dee2e1b9b0..48c5495f046 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7179,7 +7179,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, const Token* tok2 = tok->astOperand2(); bool haveString = false; MathLib::bigint size = 0; - while (Token::simpleMatch(tok2, "+")) { + while (Token::simpleMatch(tok2, "+") && tok2->astOperand2()) { size += valueFlowGetStrLength(tok2->astOperand2()); haveString = haveString || astIsContainerString(tok2->astOperand2()); tok2 = tok2->astOperand1(); From ba9c157386f1dbdcf9001557e4c65f7f967587a2 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Thu, 8 Aug 2024 00:55:42 +0200 Subject: [PATCH 6/7] Simplify --- lib/valueflow.cpp | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 48c5495f046..d4d753def6d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7094,6 +7094,20 @@ static void valueFlowContainerSize(const TokenList& tokenlist, } } + auto forwardImpossibleContainerSize = [&](MathLib::bigint size, Token* opTok, const Token* exprTok) -> void { + if (size == 0) + return; + + ValueFlow::Value value(size - 1); + value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + value.bound = ValueFlow::Value::Bound::Upper; + value.setImpossible(); + Token* next = nextAfterAstRightmostLeaf(opTok); + if (!next) + next = opTok->next(); + valueFlowForward(next, exprTok, std::move(value), tokenlist, errorLogger, settings); + }; + // after assignment for (const Scope *functionScope : symboldatabase.functionScopes) { for (auto* tok = const_cast(functionScope->bodyStart); tok != functionScope->bodyEnd; tok = tok->next()) { @@ -7161,42 +7175,22 @@ static void valueFlowContainerSize(const TokenList& tokenlist, value.setImpossible(); valueFlowForward(tok->linkAt(2), containerTok, std::move(value), tokenlist, errorLogger, settings); } + } else if (tok->str() == "+=" && astIsContainer(tok->astOperand1())) { const Token* containerTok = tok->astOperand1(); const Token* valueTok = tok->astOperand2(); - MathLib::bigint size = valueFlowGetStrLength(valueTok); - if (size == 0) - continue; - ValueFlow::Value value(size - 1); - value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; - value.bound = ValueFlow::Value::Bound::Upper; - value.setImpossible(); - Token* next = nextAfterAstRightmostLeaf(tok); - if (!next) - next = tok->next(); - valueFlowForward(next, containerTok, std::move(value), tokenlist, errorLogger, settings); - } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+")) { + const MathLib::bigint size = valueFlowGetStrLength(valueTok); + forwardImpossibleContainerSize(size, tok, containerTok); + + } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+") && astIsContainerString(tok)) { const Token* tok2 = tok->astOperand2(); - bool haveString = false; MathLib::bigint size = 0; while (Token::simpleMatch(tok2, "+") && tok2->astOperand2()) { size += valueFlowGetStrLength(tok2->astOperand2()); - haveString = haveString || astIsContainerString(tok2->astOperand2()); tok2 = tok2->astOperand1(); } size += valueFlowGetStrLength(tok2); - haveString = haveString || astIsContainerString(tok2); - if (size == 0 || !haveString) - continue; - - ValueFlow::Value value(size - 1); - value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; - value.bound = ValueFlow::Value::Bound::Upper; - value.setImpossible(); - Token* next = nextAfterAstRightmostLeaf(tok); - if (!next) - next = tok->next(); - valueFlowForward(next, tok->astOperand1(), std::move(value), tokenlist, errorLogger, settings); + forwardImpossibleContainerSize(size, tok, tok->astOperand1()); } } } From d39693c903ebb1200432026ee682747694923785 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 8 Aug 2024 01:11:50 +0200 Subject: [PATCH 7/7] Rename --- lib/valueflow.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d4d753def6d..fac53ecb3f2 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7094,7 +7094,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, } } - auto forwardImpossibleContainerSize = [&](MathLib::bigint size, Token* opTok, const Token* exprTok) -> void { + auto forwardMinimumContainerSize = [&](MathLib::bigint size, Token* opTok, const Token* exprTok) -> void { if (size == 0) return; @@ -7180,7 +7180,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, const Token* containerTok = tok->astOperand1(); const Token* valueTok = tok->astOperand2(); const MathLib::bigint size = valueFlowGetStrLength(valueTok); - forwardImpossibleContainerSize(size, tok, containerTok); + forwardMinimumContainerSize(size, tok, containerTok); } else if (tok->str() == "=" && Token::simpleMatch(tok->astOperand2(), "+") && astIsContainerString(tok)) { const Token* tok2 = tok->astOperand2(); @@ -7190,7 +7190,7 @@ static void valueFlowContainerSize(const TokenList& tokenlist, tok2 = tok2->astOperand1(); } size += valueFlowGetStrLength(tok2); - forwardImpossibleContainerSize(size, tok, tok->astOperand1()); + forwardMinimumContainerSize(size, tok, tok->astOperand1()); } } }