Page MenuHomePhabricator

[clang][checkers] Added new checker 'alpha.unix.ErrorReturn'.
Needs ReviewPublic

Authored by balazske on Tue, Jan 14, 7:49 AM.

Details

Summary

First simple implementation, infrastructure is ready.
Only one kind of check (NULL error return value) is implemented.
Documentation not added yet.
Idea of the checker is taken from:
https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors

Event Timeline

balazske created this revision.Tue, Jan 14, 7:49 AM

I am experimenting with somewhat other check algorithm for this problem, the review of this code can be suspended for now.

balazske updated this revision to Diff 238781.Fri, Jan 17, 7:59 AM

A better (?) implementation.

Hello,

This checker is an alternative approach to our not yet published statistics-based checkers in a way: 2019 EuroLLVM Developers’ Meeting: A. Balogh “Statistics Based Checkers in the Clang Static Analyzer”

There are two main differences: The first is that in the statistics-based Special Return Value checker we read the function names from a YAML file. There is no other way because the data was previously dynamically collected, but we can also add a static part to it which contains the functions hard-wired in your checker. I think that is a better approach.

The other, even more important difference is that in the Special Return Value checker we do not check explicitly whether the return value was compared to NULL pointer but we fork a new execution path where it is a NULL pointer and expect other checkers to report a bug on this path. I wonder which approach is the better one: Your approach also finds cases which our checkers can not, e.g. there is not enough memory for malloc(). However it also finds more false positives where the return value was left intentionally unchecked because in that particular case no error can happen.

balazske updated this revision to Diff 239874.Thu, Jan 23, 5:57 AM

Small bugfixes, added some tests.

balazske updated this revision to Diff 239891.Thu, Jan 23, 7:06 AM

Added garbage collection and better problem detection.

Without preventing garbage collection it is not possible to improve the checker
(in this way).

balazske updated this revision to Diff 240111.Fri, Jan 24, 12:12 AM

Using check::DeadSymbols.

I am still unsure about how this checker works if the function to check is "modeled" or evaluated by another checker. Then the function may have already a constrained value at the PostCall event (for example if a malloc fails) and the basic idea of this checker that the constraints on the return value indicate the checks that the program makes does not work. So it may be better to check for the initial constraint somehow and if found ignore that function. Or create a new state where the function call's value is not constrained.