diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -207,6 +207,10 @@ Static Analyzer --------------- +- Checker ``alpha.unix.Stream`` is improved to be able to detect bugs related + to bad ``errno`` usage. The effects are visible if checker + ``alpha.unix.Errno`` is turned on additionally. + .. _release-notes-ubsan: Undefined Behavior Sanitizer (UBSan) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "ErrnoModeling.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" @@ -17,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" @@ -278,6 +280,9 @@ 0}}, }; + mutable bool EofInitialized = false; + mutable int EofVal = -1; + void evalFopen(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -411,6 +416,25 @@ }); } + /// Get a SVal to be used as new errno value at failure cases of stream + /// functions. + NonLoc getErrnoSVal(CheckerContext &C, const CallEvent &Call) const { + return C.getSValBuilder() + .conjureSymbolVal(this, Call.getOriginExpr(), C.getLocationContext(), + C.getASTContext().IntTy, C.blockCount()) + .castAs(); + } + + void initEof(CheckerContext &C) const { + if (EofInitialized) + return; + + if (const llvm::Optional OptInt = + tryExpandAsInteger("EOF", C.getPreprocessor())) + EofVal = *OptInt; + EofInitialized = true; + } + /// Searches for the ExplodedNode where the file descriptor was acquired for /// StreamSym. static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N, @@ -457,6 +481,8 @@ void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { + initEof(C); + const FnDescription *Desc = lookupFn(Call); if (!Desc || !Desc->PreFn) return; @@ -497,11 +523,18 @@ StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened(Desc)); + StateNotNull = errno_modeling::setErrnoForStdSuccess(StateNotNull, C); + StateNull = StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); - - C.addTransition(StateNotNull, - constructNoteTag(C, RetSym, "Stream opened here")); + StateNull = errno_modeling::setErrnoForStdFailure(StateNull, C, + getErrnoSVal(C, Call)); + + ExplodedNode *N = C.addTransition( + StateNotNull, constructNoteTag(C, RetSym, "Stream opened here")); + C.addTransition(N->getState(), N, + errno_modeling::getNoteTagForStdSuccess( + C, cast(Call.getDecl())->getNameAsString())); C.addTransition(StateNull); } @@ -555,11 +588,17 @@ StateRetNotNull = StateRetNotNull->set(StreamSym, StreamState::getOpened(Desc)); + StateRetNotNull = errno_modeling::setErrnoForStdSuccess(StateRetNotNull, C); + StateRetNull = StateRetNull->set(StreamSym, StreamState::getOpenFailed(Desc)); + StateRetNull = errno_modeling::setErrnoForStdFailure(StateRetNull, C, + getErrnoSVal(C, Call)); - C.addTransition(StateRetNotNull, - constructNoteTag(C, StreamSym, "Stream reopened here")); + ExplodedNode *N = C.addTransition( + StateRetNotNull, constructNoteTag(C, StreamSym, "Stream reopened here")); + C.addTransition(N->getState(), N, + errno_modeling::getNoteTagForStdSuccess(C, "freopen")); C.addTransition(StateRetNull); } @@ -574,6 +613,10 @@ if (!SS) return; + auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return; + assertStreamStateOpened(SS); // Close the File Descriptor. @@ -581,7 +624,21 @@ // and can not be used any more. State = State->set(Sym, StreamState::getClosed(Desc)); - C.addTransition(State); + // Return 0 on success, EOF on failure. + SValBuilder &SVB = C.getSValBuilder(); + ProgramStateRef StateSuccess = State->BindExpr( + CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy)); + ProgramStateRef StateFailure = + State->BindExpr(CE, C.getLocationContext(), + SVB.makeIntVal(EofVal, C.getASTContext().IntTy)); + + StateSuccess = errno_modeling::setErrnoForStdSuccess(StateSuccess, C); + StateFailure = errno_modeling::setErrnoForStdFailure(StateFailure, C, + getErrnoSVal(C, Call)); + + C.addTransition(StateSuccess, + errno_modeling::getNoteTagForStdSuccess(C, "fclose")); + C.addTransition(StateFailure); } void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call, @@ -663,6 +720,8 @@ // This is the "size or nmemb is zero" case. // Just return 0, do nothing more (not clear the error flags). State = bindInt(0, State, C, CE); + // It is unspecified what can happen with 'errno'. + State = errno_modeling::setErrnoState(State, errno_modeling::Irrelevant); C.addTransition(State); return; } @@ -674,7 +733,10 @@ State->BindExpr(CE, C.getLocationContext(), *NMembVal); StateNotFailed = StateNotFailed->set(StreamSym, StreamState::getOpened(Desc)); - C.addTransition(StateNotFailed); + StateNotFailed = errno_modeling::setErrnoForStdSuccess(StateNotFailed, C); + C.addTransition(StateNotFailed, + errno_modeling::getNoteTagForStdSuccess( + C, cast(Call.getDecl())->getNameAsString())); } // Add transition for the failed state. @@ -701,6 +763,8 @@ // indicator for the stream is indeterminate. StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); StateFailed = StateFailed->set(StreamSym, NewSS); + StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C, + getErrnoSVal(C, Call)); if (IsFread && OldSS->ErrorState != ErrorFEof) C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); else @@ -762,7 +826,12 @@ StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true)); - C.addTransition(StateNotFailed); + StateNotFailed = errno_modeling::setErrnoForStdSuccess(StateNotFailed, C); + StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C, + getErrnoSVal(C, Call)); + + C.addTransition(StateNotFailed, + errno_modeling::getNoteTagForStdSuccess(C, "fseek")); C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); } diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/stream-errno.c @@ -0,0 +1,196 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,debug.ExprInspection -analyzer-output text -verify %s + +#include "Inputs/system-header-simulator.h" +#include "Inputs/errno_func.h" + +extern void clang_analyzer_eval(int); +extern void clang_analyzer_dump(int); + +void check_fopen(void) { + FILE *F = fopen("xxx", "r"); + // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}} + // expected-note@+4{{'F' is null}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_tmpfile(void) { + FILE *F = tmpfile(); + // expected-note@-1{{Assuming that function 'tmpfile' is successful, in this case the value 'errno' may be undefined after the call and should not be used}} + // expected-note@+4{{'F' is null}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_freopen(void) { + FILE *F = tmpfile(); + // expected-note@+4{{'F' is non-null}} + // expected-note@+3{{Taking false branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + F = freopen("xxx", "w", F); + // expected-note@-1{{Assuming that function 'freopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}} + // expected-note@+4{{'F' is null}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_fclose(void) { + FILE *F = tmpfile(); + // expected-note@+4{{'F' is non-null}} + // expected-note@+3{{Taking false branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + int Ret = fclose(F); + // expected-note@-1{{Assuming that function 'fclose' is successful, in this case the value 'errno' may be undefined after the call and should not be used}} + // expected-note@+2{{Taking true branch}} + // expected-note@+1{{Taking false branch}} + if (Ret == EOF) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_fread(void) { + char Buf[10]; + FILE *F = tmpfile(); + // expected-note@+4{{'F' is non-null}} + // expected-note@+3{{Taking false branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + fread(Buf, 0, 1, F); + // expected-note@+2{{Taking false branch}} + // expected-note@+1{{Taking false branch}} + if (errno) {} // no-warning + fread(Buf, 1, 0, F); + // expected-note@+2{{Taking false branch}} + // expected-note@+1{{Taking false branch}} + if (errno) {} // no-warning + + int R = fread(Buf, 1, 10, F); + // expected-note@-1{{function 'fread' is successful}} + // expected-note@+4{{'R' is < 10}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'R' is >= 10}} + // expected-note@+1{{Taking false branch}} + if (R < 10) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + fclose(F); + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_fwrite(void) { + char Buf[] = "0123456789"; + FILE *F = tmpfile(); + // expected-note@+4{{'F' is non-null}} + // expected-note@+3{{Taking false branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + fwrite(Buf, 0, 1, F); + // expected-note@+2{{Taking false branch}} + // expected-note@+1{{Taking false branch}} + if (errno) {} // no-warning + fwrite(Buf, 1, 0, F); + // expected-note@+2{{Taking false branch}} + // expected-note@+1{{Taking false branch}} + if (errno) {} // no-warning + + int R = fwrite(Buf, 1, 10, F); + // expected-note@-1{{function 'fwrite' is successful}} + // expected-note@+4{{'R' is < 10}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'R' is >= 10}} + // expected-note@+1{{Taking false branch}} + if (R < 10) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + fclose(F); + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_fseek(void) { + FILE *F = tmpfile(); + // expected-note@+4{{'F' is non-null}} + // expected-note@+3{{Taking false branch}} + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + int S = fseek(F, 11, SEEK_SET); + // expected-note@-1{{Assuming that function 'fseek' is successful}} + // expected-note@+4{{'S' is not equal to 0}} + // expected-note@+3{{Taking true branch}} + // expected-note@+2{{'S' is equal to 0}} + // expected-note@+1{{Taking false branch}} + if (S != 0) { + clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}} + if (errno) {} + fclose(F); + return; + } + if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}} + // expected-note@-1{{An undefined value may be read from 'errno'}} +} + +void check_no_errno_change(void) { + FILE *F = tmpfile(); + // expected-note@+2{{'F' is non-null}} + // expected-note@+1{{Taking false branch}} + if (!F) + return; + errno = 1; + clearerr(F); + // expected-note@+1{{Taking true branch}} + if (errno) {} // no-warning + feof(F); + // expected-note@+1{{Taking true branch}} + if (errno) {} // no-warning + ferror(F); + // expected-note@+1{{Taking true branch}} + if (errno) {} // no-warning + clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}} expected-note{{TRUE}} + fclose(F); +} +