[cfe-dev] Why these problems below were not found by Clang Static Analyzer?

[Nico Weber]

On Fri, Mar 14, 2014 at 10:00 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> 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...
>
> On Mar 11, 2014, at 19:53 , apache <ehcapa at qq.com> wrote:
>
> ----------------------------------------------------------------------
> 1.case without break
> e.g.
> int test(const int n) {
>     int ret = 0;
>     switch(n) {
>     case 1:
>         ret = 1;
>         break;
>     case 2:
>         ret = 2; // this case branch has no 'break' statement.(coverity
> gived a warning here, but Clang didn't)
>     default:
>         break;
>     }
>     return ret;
> }
>
>
> This is supposed to be -Wimplicit-fallthrough, but I don't see it working
> either! Richard, do you know what's going on here?
>

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.)


>
>
> ----------------------------------------------------------------------
> 2.Dead code like below
> #define MAX_NUM 10
> void test(const int n) {
>     if(n >= MAX_NUM && n < MAX_NUM) {
>         printf("yes\n"); // this code will never be executed!(coverity
> gived a warning here, but Clang didn't)
>     }
> }
>
>
> 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'.
>
>
> ----------------------------------------------------------------------
> 3.NULL-Pointer reference like below
> typedef struct {
>     int age;
>     int sex;
> } Person;
>
> Person *one_person(char flag)
> {
>     static Person p = {0, 0};
>     if(flag == 1) {
>         return &p;
>     }
>     return 0;
> }
>
> void test()
> {
>     Person *p = on_person(0);
>     p->age = 24; // NULL-Pointer reference(coverity gived a warning here,
> but Clang didn't)
>     p->sex = 0;
> }
>
>
> 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.
>
> I've had a bug on me for a while to make this work if we can *prove* 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.
>
> Thanks for the reports. If you're interested in seeing them followed up on
> specifically, please file bugs at http://llvm.org/bugs/
>
> Jordan
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140314/4b7e6dbf/attachment.html>