<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 14, 2014 at 10:00 AM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>The analyzer tries not to give results that are already covered by regular Clang warnings. This is sometimes problematic, since the Clang warnings may not be on by default. Nevertheless...</div>
<br><div><div class=""><div>On Mar 11, 2014, at 19:53 , apache <<a href="mailto:ehcapa@qq.com" target="_blank">ehcapa@qq.com</a>> wrote:</div><br><blockquote type="cite"><div>----------------------------------------------------------------------</div>
<div>1.case without break</div><div>e.g.</div><div>int test(const int n) {</div><div> int ret = 0;</div><div> switch(n) {</div><div> case 1:</div><div> ret = 1;</div><div> break;</div><div> case 2:</div>
<div> ret = 2; // this case branch has no 'break' statement.(coverity gived a warning here, but Clang didn't)</div><div> default:</div><div> break;</div><div> }</div><div> return ret;</div>
<div>}</div></blockquote><div><br></div></div><div>This is supposed to be -Wimplicit-fallthrough, but I don't see it working either! Richard, do you know what's going on here?</div></div></div></blockquote><div><br>
</div><div>In general, -Wimplicit-fallthrough does warn on this, which I mostly consider a bug: Falling through to a default: branch that contains nothing but a break is a) safe b) relatively common, so I think this shouldn't warn. (But it currently does, at least in blink.)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class=""><div><br></div><br><blockquote type="cite"><div>----------------------------------------------------------------------</div>
<div>2.Dead code like below</div><div>#define MAX_NUM 10</div><div>void test(const int n) {</div><div> if(n >= MAX_NUM && n < MAX_NUM) {</div><div> printf("yes\n"); // this code will never be executed!(coverity gived a warning here, but Clang didn't)</div>
<div> }</div><div>}</div></blockquote><div><br></div></div><div>This is a true deficiency in the analyzer and in -Wunreachable-code: the former isn't yet good at answering questions that are true "for all paths through the code" (dead stores being an exception), and the latter isn't smart enough to handle the combination of constraints on 'n'.</div>
<div><br></div><br><blockquote type="cite"><div class=""><div>----------------------------------------------------------------------</div><div>3.NULL-Pointer reference like below</div><div>typedef struct {</div><div> int age;</div>
<div> int sex;</div><div>} Person;</div><div><br></div><div>Person *one_person(char flag)</div><div>{</div><div> static Person p = {0, 0};</div><div> if(flag == 1) {</div><div> return &p;</div><div> }</div>
</div><div> return 0;</div><div class=""><div>}</div><div><br></div><div>void test()</div><div>{</div><div> Person *p = on_person(0); </div><div> p->age = 24; // NULL-Pointer reference(coverity gived a warning here, but Clang didn't)</div>
<div> p->sex = 0;</div><div>}</div></div></blockquote><br></div><div>This is a heuristic in the analyzer, assuming that in most cases you won't actually be taking the null-return case of a function you call. This is important for things like std::map::operator[], which will create a new element if the key isn't already in the map. If the value type is a pointer type, this is going to give back a null pointer and cause a spurious analyzer report.</div>
<div><br></div><div>I've had a bug on me for a while to make this work if we can <i>prove</i> that the null pointer return isn't the result of incomplete knowledge (i.e. that the analyzer didn't have to guess at the value of any of the parameters), but I haven't gotten around to it yet. It's something we have to do carefully: the analyzer has to strike a balance between false negatives and false positives, and that balance is different from Coverity's for a variety of reasons.</div>
<div><br></div><div>Thanks for the reports. If you're interested in seeing them followed up on specifically, please file bugs at <a href="http://llvm.org/bugs/" target="_blank">http://llvm.org/bugs/</a></div><span class="HOEnZb"><font color="#888888"><div>
<br></div><div>Jordan</div><br></font></span></div><br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>