Repository navigation
Partial fix for #15048 Inconsistent error severity for uninitvar - #8966
chrchr-github wants to merge 5 commits into
Conversation
|
Test results for commit fe389d3 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8966/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The change makes the severity less consistent and turns definite bugs into warnings. Whether Value::conditional is set depends on the code after the if, not on whether the read is guarded by a condition. Two examples:
if (b) return i; return 0;is still reported as an error.if (b) return i; i = 1; return i;is now reported as a warning.
In both functions the read inside the branch is uninitialized every time it runs, so the second case should still be an error. Calls such as if (b) g(i); i = 1; g(i); are also demoted to a warning, although if (b) g(i); alone stays an error.
For if (b) return i; return i;, the read at line 17 is now a warning, while the unconditional return i; on the next line is reported only by legacyUninitvar (as an error). The new test makes the demotion of a definite bug the expected result. I found no crashes.
Testing
I built cppcheck and testrunner in both pr/ and base/.
./testrunner TestUninitVar: all passed.- Full
./testrunner: all passed.
Test file b.cpp:
int f1(bool b) {
int i;
if (b)
return i;
i = 1;
return i;
}
int f3(bool b) {
int i;
if (b)
return i;
return 0;
}
int f4(bool b) {
int i;
if (b)
return i;
return i;
}
int f5(bool b) {
int i;
if (b)
return i + 1;
i = 2;
return i;
}
void g(int);
void f6(bool b) {
int i;
if (b)
g(i);
i = 1;
g(i);
}Command: cppcheck -q --template='{line}:{severity}:{id}:{message}' b.cpp
base:
4:error:uninitvar:Uninitialized variable: i
11:error:uninitvar:Uninitialized variable: i
17:error:uninitvar:Uninitialized variable: i
23:error:uninitvar:Uninitialized variable: i
31:error:uninitvar:Uninitialized variable: i
18:error:legacyUninitvar:Uninitialized variable: i
PR:
4:warning:uninitvar:Uninitialized variable: i
11:error:uninitvar:Uninitialized variable: i
17:warning:uninitvar:Uninitialized variable: i
23:warning:uninitvar:Uninitialized variable: i
31:warning:uninitvar:Uninitialized variable: i
18:error:legacyUninitvar:Uninitialized variable: i
Lines 4 and 11 are the same kind of read, but they now get different severities. Lines 4, 17, 23 and 31 are reads that are always uninitialized whenever they run, yet they were demoted from error to warning.
A second set of 14 one-line variants (if (b) g(i);, loops, switch, ?:, &&, nested if, pointer dereference) gave the same results in base and PR, apart from the case above.
Automated review of fe389d3 by Claude. The pull request was built and tested locally.
|
ValueFlow has special handling for UNINIT values, see https://trac.cppcheck.net/ticket/15168. |
| " return i;\n" | ||
| "}\n"); | ||
| ASSERT_EQUALS("[test.cpp:4:16]: warning: Uninitialized variable: i [uninitvar]\n" | ||
| "[test.cpp:3:9]: note: Assuming condition is true\n" |
There was a problem hiding this comment.
It is not your fault, don't try to fix it in this PR.
But I am skeptic about this note. It's clear in the code this condition must have been true if this line is reached.
No description provided.