diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index a070f451694a3..65bdc4cac3094 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -21,6 +21,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" +#include "llvm/ADT/Sequence.h" #include #include @@ -629,6 +630,21 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, return nullptr; } +static ProgramStateRef escapeArgs(ProgramStateRef State, CheckerContext &C, + const CallEvent &Call, + ArrayRef EscapingArgs) { + const auto *CE = Call.getOriginExpr(); + + SmallVector EscapingVals; + EscapingVals.reserve(EscapingArgs.size()); + for (auto EscArgIdx : EscapingArgs) + EscapingVals.push_back(Call.getArgSVal(EscArgIdx)); + State = State->invalidateRegions(EscapingVals, CE, C.blockCount(), + C.getLocationContext(), + /*CausesPointerEscape=*/false); + return State; +} + //===----------------------------------------------------------------------===// // Methods of StreamChecker. //===----------------------------------------------------------------------===// @@ -819,6 +835,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, return; } + // At read, invalidate the buffer in any case of error or success, + // except if EOF was already present. + if (IsFread && !E.isStreamEof()) + State = escapeArgs(State, C, Call, {0}); + // Generate a transition for the success state. // If we know the state to be FEOF at fread, do not add a success state. if (!IsFread || !E.isStreamEof()) { @@ -863,6 +884,9 @@ void StreamChecker::evalFgetx(const FnDescription *Desc, const CallEvent &Call, return; if (!E.isStreamEof()) { + // If there was already EOF, assume that read buffer is not changed. + // Otherwise it may change at success or failure. + State = escapeArgs(State, C, Call, {0}); if (SingleChar) { // Generate a transition for the success state of `fgetc`. NonLoc RetVal = makeRetVal(C, E.CE).castAs(); @@ -1011,6 +1035,14 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call, State->BindExpr(E.CE, C.getLocationContext(), RetVal); StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call)); + if (!StateNotFailed) + return; + + SmallVector EscArgs; + for (auto EscArg : llvm::seq(2u, Call.getNumArgs())) + EscArgs.push_back(EscArg); + StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs); + if (StateNotFailed) C.addTransition(StateNotFailed); } @@ -1073,8 +1105,12 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc, // return -1. // If an error occurs, the function shall return -1 and set 'errno'. - // Add transition for the successful state. if (!E.isStreamEof()) { + // Escape buffer and size (may change by the call). + // May happen even at error (partial read?). + State = escapeArgs(State, C, Call, {0, 1}); + + // Add transition for the successful state. NonLoc RetVal = makeRetVal(C, E.CE).castAs(); ProgramStateRef StateNotFailed = State->BindExpr(E.CE, C.getLocationContext(), RetVal); @@ -1161,6 +1197,7 @@ void StreamChecker::evalFgetpos(const FnDescription *Desc, ProgramStateRef StateNotFailed, StateFailed; std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C); + StateNotFailed = escapeArgs(StateNotFailed, C, Call, {1}); // This function does not affect the stream state. // Still we add success and failure state with the appropriate return value. diff --git a/clang/test/Analysis/stream-invalidate.c b/clang/test/Analysis/stream-invalidate.c new file mode 100644 index 0000000000000..6745d11a2fe70 --- /dev/null +++ b/clang/test/Analysis/stream-invalidate.c @@ -0,0 +1,147 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-checker=debug.ExprInspection + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump(int); + +void test_fread(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + + char Buf[3] = {10, 10, 10}; + fread(Buf, 1, 3, F); + // The check applies to success and failure. + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { + char Buf1[3] = {10, 10, 10}; + fread(Buf1, 1, 3, F); // expected-warning {{is in EOF state}} + clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} + clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fwrite(void) { + FILE *F = fopen("file", "r+"); + if (!F) + return; + + char Buf[3] = {10, 10, 10}; + fwrite(Buf, 1, 3, F); + // The check applies to success and failure. + clang_analyzer_dump(Buf[0]); // expected-warning {{10 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{10 S32b}} + + fclose(F); +} + +void test_fgets() { + FILE *F = tmpfile(); + if (!F) + return; + + char Buf[3] = {10, 10, 10}; + fgets(Buf, 3, F); + // The check applies to success and failure. + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { + char Buf1[3] = {10, 10, 10}; + fgets(Buf1, 3, F); // expected-warning {{is in EOF state}} + clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} + clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fputs() { + FILE *F = tmpfile(); + if (!F) + return; + + char *Buf = "aaa"; + fputs(Buf, F); + // The check applies to success and failure. + clang_analyzer_dump(Buf[0]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[3]); // expected-warning {{0 S32b}} + + fclose(F); +} + +void test_fscanf() { + FILE *F = tmpfile(); + if (!F) + return; + + int a = 1; + unsigned b; + int Ret = fscanf(F, "%d %u", &a, &b); + if (Ret == 0) { + clang_analyzer_dump(a); // expected-warning {{conj_$}} + // FIXME: should be {{1 S32b}}. + clang_analyzer_dump(b); // expected-warning {{conj_$}} + // FIXME: should be {{uninitialized value}}. + } else if (Ret == 1) { + clang_analyzer_dump(a); // expected-warning {{conj_$}} + clang_analyzer_dump(b); // expected-warning {{conj_$}} + // FIXME: should be {{uninitialized value}}. + } else if (Ret >= 2) { + clang_analyzer_dump(a); // expected-warning {{conj_$}} + clang_analyzer_dump(b); // expected-warning {{conj_$}} + clang_analyzer_eval(Ret == 2); // expected-warning {{FALSE}} expected-warning {{TRUE}} + // FIXME: should be only TRUE. + } else { + clang_analyzer_dump(a); // expected-warning {{1 S32b}} + clang_analyzer_dump(b); // expected-warning {{uninitialized value}} + } + + fclose(F); +} + +void test_getdelim(char *P, size_t Sz) { + FILE *F = tmpfile(); + if (!F) + return; + + char *P1 = P; + size_t Sz1 = Sz; + ssize_t Ret = getdelim(&P, &Sz, '\t', F); + if (Ret < 0) { + clang_analyzer_eval(P == P1); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} + clang_analyzer_eval(Sz == Sz1); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} + } else { + clang_analyzer_eval(P == P1); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} + clang_analyzer_eval(Sz == Sz1); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} + } + + fclose(F); +} + +void test_fgetpos() { + FILE *F = tmpfile(); + if (!F) + return; + + fpos_t Pos = 1; + int Ret = fgetpos(F, &Pos); + if (Ret == 0) { + clang_analyzer_dump(Pos); // expected-warning {{conj_$}} + } else { + clang_analyzer_dump(Pos); // expected-warning {{1 S32b}} + } + + fclose(F); +}