diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index f188f18ba5557..fb748d23a53d0 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3138,10 +3138,16 @@ are detected: allowed in this state. * Invalid 3rd ("``whence``") argument to ``fseek``. -The checker does not track the correspondence between integer file descriptors -and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not -treated specially and are therefore often not recognized (because these streams -are usually not opened explicitly by the program, and are global variables). +The stream operations are by this checker usually split into two cases, a success +and a failure case. However, in the case of write operations (like ``fwrite``, +``fprintf`` and even ``fsetpos``) this behavior could produce a large amount of +unwanted reports on projects that don't have error checks around the write +operations, so by default the checker assumes that write operations always succeed. +This behavior can be controlled by the ``Pedantic`` flag: With +``-analyzer-config alpha.unix.Stream:Pedantic=true`` the checker will model the +cases where a write operation fails and report situations where this leads to +erroneous behavior. (The default is ``Pedantic=false``, where write operations +are assumed to succeed.) .. code-block:: c @@ -3196,6 +3202,13 @@ are usually not opened explicitly by the program, and are global variables). fclose(p); } +**Limitations** + +The checker does not track the correspondence between integer file descriptors +and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not +treated specially and are therefore often not recognized (because these streams +are usually not opened explicitly by the program, and are global variables). + .. _alpha-unix-cstring-BufferOverlap: alpha.unix.cstring.BufferOverlap (C) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 5fe5c9286dabb..9aa1c6ddfe449 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -604,6 +604,15 @@ def PthreadLockChecker : Checker<"PthreadLock">, def StreamChecker : Checker<"Stream">, HelpText<"Check stream handling functions">, WeakDependencies<[NonNullParamChecker]>, + CheckerOptions<[ + CmdLineOption + ]>, Documentation; def SimpleStreamChecker : Checker<"SimpleStream">, diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 069e3a633c121..31c756ab0c581 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -297,6 +297,9 @@ class StreamChecker : public Checker FnDescriptions = { {{{"fopen"}, 2}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, @@ -945,6 +948,10 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, } // Add transition for the failed state. + // At write, add failure case only if "pedantic mode" is on. + if (!IsFread && !PedanticMode) + return; + NonLoc RetVal = makeRetVal(C, E.CE).castAs(); ProgramStateRef StateFailed = State->BindExpr(E.CE, C.getLocationContext(), RetVal); @@ -1057,6 +1064,9 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call, C.addTransition(StateNotFailed); } + if (!PedanticMode) + return; + // Add transition for the failed state. The resulting value of the file // position indicator for the stream is indeterminate. ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal); @@ -1092,6 +1102,9 @@ void StreamChecker::evalFprintf(const FnDescription *Desc, E.setStreamState(StateNotFailed, StreamState::getOpened(Desc)); C.addTransition(StateNotFailed); + if (!PedanticMode) + return; + // Add transition for the failed state. The resulting value of the file // position indicator for the stream is indeterminate. StateFailed = E.setStreamState( @@ -1264,21 +1277,23 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call, if (!E.Init(Desc, Call, C, State)) return; - // Bifurcate the state into failed and non-failed. - // Return zero on success, -1 on error. + // Add success state. ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, 0); - ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1); - // No failure: Reset the state to opened with no error. StateNotFailed = E.setStreamState(StateNotFailed, StreamState::getOpened(Desc)); C.addTransition(StateNotFailed); + if (!PedanticMode) + return; + + // Add failure state. // At error it is possible that fseek fails but sets none of the error flags. // If fseek failed, assume that the file position becomes indeterminate in any // case. // It is allowed to set the position beyond the end of the file. EOF error // should not occur. + ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1); StateFailed = E.setStreamState( StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true)); C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); @@ -1316,6 +1331,10 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc, StateNotFailed = E.setStreamState( StateNotFailed, StreamState::getOpened(Desc, ErrorNone, false)); + C.addTransition(StateNotFailed); + + if (!PedanticMode) + return; // At failure ferror could be set. // The standards do not tell what happens with the file position at failure. @@ -1324,7 +1343,6 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc, StateFailed = E.setStreamState( StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true)); - C.addTransition(StateNotFailed); C.addTransition(StateFailed, E.getFailureNoteTag(this, C)); } @@ -1794,7 +1812,9 @@ ProgramStateRef StreamChecker::checkPointerEscape( //===----------------------------------------------------------------------===// void ento::registerStreamChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + auto *Checker = Mgr.registerChecker(); + Checker->PedanticMode = + Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "Pedantic"); } bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) { diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 2167a2b32f596..23e37a856d09f 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -12,6 +12,7 @@ // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = "" +// CHECK-NEXT: alpha.unix.Stream:Pedantic = false // CHECK-NEXT: apply-fixits = false // CHECK-NEXT: assume-controlled-environment = false // CHECK-NEXT: avoid-suppressing-null-argument-paths = false diff --git a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c b/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c index 281fbaaffe703..cac3fe5c5151c 100644 --- a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c +++ b/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c @@ -1,6 +1,7 @@ // Check the case when only the StreamChecker is enabled. // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core,alpha.unix.Stream \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -analyzer-config eagerly-assume=false \ // RUN: -triple x86_64-unknown-linux \ @@ -19,6 +20,7 @@ // StdLibraryFunctionsChecker are enabled. // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core,alpha.unix.Stream \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-config unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \ // RUN: -analyzer-checker=debug.ExprInspection \ diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c index 2411a2d9a00a7..fb12f0bace937 100644 --- a/clang/test/Analysis/stream-errno-note.c +++ b/clang/test/Analysis/stream-errno-note.c @@ -1,5 +1,6 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-checker=unix.Errno \ // RUN: -analyzer-checker=unix.StdCLibraryFunctions \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \ diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c index 5f0a58032fa26..08382eaf6abf9 100644 --- a/clang/test/Analysis/stream-errno.c +++ b/clang/test/Analysis/stream-errno.c @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify %s #include "Inputs/system-header-simulator.h" diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c index 7f9116ff40144..2abf4b900a047 100644 --- a/clang/test/Analysis/stream-error.c +++ b/clang/test/Analysis/stream-error.c @@ -1,6 +1,7 @@ // RUN: %clang_analyze_cc1 -verify %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-checker=debug.StreamTester \ // RUN: -analyzer-checker=debug.ExprInspection diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c index 54ea699f46674..03a8ff4e468f6 100644 --- a/clang/test/Analysis/stream-note.c +++ b/clang/test/Analysis/stream-note.c @@ -1,6 +1,8 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -analyzer-output text \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.StdCLibraryFunctions -analyzer-output text \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=expected,stdargs %s #include "Inputs/system-header-simulator.h" diff --git a/clang/test/Analysis/stream-pedantic.c b/clang/test/Analysis/stream-pedantic.c new file mode 100644 index 0000000000000..2bbea81d47ef6 --- /dev/null +++ b/clang/test/Analysis/stream-pedantic.c @@ -0,0 +1,95 @@ +// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=false -verify=nopedantic %s + +// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true -verify=pedantic %s + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); + +void check_fwrite(void) { + char *Buf = "123456789"; + FILE *Fp = tmpfile(); + if (!Fp) + return; + size_t Ret = fwrite(Buf, 1, 10, Fp); + clang_analyzer_eval(Ret == 0); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fputc(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + int Ret = fputc('A', Fp); + clang_analyzer_eval(Ret == EOF); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fputs(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + int Ret = fputs("ABC", Fp); + clang_analyzer_eval(Ret == EOF); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fprintf(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + int Ret = fprintf(Fp, "ABC"); + clang_analyzer_eval(Ret < 0); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fseek(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + int Ret = fseek(Fp, 0, 0); + clang_analyzer_eval(Ret == -1); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fseeko(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + int Ret = fseeko(Fp, 0, 0); + clang_analyzer_eval(Ret == -1); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} + +void check_fsetpos(void) { + FILE *Fp = tmpfile(); + if (!Fp) + return; + fpos_t Pos; + int Ret = fsetpos(Fp, &Pos); + clang_analyzer_eval(Ret); // nopedantic-warning {{FALSE}} \ + // pedantic-warning {{FALSE}} \ + // pedantic-warning {{TRUE}} + fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}} + fclose(Fp); +} diff --git a/clang/test/Analysis/stream-stdlibraryfunctionargs.c b/clang/test/Analysis/stream-stdlibraryfunctionargs.c index 0053510163efc..2ea6a8c472c61 100644 --- a/clang/test/Analysis/stream-stdlibraryfunctionargs.c +++ b/clang/test/Analysis/stream-stdlibraryfunctionargs.c @@ -1,10 +1,13 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.StdCLibraryFunctions,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.StdCLibraryFunctions,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \ // RUN: -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stdfunc,any %s #include "Inputs/system-header-simulator.h" diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index ba5e66a4102e3..93ed555c89ebd 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -1,7 +1,11 @@ -// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s -// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s -// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s -// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s +// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s +// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s +// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \ +// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s #include "Inputs/system-header-simulator.h" #include "Inputs/system-header-simulator-for-malloc.h"