New checker for "error return"

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

New checker for "error return"

Manas via cfe-dev
Hi!

I had the idea of a checker that inspects if the return value of specific system functions (or any other) is correctly checked for error. It is applicable to functions that return some value on success and a special "error return" value on failure, for example 'malloc' or 'rename'. The checker should verify that there is a part in the source code that tests if the returned value is the "error return" value, as the first "usage" of the returned value. There is a similar CERT rule ERR33-C (https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors).

My question is, does it make sense to have this kind of check, how can it be improved, is there other better way of implementing a similar mechanism. It seems that the algorithm is not easy to understand so I try to explain it here.

The patch D72705 contains the checker. It works the following way:

1. Detect a call to a (system) function that should be checked.
2. Returned value from the function is remembered.
3. The first place is found in the code where the returned value is "used" (read, loaded). The value can be assigned to other variables or passed into functions (or returned?) before this point.

void *M = malloc(10);
void *A;
A = M; // not an "use"
void *B = A; // not an "use"
f(M); // not an "use"

4. At the first "use" (that is found in step 3) it is checked if that use is valid. An "use" is an (clang AST node) expression where the value appears. Specific expressions are detected as "error check" by the checker, others as "invalid use". At the "error check" case the checker forgets the return value and does not produce warning. At the "invalid use" case a warning is produced. If neither are found, the next subexpression is evaluated (in the same expression tree or at later statements). This is a syntactical check of the expressions.

if (M == nullptr) // "error check" found
if (M == nullptr || <other things>) // "error check" found
if (nullptr == (M = malloc(10))) // "error check found"
void *M1 = (char*)M + 1 // "invalid use"
free(M) // "invalid use" ('free' is a system-call function)

If no "use" is found and the value is garbage-collected, a "unchecked value" can be reported by the checker.

I know that there are other ways of detecting similar bugs, but this can find cases that others don't. And no state split is done by this checker. The check can work for functions that return nullptr on error, or return other numerical constant value on error (that is used only to indicate if there was error). In the most simple case, if any read of the return value is found as "error check", it is a form of unused value check.

Thanks,
Balázs


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev