Skip to content

Fix #15016 Inconsistent error paths for if conditions #8870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
chrchr-github wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Fix #15016 Inconsistent error paths for if conditions #8870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
922571c
Update checkbufferoverrun.cpp
chrchr-github Sep 8, 2026
fe2254b
Update checkother.cpp
chrchr-github Sep 8, 2026
9e74aa5
Update vf_settokenvalue.cpp
chrchr-github Sep 8, 2026
0b5c882
Update testbufferoverrun.cpp
chrchr-github Sep 8, 2026
9d9305a
Update testother.cpp
chrchr-github Sep 8, 2026
6b12ec5
Fix #15016 Inconsistent error paths for if conditions
web-flow Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ void CheckBufferOverrunImpl::negativeIndexError(const Token* tok,
}

reportError(getErrorPath(tok, negativeValue, "Negative array index"),
negativeValue->errorSeverity() ? Severity::error : Severity::warning,
(negativeValue->errorSeverity() && !negativeValue->conditional) ? Severity::error : Severity::warning,
"negativeIndex",
arrayIndexMessage(tok, dimensions, indexes, condition),
CWE_BUFFER_UNDERRUN,
Expand Down
4 changes: 2 additions & 2 deletions lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2570,8 +2570,8 @@ void CheckOtherImpl::zerodivError(const Token *tok, const ValueFlow::Value *valu
errmsg << "Division by zero.";

reportError(std::move(errorPath),
value->errorSeverity() ? Severity::error : Severity::warning,
value->condition ? "zerodivcond" : "zerodiv",
(value->errorSeverity() && !value->conditional) ? Severity::error : Severity::warning,
(value->condition || value->conditional) ? "zerodivcond" : "zerodiv",
errmsg.str(), CWE369, value->isInconclusive() ? Certainty::inconclusive : Certainty::normal);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/forwardanalyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ namespace {
return Break();
}
}
analyzer->assume(condTok, !inElse, Analyzer::Assume::Quiet);
analyzer->assume(condTok, !inElse);
assert(!inDoWhile || Token::simpleMatch(tok, "} while ("));
if (hasElse || inDoWhile)
tok = tok->linkAt(2);
Expand Down
14 changes: 12 additions & 2 deletions lib/vf_analyzers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,12 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {

void addErrorPath(const Token* tok, const std::string& s) override {
for (auto&& p:values) {
p.second.errorPath.emplace_back(tok, s);
auto& ep = p.second.errorPath;
if (std::any_of(ep.begin(), ep.end(), [&](const ErrorPathItem& epi) {
return epi.first == tok && epi.second == s;
}))
continue;
ep.emplace_back(tok, s);
}
}

Expand Down Expand Up @@ -1146,7 +1151,12 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer {
}

void addErrorPath(const Token* tok, const std::string& s) override {
value.errorPath.emplace_back(tok, s);
auto& ep = value.errorPath;
if (std::any_of(ep.begin(), ep.end(), [&](const ErrorPathItem& epi) {
return epi.first == tok && epi.second == s;
}))
return;
ep.emplace_back(tok, s);
}

template<class T>
Expand Down
2 changes: 2 additions & 0 deletions lib/vf_settokenvalue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,8 @@ namespace ValueFlow
}

else if (parent->str() == ":" && Token::simpleMatch(parent->astParent(), "?")) {
const std::string condStr(tok == parent->astOperand1() ? "true" : "false");
value.errorPath.emplace_back(parent->astParent()->astOperand1(), "Assuming condition '" + parent->astParent()->astOperand1()->expressionString() + "' is " + condStr);
setTokenValue(parent,std::move(value),settings);
}

Expand Down
16 changes: 8 additions & 8 deletions test/testautovariables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3591,7 +3591,7 @@ class TestAutoVariables : public TestFixture {
" }\n"
" *p = 0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5:13] -> [test.cpp:2:9] -> [test.cpp:7:6]: (error) Static variable 'p' will use pointer to local variable 'a'. [danglingLifetime]\n", errout_str());
ASSERT_EQUALS("[test.cpp:5:13] -> [test.cpp:4:9] -> [test.cpp:2:9] -> [test.cpp:7:6]: (error) Static variable 'p' will use pointer to local variable 'a'. [danglingLifetime]\n", errout_str());

// #10902
check("void f() {\n"
Expand Down Expand Up @@ -4330,7 +4330,7 @@ class TestAutoVariables : public TestFixture {
" std::vector<char*> cargs = f({ \"0\", \"0\" });\n"
" (void)cargs;\n"
"};\n");
ASSERT_EQUALS("[test.cpp:6:12] -> [test.cpp:4:47] -> [test.cpp:3:22] -> [test.cpp:1:58] -> [test.cpp:4:40] -> [test.cpp:9:34] -> [test.cpp:9:34] -> [test.cpp:10:11]: (error) Using object that is a temporary. [danglingTemporaryLifetime]\n", errout_str());
ASSERT_EQUALS("[test.cpp:6:12] -> [test.cpp:4:47] -> [test.cpp:3:22] -> [test.cpp:1:58] -> [test.cpp:4:40] -> [test.cpp:3:24] -> [test.cpp:9:34] -> [test.cpp:9:34] -> [test.cpp:10:11]: (error) Using object that is a temporary. [danglingTemporaryLifetime]\n", errout_str());

check("struct C {\n" // #9194
" const int& m;\n"
Expand Down Expand Up @@ -4692,7 +4692,7 @@ class TestAutoVariables : public TestFixture {
" }\n"
" f();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5:25] -> [test.cpp:4:13] -> [test.cpp:7:5]: (error) Using lambda that captures local variable 'b' that is out of scope. [invalidLifetime]\n", errout_str());
ASSERT_EQUALS("[test.cpp:5:25] -> [test.cpp:3:11] -> [test.cpp:4:13] -> [test.cpp:7:5]: (error) Using lambda that captures local variable 'b' that is out of scope. [invalidLifetime]\n", errout_str());

check("void f(bool b) {\n"
" int* x;\n"
Expand All @@ -4703,7 +4703,7 @@ class TestAutoVariables : public TestFixture {
" x[3];\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:5:9] -> [test.cpp:4:9] -> [test.cpp:7:3]: (error) Using pointer to local variable 'y' that is out of scope. [invalidLifetime]\n",
"[test.cpp:5:9] -> [test.cpp:3:6] -> [test.cpp:4:9] -> [test.cpp:7:3]: (error) Using pointer to local variable 'y' that is out of scope. [invalidLifetime]\n",
errout_str());

check("void foo(int a) {\n"
Expand Down Expand Up @@ -4893,7 +4893,7 @@ class TestAutoVariables : public TestFixture {
" }\n"
" *p = 0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:4:9] -> [test.cpp:7:4]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str());
ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:3:7] -> [test.cpp:4:9] -> [test.cpp:7:4]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n", errout_str());

// FP: don't warn in subfunction
check("void f(struct KEY *key) {\n"
Expand Down Expand Up @@ -4934,7 +4934,7 @@ class TestAutoVariables : public TestFixture {
" dosth();\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:5:24] -> [test.cpp:3:47] -> [test.cpp:4:26] -> [test.cpp:7:9]: (error) Using pointer to local variable 'item' that is out of scope. [invalidLifetime]\n",
"[test.cpp:5:24] -> [test.cpp:3:47] -> [test.cpp:3:47] -> [test.cpp:4:26] -> [test.cpp:7:9]: (error) Using pointer to local variable 'item' that is out of scope. [invalidLifetime]\n",
errout_str());

// #6575
Expand All @@ -4960,7 +4960,7 @@ class TestAutoVariables : public TestFixture {
" return 0;\n"
"}\n");
ASSERT_EQUALS(
"[test.cpp:5:16] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n",
"[test.cpp:5:16] -> [test.cpp:3:8] -> [test.cpp:7:10] -> [test.cpp:4:13] -> [test.cpp:8:17]: (error) Using pointer to local variable 'x' that is out of scope. [invalidLifetime]\n",
errout_str());

// #11753
Expand All @@ -4972,7 +4972,7 @@ class TestAutoVariables : public TestFixture {
" }\n"
" std::cout << s;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5:26] -> [test.cpp:4:14] -> [test.cpp:7:18]: (error) Using pointer to local variable 'buff' that is out of scope. [invalidLifetime]\n", errout_str());
ASSERT_EQUALS("[test.cpp:5:26] -> [test.cpp:3:14] -> [test.cpp:4:14] -> [test.cpp:7:18]: (error) Using pointer to local variable 'buff' that is out of scope. [invalidLifetime]\n", errout_str());

check("char* f(char* dst) {\n"
" const char* src = \"abc\";\n"
Expand Down
28 changes: 28 additions & 0 deletions test/testbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3676,6 +3676,34 @@ class TestBufferOverrun : public TestFixture {
"[test.cpp:8:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
"[test.cpp:7:11]: note: Assuming that condition 'i!=2' is not redundant\n"
"[test.cpp:8:12]: note: Buffer overrun\n", errout_str());

check("int a[3];\n"
"int f1(int i, bool b) {\n"
" int j = b ? i : -1;\n"
" return a[j];\n"
"}"
"int f2(int i, bool b) {\n"
" int j = b ? -1 : i;\n"
" return a[j];\n"
"}"
"int f3(int i, bool b) {\n"
" int j = -1;\n"
" if (b)\n"
" j = i;\n"
" return a[j];\n"
"}", s);
ASSERT_EQUALS("[test.cpp:4:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
"[test.cpp:3:13]: note: Assuming condition 'b' is false\n"
"[test.cpp:3:15]: note: Assignment 'j=b?i:-1', assigned value is -1\n"
"[test.cpp:4:13]: note: Negative array index\n"
"[test.cpp:7:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
"[test.cpp:6:13]: note: Assuming condition 'b' is true\n"
"[test.cpp:6:15]: note: Assignment 'j=b?-1:i', assigned value is -1\n"
"[test.cpp:7:13]: note: Negative array index\n"
"[test.cpp:12:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
"[test.cpp:9:14]: note: Assignment 'j=-1', assigned value is -1\n"
"[test.cpp:10:9]: note: Assuming condition is false\n"
"[test.cpp:12:13]: note: Negative array index\n", errout_str());
}

void buffer_overrun_bailoutIfSwitch() {
Expand Down
34 changes: 34 additions & 0 deletions test/testother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class TestOther : public TestFixture {
TEST_CASE(zeroDiv22);

TEST_CASE(zeroDivCond); // division by zero / useless condition
TEST_CASE(zeroDivErrorPath);

TEST_CASE(nanInArithmeticExpression);

Expand Down Expand Up @@ -897,6 +898,39 @@ class TestOther : public TestFixture {
ASSERT_EQUALS("", errout_str());
}

void zeroDivErrorPath() {
setMultiline();
Settings s = settings0;
s.templateLocation = "{file}:{line}:note:{info}\n";

check("int f1(int i, bool b) {\n"
" int j = b ? i : 0;\n"
" return 1 / j;\n"
"}\n"
"int f2(int i, bool b) {\n"
" int j = b ? 0 : i;\n"
" return 1 / j;\n"
"}\n"
"int f3(int i, bool b) {\n"
" int j = 1;\n"
" if (b)\n"
" j = 0;\n"
" return 1 / j;\n"
"}\n", dinit(CheckOptions, $.settings = &s));
ASSERT_EQUALS("[test.cpp:3:14]: warning: Division by zero. [zerodivcond]\n"
"[test.cpp:2:13]: note: Assuming condition 'b' is false\n"
"[test.cpp:2:15]: note: Assignment 'j=b?i:0', assigned value is 0\n"
"[test.cpp:3:14]: note: Division by zero\n"
"[test.cpp:7:14]: warning: Division by zero. [zerodivcond]\n"
"[test.cpp:6:13]: note: Assuming condition 'b' is true\n"
"[test.cpp:6:15]: note: Assignment 'j=b?0:i', assigned value is 0\n"
"[test.cpp:7:14]: note: Division by zero\n"
"[test.cpp:13:14]: warning: Division by zero. [zerodivcond]\n"
"[test.cpp:12:13]: note: Assignment 'j=0', assigned value is 0\n"
"[test.cpp:11:9]: note: Assuming condition is true\n"
"[test.cpp:13:14]: note: Division by zero\n", errout_str());
}

void nanInArithmeticExpression() {
check("void f()\n"
"{\n"
Expand Down
Loading