From 1af83ad821639cfd9c3cfdc71c89efbd329a3491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 8 Dec 2023 21:54:23 +0100 Subject: [PATCH] Fix #12091 (False negative: Uninitialized variable read in subfunction (regression)) (#5739) --- lib/valueflow.cpp | 13 ++++++++++--- test/teststl.cpp | 11 +++++------ test/testuninitvar.cpp | 2 +- test/testvalueflow.cpp | 13 ++++++++++++- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 16d7595a9..10f0a6638 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7325,9 +7325,16 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { return false; } - bool stopOnCondition(const Token* /*condTok*/) const override { - // TODO fix false negatives - return true; // isConditional(); + bool stopOnCondition(const Token* condTok) const override { + if (isConditional()) + return true; + if (!condTok->hasKnownIntValue() && values.count(condTok->varId()) == 0) { + const auto& values_ = condTok->values(); + return std::any_of(values_.cbegin(), values_.cend(), [](const ValueFlow::Value& v) { + return v.isSymbolicValue() && Token::Match(v.tokvalue, "%oror%|&&"); + }); + } + return false; } bool updateScope(const Token* endBlock, bool /*modified*/) const override { diff --git a/test/teststl.cpp b/test/teststl.cpp index 523a0c59b..190978c9c 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2403,12 +2403,11 @@ private: "void g(const std::vector& w) {\n" " f(-1, w);\n" "}\n"); - TODO_ASSERT_EQUALS("test.cpp:5:warning:Array index -1 is out of bounds.\n" - "test.cpp:8:note:Calling function 'f', 1st argument '-1' value is -1\n" - "test.cpp:3:note:Assuming condition is false\n" - "test.cpp:5:note:Negative array index\n", - "", - errout.str()); + ASSERT_EQUALS("test.cpp:5:warning:Array index -1 is out of bounds.\n" + "test.cpp:8:note:Calling function 'f', 1st argument '-1' value is -1\n" + "test.cpp:3:note:Assuming condition is false\n" + "test.cpp:5:note:Negative array index\n", + errout.str()); settings = oldSettings; } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 85d80b991..3b4c548c7 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -6587,7 +6587,7 @@ private: " bool copied_all = true;\n" " g(&copied_all, 5, 6, &bytesCopied);\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:2]: (warning) Uninitialized variable: *buflen\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:2]: (warning) Uninitialized variable: *buflen\n", errout.str()); // # 9953 valueFlowUninit("uint32_t f(uint8_t *mem) {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 13d3b5bdd..b3b9152c5 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -4511,7 +4511,7 @@ private: "void f(Object *obj) {\n" " if (valid(obj, K0)) {}\n" "}\n"; - TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 7U, 0)); + ASSERT_EQUALS(true, testValueOfX(code, 7U, 0)); ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, 0)); code = "int f(int i) {\n" @@ -5624,6 +5624,17 @@ private: "}\n"; values = tokenValues(code, "x <", ValueFlow::Value::ValueType::UNINIT); ASSERT_EQUALS(0, values.size()); + + code = "void g(bool *result, size_t *buflen) {\n" // #12091 + " if (*result && *buflen >= 5) {}\n" // <- *buflen might not be initialized + "}\n" + "void f() {\n" + " size_t bytesCopied;\n" + " bool copied_all = true;\n" + " g(&copied_all, &bytesCopied);\n" + "}"; + values = tokenValues(code, "buflen >=", ValueFlow::Value::ValueType::UNINIT); + ASSERT_EQUALS(1, values.size()); } void valueFlowConditionExpressions() {