balazske updated this revision to Diff 241478. balazske added a comment. Adding a new implementation. Now the conditions itself are checked, not the constraints. The appearance of the value to check is detected and tested if it is inside an error check-looking construct. The location of the error check is available and use before check is detected too. More tricky cases of error check code are not found but these should be rare or "questionable" cases.
Probably some possibilities are missed from this code. There is still some debug code left. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/ https://reviews.llvm.org/D72705 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp clang/test/Analysis/Inputs/system-header-simulator.h clang/test/Analysis/error-return.c
Index: clang/test/Analysis/error-return.c =================================================================== --- /dev/null +++ clang/test/Analysis/error-return.c @@ -0,0 +1,437 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.ErrorReturn -verify %s + +#include "Inputs/system-header-simulator.h" + +/* +Functions from CERT ERR33-C that should be checked for error: +https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors + +void *aligned_alloc( size_t alignment, size_t size ); +errno_t asctime_s(char *buf, rsize_t bufsz, const struct tm *time_ptr); +int at_quick_exit( void (*func)(void) ); +int atexit( void (*func)(void) ); +void* bsearch( const void *key, const void *ptr, size_t count, size_t size, + int (*comp)(const void*, const void*) ); +void* bsearch_s( const void *key, const void *ptr, rsize_t count, rsize_t size, + int (*comp)(const void *, const void *, void *), + void *context ); +wint_t btowc( int c ); +size_t c16rtomb( char * restrict s, char16_t c16, mbstate_t * restrict ps ); +size_t c32rtomb( char * restrict s, char32_t c32, mbstate_t * restrict ps ); +void* calloc( size_t num, size_t size ); +clock_t clock(void); +int cnd_broadcast( cnd_t *cond ); +int cnd_init( cnd_t* cond ); +int cnd_signal( cnd_t *cond ); +int cnd_timedwait( cnd_t* restrict cond, mtx_t* restrict mutex, + const struct timespec* restrict time_point ); +int cnd_wait( cnd_t* cond, mtx_t* mutex ); +errno_t ctime_s(char *buffer, rsize_t bufsz, const time_t *time); +int fclose( FILE *stream ); +int fflush( FILE *stream ); +int fgetc( FILE *stream ); +int fgetpos( FILE *restrict stream, fpos_t *restrict pos ); +char *fgets( char *restrict str, int count, FILE *restrict stream ); +wint_t fgetwc( FILE *stream ); +FILE *fopen( const char *restrict filename, const char *restrict mode ); +errno_t fopen_s(FILE *restrict *restrict streamptr, + const char *restrict filename, + const char *restrict mode); +int fprintf( FILE *restrict stream, const char *restrict format, ... ); +int fprintf_s(FILE *restrict stream, const char *restrict format, ...); +int fputc( int ch, FILE *stream ); +int fputs( const char *restrict str, FILE *restrict stream ); +wint_t fputwc( wchar_t ch, FILE *stream ); +int fputws( const wchar_t * restrict str, FILE * restrict stream ); +size_t fread( void *restrict buffer, size_t size, size_t count, + FILE *restrict stream ); +FILE *freopen( const char *restrict filename, const char *restrict mode, + FILE *restrict stream ); +errno_t freopen_s(FILE *restrict *restrict newstreamptr, + const char *restrict filename, const char *restrict mode, + FILE *restrict stream); +int fscanf( FILE *restrict stream, const char *restrict format, ... ); +int fscanf_s(FILE *restrict stream, const char *restrict format, ...); +int fseek( FILE *stream, long offset, int origin ); +int fsetpos( FILE *stream, const fpos_t *pos ); +long ftell( FILE *stream ); +int fwprintf( FILE *restrict stream, + const wchar_t *restrict format, ... ); +int fwprintf_s( FILE *restrict stream, + const wchar_t *restrict format, ...); +size_t fwrite( const void *restrict buffer, size_t size, size_t count, + FILE *restrict stream ); // more exact error return: < count +int fwscanf( FILE *restrict stream, + const wchar_t *restrict format, ... ); +int fwscanf_s( FILE *restrict stream, + const wchar_t *restrict format, ...); +int getc( FILE *stream ); +int getchar(void); +char *getenv( const char *name ); +errno_t getenv_s( size_t *restrict len, char *restrict value, + rsize_t valuesz, const char *restrict name ); +char *gets_s( char *str, rsize_t n ); +wint_t getwc( FILE *stream ); +wint_t getwchar(void); +struct tm *gmtime( const time_t *time ); +struct tm *gmtime_s(const time_t *restrict time, struct tm *restrict result); +struct tm *localtime( const time_t *time ); +struct tm *localtime_s(const time_t *restrict time, struct tm *restrict result); +void* malloc( size_t size ); +int mblen( const char* s, size_t n ); +size_t mbrlen( const char *restrict s, size_t n, mbstate_t *restrict ps ); +size_t mbrtoc16( char16_t * restrict pc16, const char * restrict s, + size_t n, mbstate_t * restrict ps ); +size_t mbrtoc32( char32_t restrict * pc32, const char * restrict s, + size_t n, mbstate_t * restrict ps ); +size_t mbrtowc( wchar_t *restrict pwc, const char *restrict s, size_t n, + mbstate_t *restrict ps ); +size_t mbsrtowcs( wchar_t *restrict dst, const char **restrict src, size_t len, + mbstate_t *restrict ps); +errno_t mbsrtowcs_s( size_t *restrict retval, + wchar_t *restrict dst, rsize_t dstsz, + const char **restrict src, rsize_t len, + mbstate_t *restrict ps); +size_t mbstowcs( wchar_t *restrict dst, const char *restrict src, size_t len); +errno_t mbstowcs_s(size_t *restrict retval, wchar_t *restrict dst, + rsize_t dstsz, const char *restrict src, rsize_t len); +int mbtowc( wchar_t *restrict pwc, const char *restrict s, size_t n ); +void* memchr( const void* ptr, int ch, size_t count ); +time_t mktime( struct tm *time ); +int mtx_init( mtx_t* mutex, int type ); +int mtx_lock( mtx_t* mutex ); +int mtx_timedlock( mtx_t *restrict mutex, + const struct timespec *restrict time_point ); +int mtx_trylock( mtx_t *mutex ); +int mtx_unlock( mtx_t *mutex ); +int printf_s(const char *restrict format, ...); +int putc( int ch, FILE *stream ); +wint_t putwc( wchar_t ch, FILE *stream ); +int raise( int sig ); +void *realloc( void *ptr, size_t new_size ); +int remove( const char *fname ); +int rename( const char *old_filename, const char *new_filename ); +char* setlocale( int category, const char* locale); +int setvbuf( FILE *restrict stream, char *restrict buffer, + int mode, size_t size ); +int scanf( const char *restrict format, ... ); +int scanf_s(const char *restrict format, ...); +void (*signal( int sig, void (*handler) (int))) (int); +int snprintf( char *restrict buffer, size_t bufsz, + const char *restrict format, ... ); +int snprintf_s(char *restrict buffer, rsize_t bufsz, + const char *restrict format, ...); +int snwprintf_s( wchar_t * restrict s, rsize_t n, + const wchar_t * restrict format, ...); // missing from CERT list +int sprintf( char *restrict buffer, const char *restrict format, ... ); +int sprintf_s(char *restrict buffer, rsize_t bufsz, + const char *restrict format, ...); +int sscanf( const char *restrict buffer, const char *restrict format, ... ); +int sscanf_s(const char *restrict buffer, const char *restrict format, ...); +char *strchr( const char *str, int ch ); +errno_t strerror_s( char *buf, rsize_t bufsz, errno_t errnum ); +size_t strftime( char *restrict str, size_t count, + const char *restrict format, const struct tm *restrict time ); +char* strpbrk( const char* dest, const char* breakset ); +char *strrchr( const char *str, int ch ); +char *strstr( const char* str, const char* substr ); +double strtod( const char *restrict str, char **restrict str_end ); +float strtof( const char *restrict str, char **restrict str_end ); +intmax_t strtoimax( const char *restrict nptr, + char **restrict endptr, int base ); +char *strtok( char *restrict str, const char *restrict delim ); +char *strtok_s(char *restrict str, rsize_t *restrict strmax, + const char *restrict delim, char **restrict ptr); +long strtol( const char *restrict str, char **restrict str_end, int base ); +long double strtold( const char *restrict str, char **restrict str_end ); +long long strtoll( const char *restrict str, char **restrict str_end, int base ); +uintmax_t strtoumax( const char *restrict nptr, + char **restrict endptr, int base ); +unsigned long strtoul( const char *restrict str, char **restrict str_end, + int base ); +unsigned long long strtoull( const char *restrict str, char **restrict str_end, + int base ); +size_t strxfrm( char *restrict dest, const char *restrict src, + size_t count ); +int swprintf( wchar_t *restrict buffer, size_t bufsz, + const wchar_t *restrict format, ... ); +int swprintf_s( wchar_t *restrict buffer, rsize_t bufsz, + const wchar_t* restrict format, ...); +int swscanf( const wchar_t *restrict buffer, + const wchar_t *restrict format, ... ); +int swscanf_s( const wchar_t *restrict s, + const wchar_t *restrict format, ...); +int thrd_create( thrd_t *thr, thrd_start_t func, void *arg ); +int thrd_detach( thrd_t thr ); +int thrd_join( thrd_t thr, int *res ); +int thrd_sleep( const struct timespec* duration, + struct timespec* remaining ); +time_t time( time_t *arg ); +int timespec_get( struct timespec *ts, int base); +FILE *tmpfile(void); +errno_t tmpfile_s(FILE * restrict * restrict streamptr); +char *tmpnam( char *filename ); +errno_t tmpnam_s(char *filename_s, rsize_t maxsize); +int tss_create( tss_t* tss_key, tss_dtor_t destructor ); +void *tss_get( tss_t tss_key ); +int tss_set( tss_t tss_id, void *val ); +int ungetc( int ch, FILE *stream ); +wint_t ungetwc( wint_t ch, FILE *stream ); +int vfprintf( FILE *restrict stream, const char *restrict format, + va_list vlist ); +int vfprintf_s( FILE *restrict stream, const char *restrict format, + va_list arg); +int vfscanf( FILE *restrict stream, const char *restrict format, + va_list vlist ); +int vfscanf_s( FILE *restrict stream, const char *restrict format, + va_list vlist); +int vfwprintf( FILE *restrict stream, + const wchar_t *restrict format, va_list vlist ); +int vfwprintf_s( FILE * restrict stream, + const wchar_t *restrict format, va_list vlist); +int vfwscanf( FILE *restrict stream, + const wchar_t *restrict format, va_list vlist ); +int vfwscanf_s( FILE *restrict stream, + const wchar_t *restrict format, va_list vlist ); +int vprintf( const char *restrict format, va_list vlist ); // missing from CERT list +int vprintf_s( const char *restrict format, va_list arg); +int vscanf( const char *restrict format, va_list vlist ); +int vscanf_s(const char *restrict format, va_list vlist); +int vsnprintf( char *restrict buffer, size_t bufsz, + const char *restrict format, va_list vlist ); +int vsnprintf_s(char *restrict buffer, rsize_t bufsz, + const char *restrict format, va_list arg); +int vsnwprintf_s( wchar_t *restrict buffer, rsize_t bufsz, + const wchar_t *restrict format, va_list vlist); // missing from CERT list +int vsprintf( char *restrict buffer, const char *restrict format, + va_list vlist ); +int vsprintf_s( char *restrict buffer, rsize_t bufsz, + const char *restrict format, va_list arg); +int vsscanf( const char *restrict buffer, const char *restrict format, + va_list vlist ); +int vsscanf_s( const char *restrict buffer, const char *restrict format, + va_list vlist); +int vswprintf( wchar_t *restrict buffer, size_t bufsz, + const wchar_t *restrict format, va_list vlist ); +int vswprintf_s( wchar_t *restrict buffer, rsize_t bufsz, + const wchar_t * restrict format, va_list vlist); +int vswscanf( const wchar_t *restrict buffer, + const wchar_t *restrict format, va_list vlist ); +int vswscanf_s( const wchar_t *restrict buffer, + const wchar_t *restrict format, va_list vlist ); +int vwprintf_s( const wchar_t *restrict format, va_list vlist); +int vwscanf( const wchar_t *restrict format, va_list vlist ); +int vwscanf_s( const wchar_t *restrict format, va_list vlist ); +size_t wcrtomb( char *restrict s, wchar_t wc, mbstate_t *restrict ps); +errno_t wcrtomb_s(size_t *restrict retval, char *restrict s, rsize_t ssz, + wchar_t wc, mbstate_t *restrict ps); // missing from CERT list +wchar_t* wcschr( const wchar_t* str, wchar_t ch ); +size_t wcsftime( wchar_t* str, size_t count, const wchar_t* format, tm* time ); +wchar_t* wcspbrk( const wchar_t* dest, const wchar_t* str ); +wchar_t* wcsrchr( const wchar_t* str, wchar_t ch ); +size_t wcsrtombs( char *restrict dst, const wchar_t **restrict src, size_t len, + mbstate_t *restrict ps); +errno_t wcsrtombs_s( size_t *restrict retval, char *restrict dst, rsize_t dstsz, + const wchar_t **restrict src, rsize_t len, + mbstate_t *restrict ps); +wchar_t* wcsstr( const wchar_t* dest, const wchar_t* src ); +double wcstod( const wchar_t * restrict str, wchar_t ** restrict str_end ); +float wcstof( const wchar_t * restrict str, wchar_t ** restrict str_end ); +intmax_t wcstoimax( const wchar_t *restrict nptr, + wchar_t **restrict endptr, int base ); +wchar_t *wcstok(wchar_t * restrict str, const wchar_t * restrict delim, + wchar_t **restrict ptr); +wchar_t *wcstok_s( wchar_t *restrict str, rsize_t *restrict strmax, + const wchar_t *restrict delim, wchar_t **restrict ptr); +long wcstol( const wchar_t * str, wchar_t ** restrict str_end, + int base ); +long double wcstold( const wchar_t * restrict str, wchar_t ** restrict str_end ); +long long wcstoll( const wchar_t * restrict str, wchar_t ** restrict str_end, + int base ); +size_t wcstombs( char *restrict dst, const wchar_t *restrict src, size_t len ); +errno_t wcstombs_s( size_t *restrict retval, char *restrict dst, rsize_t dstsz, + const wchar_t *restrict src, rsize_t len ); +uintmax_t wcstoumax( const wchar_t *restrict nptr, + wchar_t **restrict endptr, int base ); +unsigned long wcstoul( const wchar_t * restrict str, + wchar_t ** restrict str_end, int base ); +unsigned long long wcstoull( const wchar_t * restrict str, + wchar_t ** restrict str_end, int base ); +size_t wcsxfrm( wchar_t* restrict dest, const wchar_t* restrict src, size_t count ); +int wctob( wint_t c ); +int wctomb( char *s, wchar_t wc ); +errno_t wctomb_s(int *restrict status, char *restrict s, rsize_t ssz, wchar_t wc); // CERT list contains wrong description? +wctrans_t wctrans( const char* str ); +wctype_t wctype( const char* str ); +wchar_t *wmemchr( const wchar_t *ptr, wchar_t ch, size_t count ); +int wprintf_s( const wchar_t *restrict format, ...); +int wscanf( const wchar_t *restrict format, ... ); +int wscanf_s( const wchar_t *restrict format, ...); + +These are OK if not checked: +int putchar( int ch ); +wint_t putwchar( wchar_t ch ); +int puts( const char *str ); +int printf( const char *restrict format, ... ); +int vprintf( const char *restrict format, va_list vlist ); +int wprintf( const wchar_t *restrict format, ... ); +int vwprintf( const wchar_t *restrict format, va_list vlist ); + +The following functions are OK if not checked. +These have no error return value therefore can not be handled by this checker: +kill_dependency() +memcpy(), wmemcpy() +memmove(), wmemmove() +strcpy(), wcscpy() +strncpy(), wcsncpy() +strcat(), wcscat() +strncat(), wcsncat() +memset(), wmemset() +*/ + +void *ptr(); + +void test_Null_LNot() { + void *P = aligned_alloc(4, 8); + if (!P) { + } +} + +void test_Null_LAnd() { + void *P = aligned_alloc(4, 8); + if (P && ptr()) { + } +} + +void test_Null_LOr() { + void *P = aligned_alloc(4, 8); + if (P || ptr()) { + } +} + +void test_Null_EQ() { + void *P = aligned_alloc(4, 8); + if (P == NULL) { + } +} + +void test_Null_NE() { + void *P = aligned_alloc(4, 8); + if (P != NULL) { + } +} + +void test_Null_Add() { + void *P = aligned_alloc(4, 8); + void *X = P + 1; // expected-warning{{Use of return value that was not checked}} +} + +void test_Null_Cond_Cond() { + void *P = aligned_alloc(4, 8); + void *X = (P ? ptr() : NULL); +} + +int f(); + +void test_Null_Cond_Use() { + void *P = aligned_alloc(4, 8); + // Both conditional paths cause error. + void *X = (f() ? P : ptr()); + // expected-warning@-1{{Use of return value that was not checked}} + // expected-warning@-2{{Return value was not checked}} +} + +void test_Null_Call_EQ() { + if (aligned_alloc(4, 8) == NULL) { + } +} + +int test_Null_Ret_EQ() { + return aligned_alloc(4, 8) == NULL; +} + +void test_Null_Assign_EQ() { + int X = (aligned_alloc(4, 8) == NULL); +} + +void test_Null_VoidCast() { + (void)aligned_alloc(4, 8); +} + +void test_Null_Initialized() { + void *P = aligned_alloc(4, 8); + void *P1 = P; + if (!P1) { + } +} + +void test_Null_Assigned() { + void *P = aligned_alloc(4, 8); + void *P1; + P1 = P; + if (!P1) { + } +} + +void test_Null_If() { + void *P = aligned_alloc(4, 8); + if (P) { + } +} + +void test_Null_While() { + void *P = aligned_alloc(4, 8); + while (P) { + P = aligned_alloc(4, 8); + } +} + +void test_Null_Do() { + void *P; + do { + P = aligned_alloc(4, 8); + } while (P); +} + +void test_Null_For() { + void *P; + for (P = aligned_alloc(4, 8); P;) { + } +} + +void test_Null_UnusedVar() { + void *P = aligned_alloc(4, 8); +} // expected-warning{{Return value was not checked}} + +void test_Null_UnusedCall() { + aligned_alloc(4, 8); // expected-warning{{Use of return value that was not checked}} +} + +void test_Null_WrongEQ() { + if (aligned_alloc(4, 8) == (void *)1) { // expected-warning{{Use of return value that was not checked}} + } +} + +void test_Null_UncheckedSysCall() { + void *P = aligned_alloc(4, 8); + memcpy(P, ptr(), 1); // expected-warning{{Use of return value that was not checked}} +} + +void check1(void *P) { + if (P) { + } +} + +void test_Null_CheckInFn() { + void *P = aligned_alloc(4, 8); + check1(P); +} + +void check2(void *P) { + memcpy(P, ptr(), 1); // expected-warning{{Use of return value that was not checked}} +} + +void test_Null_SysCallInFn() { + void *P = aligned_alloc(4, 8); + check2(P); +} Index: clang/test/Analysis/Inputs/system-header-simulator.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator.h +++ clang/test/Analysis/Inputs/system-header-simulator.h @@ -112,4 +112,8 @@ #define NULL __DARWIN_NULL #endif -#define offsetof(t, d) __builtin_offsetof(t, d) \ No newline at end of file +#define offsetof(t, d) __builtin_offsetof(t, d) + +// Some more functions. + +void *aligned_alloc(size_t alignment, size_t size); Index: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp @@ -0,0 +1,478 @@ +//===-- ErrorReturnChecker.cpp ------------------------------------*- C++ -*--// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines ErrorReturnChecker, a builtin checker that checks for +// error checking of certain C API function return values. +// This check is taken from SEI CERT ERR33-C: +// https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Expr.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" +#include <functional> + +using namespace clang; +using namespace ento; +using namespace std::placeholders; + +/// Store a saved argument value for an API function call to check. +REGISTER_MAP_WITH_PROGRAMSTATE(ParmValMap, SymbolRef, llvm::APSInt) + +namespace { + +/// Test for specific kind of error check. +/// The "check" functions are called if a corresponding construct in the code is +/// found. The functions should return true the code is found to be acceptable +/// as error check. +class CheckForErrorResultChecker { +public: + /// Check a binary operator. + /// For example 'X < 0'. + /// The symbol to check appears on one side of the operator (`ChildIsLHS` + /// indicates if it is on LHS). The `KnownValue` appears on the other side + /// (may be null). + virtual bool testBinOp(const BinaryOperator *BinOp, + const llvm::APSInt *KnownValue, + bool ChildIsLHS) const = 0; + /// Check appearance of the symbol to check in place of a condition. + /// For example 'X' in 'if (X)'. + virtual bool testStmtCond() const = 0; +}; + +/// Error return is a NULL value. +/// Should work with any (pointer) type of null value. +class NullErrorResultChecker : public CheckForErrorResultChecker { +public: + virtual bool testBinOp(const BinaryOperator *BinOp, + const llvm::APSInt *KnownValue, + bool ChildIsLHS) const override { + BinaryOperatorKind Opcode = BinOp->getOpcode(); + switch (Opcode) { + case BO_EQ: + case BO_NE: + return KnownValue && KnownValue->isNullValue(); + case BO_LAnd: + case BO_LOr: + return true; + default: + return false; + } + } + + virtual bool testStmtCond() const override { return true; } +}; + +using FnFilter = std::function<bool(const CallEvent &, CheckerContext &)>; + +/// Data about an API function to check. +struct FnInfo { + /// Minimal argument count, only for variadic functions. + Optional<unsigned int> MinArgCount; + + /// Checker for the function. + CheckForErrorResultChecker *Checker; + + /// Index of argument whose value at call is to be saved in ParmValMap. + Optional<unsigned int> ParmI; + + /// Possibility to exclude a function from the check. + FnFilter Filter; + + // FIXME: Remove if unused. + /// Return type of the function (initialized at runtime). + mutable QualType RetTy; + + FnInfo(CheckForErrorResultChecker *Checker, Optional<unsigned int> ParmI = {}, + FnFilter Filter = {}) + : Checker(Checker), ParmI(ParmI), Filter(Filter) { + ; + } + FnInfo(unsigned int MinArgCount, CheckForErrorResultChecker *Checker, + Optional<unsigned int> ParmI = {}, FnFilter Filter = {}) + : MinArgCount(MinArgCount), Checker(Checker), ParmI(ParmI), + Filter(Filter) { + ; + } +}; + +struct CalledFunctionData { + const FnInfo *Info; + SourceRange CallLocation; + + CalledFunctionData(const CalledFunctionData &CFD) + : Info(CFD.Info), CallLocation(CFD.CallLocation) {} + CalledFunctionData(const FnInfo *Info, const SourceRange &CallLocation) + : Info{Info}, CallLocation{CallLocation} {} + + CalledFunctionData &operator=(const CalledFunctionData &CFD) { + Info = CFD.Info; + CallLocation = CFD.CallLocation; + return *this; + } + + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(Info); + ID.AddInteger(CallLocation.getBegin().getRawEncoding()); + } + + bool operator==(const CalledFunctionData &CFD) const { + return Info == CFD.Info && CallLocation == CFD.CallLocation; + } +}; + +class ErrorReturnChecker + : public Checker<check::PostCall, check::Location, check::DeadSymbols> { + mutable std::unique_ptr<BuiltinBug> BT_UncheckedUse; + mutable std::unique_ptr<BuiltinBug> BT_Unchecked; + + void checkAccess(CheckerContext &C, ProgramStateRef State, const Stmt *LoadS, + SymbolRef CallSym, const CalledFunctionData *CFD) const; + +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void checkLocation(SVal L, bool IsLoad, const Stmt *S, + CheckerContext &C) const; + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + +private: + NullErrorResultChecker CheckNullErrorResult; + + CallDescriptionMap<FnInfo> CheckedFunctions = { + {{"aligned_alloc", 2}, FnInfo{&CheckNullErrorResult}}, + {{"bsearch", 5}, FnInfo{&CheckNullErrorResult}}, + {{"bsearch_s", 6}, FnInfo{&CheckNullErrorResult}}, + {{"calloc", 2}, FnInfo{&CheckNullErrorResult}}, + {{"fgets", 3}, FnInfo{&CheckNullErrorResult}}, + {{"fopen", 2}, FnInfo{&CheckNullErrorResult}}, + {{"freopen", 3}, FnInfo{&CheckNullErrorResult}}, + {{"getenv", 1}, FnInfo{&CheckNullErrorResult}}, + {{"getenv_s", 4}, FnInfo{&CheckNullErrorResult}}, + {{"gets_s", 2}, FnInfo{&CheckNullErrorResult}}, + {{"gmtime", 1}, FnInfo{&CheckNullErrorResult}}, + {{"gmtime_s", 2}, FnInfo{&CheckNullErrorResult}}, + {{"localtime", 1}, FnInfo{&CheckNullErrorResult}}, + {{"localtime_s", 2}, FnInfo{&CheckNullErrorResult}}, + {{"malloc", 1}, FnInfo{&CheckNullErrorResult}}, + {{"memchr", 3}, FnInfo{&CheckNullErrorResult}}, + {{"realloc", 2}, FnInfo{&CheckNullErrorResult}}, + {{"setlocale", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strchr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strpbrk", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strrchr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strstr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strtok", 2}, FnInfo{&CheckNullErrorResult}}, + {{"strtok_s", 4}, FnInfo{&CheckNullErrorResult}}, + {{"tmpfile", 0}, FnInfo{&CheckNullErrorResult}}, + {{"tmpnam", 1}, FnInfo{&CheckNullErrorResult}}, + {{"wcschr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"wcspbrk", 2}, FnInfo{&CheckNullErrorResult}}, + {{"wcsrchr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"wcsstr", 2}, FnInfo{&CheckNullErrorResult}}, + {{"wcstok", 3}, FnInfo{&CheckNullErrorResult}}, + {{"wcstok_s", 4}, FnInfo{&CheckNullErrorResult}}, + {{"wmemchr", 3}, FnInfo{&CheckNullErrorResult}}, + }; +}; + +enum VisitResult { UncheckedUseFound, CheckFound, AssignmentFound }; + +class ErrorCheckTestStmtVisitor + : public ConstStmtVisitor<ErrorCheckTestStmtVisitor, VisitResult> { + CheckerContext &C; + const Stmt *Child; + const CalledFunctionData *CFD; + const ParentMap &PM; + + bool findChildInParent(const Expr *ParentCond) const { + for (const Stmt *P = Child; P; P = PM.getParent(P)) + if (P == ParentCond) + return true; + return false; + }; + + const llvm::APSInt *getKnownConstantVal(const Expr *E) const { + Optional<SVal> ConstantVal = C.getSValBuilder().getConstantVal(E); + if (ConstantVal) + return C.getSValBuilder().getKnownValue(C.getState(), *ConstantVal); + return nullptr; + } + +public: + ErrorCheckTestStmtVisitor(CheckerContext &C, const Stmt *Child, + const CalledFunctionData *CFD) + : C(C), Child(Child), CFD(CFD), + PM(C.getLocationContext()->getParentMap()) {} + VisitResult VisitBinaryOperator(const BinaryOperator *BO) { + Expr *OtherS = nullptr; + for (const Stmt *P = Child; P; P = PM.getParent(P)) { + if (P == BO->getLHS()) { + OtherS = BO->getRHS(); + break; + } else if (P == BO->getRHS()) { + OtherS = BO->getLHS(); + break; + } + } + assert(OtherS && "Statement not found under its parent."); + + BinaryOperatorKind Op = BO->getOpcode(); + if (Op == BO_Assign) { + assert(OtherS == BO->getLHS() && "Loaded value not on assignment RHS."); + return AssignmentFound; + } + + if (CFD->Info->Checker->testBinOp(BO, getKnownConstantVal(OtherS), + OtherS == BO->getLHS())) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitUnaryOperator(const UnaryOperator *UO) { + UnaryOperatorKind Op = UO->getOpcode(); + if (Op == UO_LNot && CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitConditionalOperator(const ConditionalOperator *CO) { + if (!findChildInParent(CO->getCond())) + return UncheckedUseFound; + if (CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitIfStmt(const IfStmt *If) { + if (!findChildInParent(If->getCond())) + return UncheckedUseFound; + if (CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitWhileStmt(const WhileStmt *While) { + if (!findChildInParent(While->getCond())) + return UncheckedUseFound; + if (CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitDoStmt(const DoStmt *Do) { + if (!findChildInParent(Do->getCond())) + return UncheckedUseFound; + if (CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitForStmt(const ForStmt *For) { + if (!findChildInParent(For->getCond())) + return UncheckedUseFound; + if (CFD->Info->Checker->testStmtCond()) + return CheckFound; + return UncheckedUseFound; + } + + VisitResult VisitCallExpr(const CallExpr *CE) { + const FunctionDecl *CalledF = C.getCalleeDecl(CE); + SourceLocation Loc = CalledF->getLocation(); + if (Loc.isValid() && C.getSourceManager().isInSystemHeader(Loc)) + return UncheckedUseFound; + return AssignmentFound; + } + + VisitResult VisitDeclStmt(const DeclStmt *Decl) { return AssignmentFound; } + + VisitResult VisitStmt(const Stmt *S) { return UncheckedUseFound; } +}; + +} // end anonymous namespace + +REGISTER_MAP_WITH_PROGRAMSTATE(CalledFunctionDataMap, SymbolRef, + CalledFunctionData) + +void ErrorReturnChecker::checkAccess(CheckerContext &C, ProgramStateRef State, + const Stmt *LoadS, SymbolRef CallSym, + const CalledFunctionData *CFD) const { + const ParentMap &PM = C.getLocationContext()->getParentMap(); + // LoadS->dumpColor(); + + const Stmt *ParentS = PM.getParentIgnoreParens(LoadS); + // ParentS->dumpColor(); + + ErrorCheckTestStmtVisitor FindErrorCheck{C, LoadS, CFD}; + switch (FindErrorCheck.Visit(ParentS)) { + case UncheckedUseFound: { + if (!BT_UncheckedUse) + BT_UncheckedUse.reset( + new BuiltinBug(this, "Use of unchecked return value", + "Use of return value that was not checked for error")); + + State = State->remove<CalledFunctionDataMap>(CallSym); + State = State->remove<ParmValMap>(CallSym); + + ExplodedNode *N = C.generateNonFatalErrorNode(State); + if (!N) { + C.addTransition(State); + return; + } + + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT_UncheckedUse, BT_UncheckedUse->getDescription(), N); + Report->markInteresting(CallSym); + // Report->addRange(CFDSaved.CallLocation); + C.emitReport(std::move(Report)); + return; + } + case CheckFound: + // A correct error check was found, remove from state. + State = State->remove<CalledFunctionDataMap>(CallSym); + State = State->remove<ParmValMap>(CallSym); + break; + case AssignmentFound: + // Value is assigned, still need to find a check later. + break; + }; + + C.addTransition(State); +} + +void ErrorReturnChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); + ProgramStateRef State = C.getState(); + const ParentMap &PM = C.getLocationContext()->getParentMap(); + + if (!FD || FD->getKind() != Decl::Function) + return; + + if (!Call.isGlobalCFunction() || !Call.isInSystemHeader()) + return; + + const FnInfo *Fn = CheckedFunctions.lookup(Call); + if (!Fn) + return; + + // For variadic functions check for minimal argument count. + if (Fn->MinArgCount && Call.getNumArgs() < *(Fn->MinArgCount)) + return; + + const Stmt *S = PM.getParent(Call.getOriginExpr()); + + // Check for explicit cast to void. + if (auto *Cast = dyn_cast<const CStyleCastExpr>(S)) { + if (Cast->getTypeAsWritten().getTypePtr()->isVoidType()) + return; + } + + // Use a function-specific rule to omit specific calls. + if (Fn->Filter && Fn->Filter(Call, C)) + return; + + // The call should have a symbolic return value to analyze it. + SVal RetSV = Call.getReturnValue(); + if (RetSV.isUnknownOrUndef()) + return; + SymbolRef RetSym = RetSV.getAsSymbol(); + if (!RetSym) + return; + + // Lazy-init the return type when the function is found. + if (Fn->RetTy.isNull()) + Fn->RetTy = FD->getReturnType(); + + CalledFunctionData CFD{Fn, Call.getSourceRange()}; + State = State->set<CalledFunctionDataMap>(RetSym, CFD); + + // Try to compute value of specific argument at the time of call (if needed). + if (Fn->ParmI) { + const llvm::APSInt *ParmVal = + C.getSValBuilder().getKnownValue(State, Call.getArgSVal(*Fn->ParmI)); + if (ParmVal) + State = State->set<ParmValMap>(RetSym, *ParmVal); + } + + checkAccess(C, State, Call.getOriginExpr(), RetSym, &CFD); +} + +void ErrorReturnChecker::checkLocation(SVal L, bool IsLoad, const Stmt *S, + CheckerContext &C) const { + if (!IsLoad) + return; + if (L.isUnknownOrUndef()) + return; + + auto Location = L.castAs<DefinedOrUnknownSVal>().getAs<Loc>(); + if (!Location) + return; + + ProgramStateRef State = C.getState(); + SymbolRef Sym = State->getSVal(*Location).getAsSymbol(); + if (!Sym) + return; + + const CalledFunctionData *CFD = State->get<CalledFunctionDataMap>(Sym); + if (!CFD) + return; + + checkAccess(C, State, S, Sym, CFD); +} + +void ErrorReturnChecker::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + const CalledFunctionDataMapTy &Map = State->get<CalledFunctionDataMap>(); + llvm::SmallMapVector<SymbolRef, CalledFunctionData, 8> Errors; + for (const auto &I : Map) + if (SymReaper.isDead(I.first)) + Errors.insert(I); + + if (Errors.empty()) + return; + + if (!BT_Unchecked) + BT_Unchecked.reset( + new BuiltinBug(this, "Unchecked return value", + "Return value was not checked for error")); + + ExplodedNode *N = C.generateNonFatalErrorNode(State); + + for (const auto &E : Errors) { + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT_Unchecked, BT_Unchecked->getDescription(), N); + // Report->addRange(E.second.CallLocation); + Report->markInteresting(E.first); + C.emitReport(std::move(Report)); + + State = State->remove<CalledFunctionDataMap>(E.first); + State = State->remove<ParmValMap>(E.first); + } + + C.addTransition(State, N); +} + +void ento::registerErrorReturnChecker(CheckerManager &mgr) { + mgr.registerChecker<ErrorReturnChecker>(); +} + +bool ento::shouldRegisterErrorReturnChecker(const LangOptions &LO) { + return true; +} Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -37,6 +37,7 @@ DynamicTypePropagation.cpp DynamicTypeChecker.cpp EnumCastOutOfRangeChecker.cpp + ErrorReturnChecker.cpp ExprInspectionChecker.cpp FixedAddressChecker.cpp FuchsiaHandleChecker.cpp Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -429,6 +429,10 @@ HelpText<"Check improper use of chroot">, Documentation<HasAlphaDocumentation>; +def ErrorReturnChecker : Checker<"ErrorReturn">, + HelpText<"Check for unchecked error return values">, + Documentation<HasAlphaDocumentation>; + def PthreadLockChecker : Checker<"PthreadLock">, HelpText<"Simple lock -> unlock checker">, Documentation<HasAlphaDocumentation>;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
