Hi,<div><br></div><div>The attached patch adds a new classification of uninitialized variables, between 'always used uninitialized' and 'may be used uninitialized', which (unlike 'may be used uninitialized') has no false positives (after making one conservative assumption), and produces more warnings than 'always used uninitialized'.</div>
<div><br></div><div>The core idea is to assume that for each condition which was explicitly written in the source code, and isn't constant, there exists a live code path which will cause it to be true and a live code path which will cause it to be false. If every such code path definitely leads to an uninitialized variable use, we classify the use as being 'sometimes uninitialized' rather than 'may be uninitialized'.</div>
<div><br></div><div>I've run this across a large body of code, and found mostly bugs, but also two flavors of "false positive":</div><div><br></div><div>* Dead conditions</div><div><br></div><div>int n = ...;</div>
<div>int x;</div><div>if (n < 0) { x = 0; }</div><div>else if (n >= 0) { x = 1; }</div><div>return x;</div><div><br></div><div>- and -</div><div><br></div><div><div>int x;</div><div>for (int n = 0; n < size; ++n) { // Redundant "n < size" test.</div>
<div> // This is somehow guaranteed to happen for some n.</div><div> if (f(n) == k) {</div><div> x = n;</div><div> break;</div><div> }</div><div>}</div><div>return x;</div></div><div><br></div><div>* Value dependence within function calls</div>
<div><br></div><div>bool hasFoo = false;</div><div>int foo;</div><div>if (complex_condition) {</div><div> hasFoo = true;</div><div> foo = something;</div><div>}</div><div>f(hasFoo, foo);</div><div><br></div><div>Here, f doesn't use its second argument if its first argument is false. However, an lvalue-to-rvalue conversion on an uninitialized object has undefined behavior (in C++ at least), so it doesn't seem unreasonable to complain about this.</div>
<div><br></div><div>Is this appropriate for including in trunk clang? It seems to provide a better tradeoff between false positives and false negatives than either of the existing warning options for us. If so, should these warnings be part of -Wuninitialized, or should they be kept separate?</div>
<div><br></div><div>- Richard</div><div><br></div><div><br></div><div>Examples (for more, see the testcases in the patch):</div><div><br></div><div>1) Every time an 'if' condition is not taken, uninitialized use inevitably results:</div>
<div><br></div><div>int test_if_false(bool b) {</div><div><div> int x; // expected-note {{variable}}</div><div> if (b) x = 1; // expected-note {{whenever 'if' condition is false}}</div><div> return x; // expected-warning {{sometimes uninit}}</div>
<div>}</div></div><div><br></div><div><div>test/Analysis/uninit-sometimes.cpp:8:10: warning: variable 'x' is sometimes uninitialized when used here [-Wsometimes-uninitialized]</div><div> return x; // expected-warning {{sometimes uninit}}</div>
<div> ^</div><div>test/Analysis/uninit-sometimes.cpp:7:7: note: uninitialized use occurs whenever 'if' condition is false</div><div> if (b) x = 1; // expected-note {{whenever 'if' condition is false}}</div>
<div> ^</div><div>test/Analysis/uninit-sometimes.cpp:6:8: note: initialize the variable 'x' to silence this warning</div><div> int x; // expected-note {{variable}}</div><div> ^</div><div> = 0</div>
</div><div><br></div><div><br></div><div>2) Every time a 'for' condition is false, uninitialized use inevitably results:</div><div><br></div><div><div>int test_for_false(int k) {</div><div> int x; // expected-note {{variable}}</div>
<div> for (int n = 0; n < k; ++n) { // expected-note {{whenever 'for' loop terminates normally}}</div><div> if (maybe()) {</div><div> x = n;</div><div> break;</div><div> }</div><div> }</div><div>
return x; // expected-warning {{sometimes uninit}}</div><div>}</div></div><div><br></div><div><div>test/Analysis/uninit-sometimes.cpp:70:10: warning: variable 'x' is sometimes uninitialized when used here [-Wsometimes-uninitialized]</div>
<div> return x; // expected-warning {{sometimes uninit}}</div><div> ^</div><div>test/Analysis/uninit-sometimes.cpp:64:19: note: uninitialized use occurs whenever 'for' loop terminates normally</div><div> for (int n = 0; n < k; ++n) { // expected-note {{whenever 'for' loop terminates normally}}</div>
<div> ^~~~~</div><div>test/Analysis/uninit-sometimes.cpp:63:8: note: initialize the variable 'x' to silence this warning</div><div> int x; // expected-note {{variable}}</div><div> ^</div><div>
= 0</div></div><div><br></div><div><br></div><div>3) No warning for uninitialized use resulting from a switch not matching any case: we don't assume that can happen.</div><div><br></div><div>int test_switch_suppress_1(int k) {</div>
<div><div> int x;</div><div> switch (k) {</div><div> case 0:</div><div> x = 0;</div><div> break;</div><div> case 1:</div><div> x = 1;</div><div> break;</div><div> }</div><div> return x; // no-warning</div>
<div>}</div></div><div><br></div><div><br></div><div>4) No warning if there's a loop between a conditional initialization and a loop: we can't be sure the loop terminates.</div><div><br></div><div><div>int test_no_false_positive_1(int k) {</div>
<div> int x;</div><div> if (k)</div><div> x = 5;</div><div> while (!k)</div><div> maybe();</div><div> return x;</div><div>}</div></div>