From e17d22eb8718734c4fb6307a0b0e35c12be8c755 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 28 Jan 2021 05:37:56 -0600 Subject: [PATCH] Fix issue 10134: False positive: value is not known. Early return. (#3086) --- lib/programmemory.cpp | 23 ++++++++++++++++++----- lib/programmemory.h | 3 ++- lib/valueflow.cpp | 10 +++++++--- test/testcondition.cpp | 12 ++++++++++++ test/testvalueflow.cpp | 20 ++++++++++++++++++++ 5 files changed, 59 insertions(+), 9 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index ac8d10336..d8680a7d1 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -14,10 +14,10 @@ void ProgramMemory::setValue(nonneg int exprid, const ValueFlow::Value& value) { values[exprid] = value; } -const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid) const +const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossible) const { const ProgramMemory::Map::const_iterator it = values.find(exprid); - const bool found = it != values.end() && !it->second.isImpossible(); + const bool found = it != values.end() && (impossible || !it->second.isImpossible()); if (found) return &it->second; else @@ -58,6 +58,21 @@ bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint* re } return false; } +bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const +{ + const ValueFlow::Value* value = getValue(exprid, true); + if (value && value->isContainerSizeValue()) { + if (value->isImpossible() && value->intvalue == 0) { + *result = false; + return true; + } + if (!value->isImpossible()) { + *result = (value->intvalue == 0); + return true; + } + } + return false; +} void ProgramMemory::setUnknown(nonneg int exprid) { @@ -563,10 +578,8 @@ void execute(const Token *expr, if (!programMemory->getContainerSizeValue(containerTok->exprId(), result)) *error = true; } else if (yield == Library::Container::Yield::EMPTY) { - MathLib::bigint size = 0; - if (!programMemory->getContainerSizeValue(containerTok->exprId(), &size)) + if (!programMemory->getContainerEmptyValue(containerTok->exprId(), result)) *error = true; - *result = (size == 0); } else { *error = true; } diff --git a/lib/programmemory.h b/lib/programmemory.h index d49f15ba5..761012f28 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -15,12 +15,13 @@ struct ProgramMemory { Map values; void setValue(nonneg int exprid, const ValueFlow::Value& value); - const ValueFlow::Value* getValue(nonneg int exprid) const; + const ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false) const; bool getIntValue(nonneg int exprid, MathLib::bigint* result) const; void setIntValue(nonneg int exprid, MathLib::bigint value); bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const; + bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const; void setUnknown(nonneg int exprid); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1a1c8da7d..93f2f3c7c 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4421,13 +4421,17 @@ struct ConditionHandler { std::mem_fn(&ValueFlow::Value::isPossible)); } - if (!values.empty()) { - if ((dead_if || dead_else) && !Token::Match(tok->astParent(), "&&|&")) { + if (dead_if || dead_else) { + if (Token::Match(tok->astParent(), "&&|&")) { + values.remove_if(std::mem_fn(&ValueFlow::Value::isImpossible)); + changeKnownToPossible(values); + } else { valueFlowSetConditionToKnown(tok, values, true); valueFlowSetConditionToKnown(tok, values, false); } - forward(after, scope->bodyEnd, cond.vartok, values, tokenlist, settings); } + if (!values.empty()) + forward(after, scope->bodyEnd, cond.vartok, values, tokenlist, settings); } } }); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index e7cde3c7b..f8ea359d9 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3524,6 +3524,18 @@ private: " return 0;\n" "}\n"); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:11]: (style) Condition '!y' is always true\n", errout.str()); + + // #10134 + check("bool foo(bool b);\n" + "bool thud(const std::vector& Arr, const std::wstring& Str) {\n" + " if (Arr.empty() && Str.empty())\n" + " return false;\n" + " bool OldFormat = Arr.empty() && !Str.empty();\n" + " if (OldFormat)\n" + " return foo(OldFormat);\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void alwaysTrueInfer() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index a932300d4..ce53e5895 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2616,6 +2616,26 @@ private: " return x;\n" "}\n"; ASSERT_EQUALS(false, testValueOfX(code, 7U, 0)); + + code = "void f(int x, int y) {\n" + " if (x && y)\n" + " return;\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 0)); + ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 0)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 1)); + + code = "int f(std::vector a, std::vector b) {\n" + " if (a.empty() && b.empty())\n" + " return 0;\n" + " bool x = a.empty() && !b.empty();\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXKnown(code, 5U, 0)); + ASSERT_EQUALS(false, testValueOfXKnown(code, 5U, 1)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 5U, 0)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 5U, 1)); } void valueFlowAfterConditionExpr() {