Page MenuHomePhabricator

[analyzer] Add new checker for unchecked return value.
Needs ReviewPublic

Authored by balazske on Nov 3 2020, 8:53 AM.

Details

Summary

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.

Diff Detail

Unit TestsFailed

TimeTest
400 mslinux > 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

Event Timeline

balazske created this revision.Nov 3 2020, 8:53 AM
balazske requested review of this revision.Nov 3 2020, 8:53 AM

This is a very simplified form of the check that is implemented in D72705 but still may be useful in practical cases. This check could be a clang-tidy check but it is planned that the checker takes use of the planned new attributes to determine if a function should be checked.

clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp
42

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.

86

Hmm, why StringMap<>? Why not CallDescriptionMap<>?

clang/test/Analysis/unchecked-return-value.cpp
11

Please use some valid format here. E.g. scanf("%*c");

17

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.)

balazske updated this revision to Diff 303444.Nov 6 2020, 7:07 AM

Small fixes in test.

balazske marked 2 inline comments as done.Nov 6 2020, 7:09 AM
balazske added inline comments.
clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp
42

It looks like that the matcher finds these occurrences too. A test was added for it.

86

CallDescriptionMap is only usable with CallEvent that is not used in this checker. Or it can be extended with lookup from FunctionDecl?

balazske marked an inline comment as done.Nov 11 2020, 5:46 AM

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).