Skip to content

Commit c83bee7

Browse files
chrchr-githubchrchr-github
authored
Fix #14969 Inconsistent severities for possible values (negativeIndex, zerodiv) (#8836)
1 parent f99ea4b commit c83bee7

5 files changed

Lines changed: 66 additions & 3 deletions

File tree

‎lib/checkbufferoverrun.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ void CheckBufferOverrunImpl::negativeIndexError(const Token* tok,
461461
}
462462

463463
reportError(getErrorPath(tok, negativeValue, "Negative array index"),
464-
negativeValue->errorSeverity() ? Severity::error : Severity::warning,
464+
(negativeValue->errorSeverity() && !negativeValue->conditional) ? Severity::error : Severity::warning,
465465
"negativeIndex",
466466
arrayIndexMessage(tok, dimensions, indexes, condition),
467467
CWE_BUFFER_UNDERRUN,

‎lib/checkother.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2570,8 +2570,8 @@ void CheckOtherImpl::zerodivError(const Token *tok, const ValueFlow::Value *valu
25702570
errmsg << "Division by zero.";
25712571

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

‎lib/vf_settokenvalue.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,8 @@ namespace ValueFlow
407407
}
408408

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

‎test/testbufferoverrun.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3676,6 +3676,34 @@ class TestBufferOverrun : public TestFixture {
36763676
"[test.cpp:8:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
36773677
"[test.cpp:7:11]: note: Assuming that condition 'i!=2' is not redundant\n"
36783678
"[test.cpp:8:12]: note: Buffer overrun\n", errout_str());
3679+
3680+
check("int a[3];\n"
3681+
"int f1(int i, bool b) {\n"
3682+
" int j = b ? i : -1;\n"
3683+
" return a[j];\n"
3684+
"}"
3685+
"int f2(int i, bool b) {\n"
3686+
" int j = b ? -1 : i;\n"
3687+
" return a[j];\n"
3688+
"}"
3689+
"int f3(int i, bool b) {\n"
3690+
" int j = -1;\n"
3691+
" if (b)\n"
3692+
" j = i;\n"
3693+
" return a[j];\n"
3694+
"}", s);
3695+
ASSERT_EQUALS("[test.cpp:4:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
3696+
"[test.cpp:3:13]: note: Assuming condition 'b' is false\n"
3697+
"[test.cpp:3:15]: note: Assignment 'j=b?i:-1', assigned value is -1\n"
3698+
"[test.cpp:4:13]: note: Negative array index\n"
3699+
"[test.cpp:7:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
3700+
"[test.cpp:6:13]: note: Assuming condition 'b' is true\n"
3701+
"[test.cpp:6:15]: note: Assignment 'j=b?-1:i', assigned value is -1\n"
3702+
"[test.cpp:7:13]: note: Negative array index\n"
3703+
"[test.cpp:12:13]: warning: Array 'a[3]' accessed at index -1, which is out of bounds. [negativeIndex]\n"
3704+
"[test.cpp:9:14]: note: Assignment 'j=-1', assigned value is -1\n"
3705+
"[test.cpp:10:9]: note: Assuming condition is false\n"
3706+
"[test.cpp:12:13]: note: Negative array index\n", errout_str());
36793707
}
36803708

36813709
void buffer_overrun_bailoutIfSwitch() {

‎test/testother.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class TestOther : public TestFixture {
7676
TEST_CASE(zeroDiv22);
7777

7878
TEST_CASE(zeroDivCond); // division by zero / useless condition
79+
TEST_CASE(zeroDivErrorPath);
7980

8081
TEST_CASE(nanInArithmeticExpression);
8182

@@ -897,6 +898,38 @@ class TestOther : public TestFixture {
897898
ASSERT_EQUALS("", errout_str());
898899
}
899900

901+
void zeroDivErrorPath() {
902+
setMultiline();
903+
Settings s = settings0;
904+
s.templateLocation = "{file}:{line}:note:{info}\n";
905+
906+
check("int f1(int i, bool b) {\n"
907+
" int j = b ? i : 0;\n"
908+
" return 1 / j;\n"
909+
"}\n"
910+
"int f2(int i, bool b) {\n"
911+
" int j = b ? 0 : i;\n"
912+
" return 1 / j;\n"
913+
"}\n"
914+
"int f3(int i, bool b) {\n"
915+
" int j = 1;\n"
916+
" if (b)\n"
917+
" j = 0;\n"
918+
" return 1 / j;\n"
919+
"}\n", dinit(CheckOptions, $.settings = &s));
920+
ASSERT_EQUALS("[test.cpp:3:14]: warning: Division by zero. [zerodivcond]\n"
921+
"[test.cpp:2:13]: note: Assuming condition 'b' is false\n"
922+
"[test.cpp:2:15]: note: Assignment 'j=b?i:0', assigned value is 0\n"
923+
"[test.cpp:3:14]: note: Division by zero\n"
924+
"[test.cpp:7:14]: warning: Division by zero. [zerodivcond]\n"
925+
"[test.cpp:6:13]: note: Assuming condition 'b' is true\n"
926+
"[test.cpp:6:15]: note: Assignment 'j=b?0:i', assigned value is 0\n"
927+
"[test.cpp:7:14]: note: Division by zero\n"
928+
"[test.cpp:13:14]: warning: Division by zero. [zerodivcond]\n"
929+
"[test.cpp:12:13]: note: Assignment 'j=0', assigned value is 0\n"
930+
"[test.cpp:13:14]: note: Division by zero\n", errout_str());
931+
}
932+
900933
void nanInArithmeticExpression() {
901934
check("void f()\n"
902935
"{\n"

0 commit comments

Comments
 (0)