Add a checker that checks if a function is used without examining
its return value. The set of functions is taken from CERT rule
ERR33-C "Detect and handle standard library errors". This is a
simplified check for the problem.
|400 ms||linux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp|
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp
Please note that the CallExpr does not necessarily stands alone. It may be wrapped into an ExprWithCleanUps. We should consider these CallExprs as unchecked too.
Hmm, why StringMap<>? Why not CallDescriptionMap<>?
Please use some valid format here. E.g. scanf("%*c");
Please add such simple test case for all the functions we try to check. (One call is enough for every such function, either in std:: or on the top namespace.)
Maybe it is better to have this check for every system function that has non-void return value (except specific functions)? This check is applicable to CERT rule ERR33-C, POS54-C and EXP12-C too (it is restricted but can find a part of the problems).
Looks like a neat checker! I guess the only question is the function matching, and I don't dislike it in its current state. @martong, do you have any thoughts on this?
On another note, have you checked this on some projects yet? I expect that we might need to tone down on some of the functions if this ends up being too noisy.
I suppose we need this to compensate for the fact that we can't reliably match on standard C functions. Unfortunately, using CallDescription wouldn't necessarily solve this: D81745.
Good point. @martong, you have used FunctionDecl based checking, do you have any thoughts here?
I'd like to see more tests with the following cases:
- more expressions with function calls - ternary expression, arithmetic, array literals, initializer lists, etc.
- using/not using it in if statement with initializer
- using/not using it in different parts of GNU statement expressions
- calls in different parts of exception handling (both C++ and Obj-C)
- calls in lambdas/blocks/coroutines
- calls in different parts of C++ range-for loop
- calls in different parts of Obj-C for loop over collections
- calls under labels
- calls in attributed statements (e.g. [[clang::nomerge]] foo())
- calls in goto statements
- calls in Obj-C @synchronized and @autoreleasepool statements
but it is planned that the checker takes use of the planned new attributes to determine if a function should be checked
This is literally __attribute__((warn_unused_result)) for which there already is a compiler warning.