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

[Jordan Rose]

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?


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140314/b542645f/attachment.html>