static, dynamic analysis - what mistakes were made in the code?

Question

A software company develops software packages for commercial animal farming. A special function in C calculates the daily amount of feed for different kind of animals dependent on their bodyweight. This function already run flawlessly for years in a software package for farms. And now it's integrated into a package for zoological gardens und sometimes causes wrong outputs. What mistakes were done and what are their consequences? Is it possible to detect them static/dynamic?

typedef enum {COW, HORSE, BEAR, MONKEY, PIG} Animal_A;
float feedquantity(Animal_A animalkind, float weight){
    float amount, factor, woodFlourAmount;

    switch(animalkind){
        case COW:
            factor = 0.05;
            break;
        case HORSE:
            factor = 0.1;
            break;
        case MONKEY:
            factor = 0.2;
        case BEAR:
            factor = 0.02;
            break;
    }
    amount = factor * weight;
    return amount;
}

There are some mistakes I found but I'm not sure if some of them are actually mistakes really? The first problem is that in total we have 5 animal kinds but we don't have a case for each animal kind since PIG is missing. So the typedef PIG is not used and seems useless. Another variable that is useless is woodFlourAmount. It is only defined but you don't do anything with it in the function. Another mistake which is worse for the function and will cause problems is that there is a missing break in the case for MONKEY which will, once we have the MONKEY case set factor = 0.2 and execute the next (and last) case BEAR right after which will cause a wrong output. So in case we have MONKEY, it's mistakenly treated as a BEAR.

Is it fine like that or maybe I found something which is not really a 'mistake'? But I'm not sure about how you could find these mistakes statically / dynamically or if it's possible at all :S

Answer 1

Static analysis doesn't necessarily find mistakes, but potential mistakes. It is up to the human programmer to determine whether the static analysis produced a false positive or a real problem. Many compilers perform some amount of static analysis during compilation. In particular, a type checker already is a kind of static analysis.

All of the issues you found are an example of static analysis, because you didn't run the code to detect the potential error.

• A non-exhaustive switch over an enum type is usually an error. A programmer would usually either explicitly list all cases, or add a default case.

• Even if it is intended that the switch is non-exhaustive, the PIG case is provably buggy. Then the switch will be skipped, and the uninitialized variable factor will be read.

• A fallthrough switch case is usually an error. Some static analyzers might require a special comment in case fallthrough is intended.

• Even if fallthrough were intended, the MONKEY case is provably buggy: the assignment to factor would immediately be overwritten by the BEAR case, so that code is superfluous. Static analyzers may be able to trace the data flow in order to find such dead code, for example by using Static Single Assignment (SSA) form internally.

• Similarly, such analysis will show that woodFlourAmount is unnecessary. Variables that are never used may still be useful in some cases. Some style checkers allow them only if they have leading underscores.

Some issues could also be found by dynamic analysis, i.e. observing the software while it executes.:

• Running the code under Valgrind would detect that an uninitialized memory location factor would be read if the animal type is PIG – assuming that some test case exercises that scenario. Whereas static analysis could have shown a potential problem here, dynamic analysis shows that a real problem has happened.

• C's type system is extremely weak. While the type system assumes that an enum-typed variable only holds the correct values, it could accidentally hold any integer. Dynamic analysis may be able to detect cases where an invalid enum value was provided. However, that would not be a bug within this function, but a bug with the usage of this function.