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.

Using check::DeadSymbols.

Added garbage collection and better problem detection.

Small bugfixes, added some tests.

A better (?) implementation.

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

Smaller diff with first part:
https://reviews.llvm.org/D72705

I wanted to implement the rules described here:
https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors
This lists the functions to check so this knowledge has to be built into the checker and there are examples of this in other checkers (stream or memory or string functions for example). The functions have different kinds conditions on the return value in case of error. It is not sufficient to simply check that the return value is assigned to something or used because it may be used without check for error. So I tried to do something that can find if exactly a check for the error condition was made. (Still it is not perfect because the value can be used before the error checking.)
The cast to void is the special way to avoid the warning, in every other case the return value has to be checked for error or there is an exception that should be documented (if the function "can not fail" or the error does not matter). This may be too much warnings for a normal project but acceptable if extra safe code is needed. If we want to have that clang SA can (at least partially) check for that rule "ERR33-C" this (or similar) check is needed.

The checker was implemented in smaller parts that are visible in the commit list. I can split the patch at the commits (and figure out how to use git history manipulations and phabricator in a better way?).

• Improved function list format in documentation.

Checker was tested with tmux, no problems found (there are false positives but not easy to fix).

• More fixes in test files.

• Prevent warning if call is in a return statement.
• Prevent warning if call is casted to void.
• Added documentation.
• Fixed the tests.
• Some other fixes.

• Added variadic functions, improved comments.

Works relatively good now but not perfect. The tests are sometimes too strict so there are some false positives, for example this case:

unsigned long X = strtoul("345", NULL, 10);
if (X > 100) {
 // handle error
}

The result is not checked for ULONG_MAX but still the code is correct in this way. But we can not figure out the intention of the programmer to detect what is an "error handling code" to check for error handling branches. Other solution is to detect any branch condition that involves the value (returned from the function) as a "test for error return value" but this is probably not better.

• Implemented all (now possible) functions.
• Moved ParmVal value to own state map.
• Improved state data and bug reporting.

Adding a new diff over the previous one.
(The commit was amended accidentally.)

Code is to be reformatted later.

The fix looks OK (other alternative is to remove the CHECK with CXXRecordDecl, or make a single line with regular expressions).

Rebased to monorepo and newer master.

• rearrangement in evelFreopen, updated comment in test

• Moved freopen after fopen, removed 'SValBuilder'.

Ping. It should be accepted before I can land it.

• Simplified the code.

I am still not sure in the auto type, I did not see that way of auto usage often in clang code.

• Do not allow null stream to freopen.
• Added comments.

From the description The original stream (if it exists) is closed. I think it is possible that the original stream does "not exist". But a test program crashed with NULL argument (but not with a closed file). So null argument is not permitted or at least does not work always.

I removed the previous comment because I realized that StdCLibraryFunctionsChecker does not use evalCall for fread (returns false because "non-pure" evaluation).

From the same book:

There are multiple preconditions required for inlining to happen, including:
— Source code of the callee function body needs to be available;
— No checker should evaluate the function call via eval::Call;

Abandon this change?

I wanted to remove eval::Call because only one checker can do this otherwise it is undefined behavior (according to the not very new "Analyzer Guide"). If it is essentially needed in this checker it will remain.

• Redesign again.

Ping again

• Improved check for C++17.

Ping

• Fixes from review comments, added C++ version test.

Ping

• Re-design of eval functions.
• Added a C++ test with fopen-looking function.

• Various code cleanups. Eval functions use CallEvent, CallExpr is removed from state.

• Rename to DefaultOperatorNewAlignmentCheck.

Using CallDescriptionMap.

I try to improve StreamChecker instead.

Fixed the test, fixed problems in list.rst.

Rebase and update according to comments.
C++17 related changes not implemented yet (possible check for the called allocation function).

C++17 makes things more difficult because the align is probably handled by operator new, probably not, depending on the defined allocation functions. This can be observed only with a non clang-tidy checker (we could compute the used alignment?). Probably the CERT rule is from the time before C++17. It looks like that by default the alignment is handled correctly in C++17 so the whole check is out of scope for that language version.

Code of StreamChecker does not look much better, it uses deprecated eval::Call and does not check for escape. But it handles more functions and error at open (still not freopen). A mixture of both would be a better option.

OK
Probably the ClassTemplateSpec can not be handled in liberal way because the AST data structure for template specializations do not allow multiple instances with same argument values?