Static Analysis Warning?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Static Analysis Warning?

Bill Wendling
Consider this code:

#include <stdio.h>

enum Foo {
  VAL1,
  VAL2
};

int main (int argc, const char * argv[]) {
    // insert code here...
    printf("Hello, World! %d\n", argc == 42 ? VAL1 : VAL1);
    return 0;
}

When I run the static analyzer on it, it doesn't warn. But the coder might not have meant for both values of the ?: operator to be the same. Do you think it's worth a static analyzer warning?

-bw


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis Warning?

Dale Johannesen

On Aug 27, 2010, at 2:24 PMPDT, Bill Wendling wrote:

> Consider this code:
>
> #include <stdio.h>
>
> enum Foo {
>  VAL1,
>  VAL2
> };
>
> int main (int argc, const char * argv[]) {
>    // insert code here...
>    printf("Hello, World! %d\n", argc == 42 ? VAL1 : VAL1);
>    return 0;
> }
>
> When I run the static analyzer on it, it doesn't warn. But the coder  
> might not have meant for both values of the ?: operator to be the  
> same. Do you think it's worth a static analyzer warning?

If a human wrote that it would probably be an error, although this  
does occur in test suites (all compilers here do the optimization).  
The question is whether things like that would arise in machine-
generated code or after macro expansion often enough for false  
positives to be annoying.   I would guess no, and vote for the warning.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis Warning?

Michael Price - Dev
In reply to this post by Bill Wendling
Consider:

#define FLOOR 1
#define CEILING 1

int main (int argc, const char * argc[])
{
     printf("Hello, World! %d\n", useFloor() ? FLOOR : CEILING);

     return 0;
}

where useFloor() returns a bool.

I would NOT want that flagged with a warning.

> int main (int argc, const char * argv[]) {
>    // insert code here...
>    printf("Hello, World! %d\n", argc == 42 ? VAL1 : VAL1);
>    return 0;
> }


On Aug 27, 2010, at 4:24 PM, Bill Wendling <[hidden email]> wrote:

> Consider this code:
>
> #include <stdio.h>
>
> enum Foo {
>  VAL1,
>  VAL2
> };
>
> int main (int argc, const char * argv[]) {
>    // insert code here...
>    printf("Hello, World! %d\n", argc == 42 ? VAL1 : VAL1);
>    return 0;
> }
>
> When I run the static analyzer on it, it doesn't warn. But the coder  
> might not have meant for both values of the ?: operator to be the  
> same. Do you think it's worth a static analyzer warning?
>
> -bw
>
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis Warning?

Jordy Rose

On Fri, 27 Aug 2010 17:39:12 -0500, Michael Price - Dev
<[hidden email]> wrote:

> Consider:
>
> #define FLOOR 1
> #define CEILING 1
>
> int main (int argc, const char * argc[])
> {
>      printf("Hello, World! %d\n", useFloor() ? FLOOR : CEILING);
>
>      return 0;
> }
>
> where useFloor() returns a bool.
>
> I would NOT want that flagged with a warning.

Good point, Michael. I think this would be more useful as a Sema warning
about identical expressions for both results of a ?:, not a value-sensitive
analyzer report. (Taking macros into account, of course; only warn if
neither expression involves a macro or both use identical macros.)
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis Warning?

Bill Wendling
On Aug 27, 2010, at 3:46 PM, Jordy Rose wrote:

> On Fri, 27 Aug 2010 17:39:12 -0500, Michael Price - Dev
> <[hidden email]> wrote:
>> Consider:
>>
>> #define FLOOR 1
>> #define CEILING 1
>>
>> int main (int argc, const char * argc[])
>> {
>>     printf("Hello, World! %d\n", useFloor() ? FLOOR : CEILING);
>>
>>     return 0;
>> }
>>
>> where useFloor() returns a bool.
>>
>> I would NOT want that flagged with a warning.
>
> Good point, Michael. I think this would be more useful as a Sema warning
> about identical expressions for both results of a ?:, not a value-sensitive
> analyzer report. (Taking macros into account, of course; only warn if
> neither expression involves a macro or both use identical macros.)

I agree that it's a good point. I don't thinks that this would make a good compiler warning, but would be limited to the static analyzer. The analyzer has information about whether something comes from a macro, enum, inlined function, etc. I would vote for it emitting a warning for the same enums coming from the same enum type.

-bw


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Static Analysis Warning?

Tom Care
This probably wouldn't be too hard to do in the IdempotentOperationChecker. At the moment we only look at BinaryOperators and don't handle ternary statements explicitly. I think this falls under the 'pointless operation' category.

Tom

On Aug 27, 2010, at 3:52 PM, Bill Wendling wrote:

> On Aug 27, 2010, at 3:46 PM, Jordy Rose wrote:
>
>> On Fri, 27 Aug 2010 17:39:12 -0500, Michael Price - Dev
>> <[hidden email]> wrote:
>>> Consider:
>>>
>>> #define FLOOR 1
>>> #define CEILING 1
>>>
>>> int main (int argc, const char * argc[])
>>> {
>>>    printf("Hello, World! %d\n", useFloor() ? FLOOR : CEILING);
>>>
>>>    return 0;
>>> }
>>>
>>> where useFloor() returns a bool.
>>>
>>> I would NOT want that flagged with a warning.
>>
>> Good point, Michael. I think this would be more useful as a Sema warning
>> about identical expressions for both results of a ?:, not a value-sensitive
>> analyzer report. (Taking macros into account, of course; only warn if
>> neither expression involves a macro or both use identical macros.)
>
> I agree that it's a good point. I don't thinks that this would make a good compiler warning, but would be limited to the static analyzer. The analyzer has information about whether something comes from a macro, enum, inlined function, etc. I would vote for it emitting a warning for the same enums coming from the same enum type.
>
> -bw
>
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev