Notes from C++ static analysis

bearophile bearophileHUGS at lycos.com
Wed Jun 26 11:08:08 PDT 2013


An interesting blog post found through Reddit:

http://randomascii.wordpress.com/2013/06/24/two-years-and-thousands-of-bugs-of-/

The post is about the heavy usage of static analysis on lot of 
C++ code. They have a Python script that shows new warnings only 
the first time they appear in the code base. This is a simple but 
very useful memory, to solve one of the most important downsides 
of warnings.

The article groups bugs in some different categories. Some of the 
D code below is derived from the article.

- - - - - - - - - - - - - - - - - -

Format strings:

The most common problem they find are errors in the format string 
of printf-like functions (despite the code is C++):

>The top type of bug that /analyze finds is format string errors 
>– mismatches between printf-style format strings and the 
>corresponding arguments. Sometimes there is a missing argument, 
>sometimes there is an extra argument, and sometimes the 
>arguments don’t match, such as printing a float, long or ‘long 
>long’ with %d.<

Such errors in D are less bad, because writef("%d",x) is usable 
for all kind of integral values. On the other hand this D program 
prints just "10" with no errors, ignoring the second x:

import std.stdio;
void main() {
     size_t x = 10;
     writefln("%d", x, x);
}

In a modern statically typed language I'd like such code to give 
a compile-time error.

This is how how Rust gets this right:

println(fmt!("hello, %d", j))

https://github.com/mozilla/rust/blob/master/src/libsyntax/ext/fmt.rs
https://github.com/Aatch/rust-fmt

In D it can be written a safe function that needs no extra static 
analysis:

ctWritefln!"%d"(x, x);

- - - - - - - - - - - - - - - - - -

Variable shadowing:

This is a much less common problem in D because this code gives a 
errors:

void main() {
     bool result = true;
     if (true) {
         bool result = false;
     }
     foreach (i; 0 .. 10) {
         foreach (i; 0 .. 20) {
         }
     }
     for (int i = 0; i < 10; i++) {
         for (int i = 0; i < 20; i++) {
         }
     }
}


test.d(4): Error: is shadowing declaration test.main.result
test.d(7): Error: is shadowing declaration test.main.i
test.d(11): Error: is shadowing declaration test.main.i


There are some situations where this doesn't help, but they are 
not common in idiomatic D code:

void main() {
     int i, j;
     for (i = 0; i < 10; i++) {
         for (i = 0; i < 20; i++) {
         }
     }
}


In D this is one case similar to variable shadowing, that the 
compiler doesn't help you with:

class Foo {
     int x, y, z, w;
     this(in int x_, in int y_, in int z_, in int w_) {
         this.x = x_;
         this.y = y_;
         this.z = z;
         this.w = w_;
     }
}
void main() {
     auto f = new Foo(1, 2, 3, 4);
}


I believe the compile should give some warning there:
http://d.puremagic.com/issues/show_bug.cgi?id=3878

- - - - - - - - - - - - - - - - - -

Logic bugs:


bool someFunction() { return true; }
uint getFlags() { return uint.max; }
void main() {
     uint kFlagValue = 2u ^^ 14;
     if (someFunction() || getFlags() | kFlagValue) {}
}


The D compiler gives no warnings. from the article:

>The code above is an expensive and misleading way to go "if ( 
>true )". Visual Studio gave a clear warning that described the 
>problem well:

     warning C6316: Incorrect operator:  tested expression is 
constant and non-zero.  Use bitwise-and to determine whether bits 
are set.<


See:
http://msdn.microsoft.com/en-us/library/f921xb29.aspx

A simpler example:

enum INPUT_VALUE = 2;
void f(uint flags) {
     if (flags | INPUT_VALUE) {}
}


I have just added it to Bugzilla:
http://d.puremagic.com/issues/show_bug.cgi?id=10480



Another problem:

void main() {
     bool bOnFire = true;
     float angle = 20.0f + bOnFire ? 5.0f : 10.0f;
}


D compiler gives no warnings.

Visual Studio gave:

>warning C6336: Arithmetic operator has precedence over question 
>operator, use parentheses to clarify intent.
warning C6323: Use of arithmetic operator on Boolean type(s).<


See:
http://msdn.microsoft.com/en-us/library/ms182085.aspx

I opened an ER lot of time ago, "Require parenthesization of 
ternary operator when compounded":
http://d.puremagic.com/issues/show_bug.cgi?id=8757

- - - - - - - - - - - - - - - - - -

Signed, unsigned, and tautologies:

Currently this gives no warnings:


>This code would have been fine if both a and b were signed – but 
>one of them wasn’t, making this operation nonsensical.<


import std.algorithm: max;
void main() {
     int a = -10;
     uint b = 5;
     auto result = max(0, a - b);
}



>We had quite a few places where we were checking to see if 
>unsigned variables were less than zero -- now we have fewer.<

This is a well known problem, it's an issue in Bugzilla since lot 
of time, and it seems there is no simple way to face it in D.

Bye,
bearophile


More information about the Digitalmars-d mailing list