[cfe-dev] New checker for "error return"

[Balázs Kéri 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201009/9edb3cf8/attachment.html>