[clang] c2067c1 - [clang][analyzer] Add "pedantic" mode to StreamChecker. (#87322)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 8 03:19:07 PDT 2024
Author: Balázs Kéri
Date: 2024-04-08T12:19:03+02:00
New Revision: c2067c1f471ac54312cb5e1e0efd4ea5fd21cc79
URL: https://github.com/llvm/llvm-project/commit/c2067c1f471ac54312cb5e1e0efd4ea5fd21cc79
DIFF: https://github.com/llvm/llvm-project/commit/c2067c1f471ac54312cb5e1e0efd4ea5fd21cc79.diff
LOG: [clang][analyzer] Add "pedantic" mode to StreamChecker. (#87322)
The checker may create failure branches for all stream write operations
only if the new option "pedantic" is set to true.
Result of the write operations is often not checked in typical code. If
failure branches are created the checker will warn for unchecked write
operations and generate a lot of "false positives" (these are valid
warnings but the programmer does not care about this problem).
Added:
clang/test/Analysis/stream-pedantic.c
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
clang/test/Analysis/stream-errno-note.c
clang/test/Analysis/stream-errno.c
clang/test/Analysis/stream-error.c
clang/test/Analysis/stream-note.c
clang/test/Analysis/stream-stdlibraryfunctionargs.c
clang/test/Analysis/stream.c
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index f188f18ba5557e..fb748d23a53d01 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 5fe5c9286dabb7..9aa1c6ddfe4492 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<Boolean,
+ "Pedantic",
+ "If false, assume that stream operations which are often not "
+ "checked for error do not fail."
+ "fail.",
+ "false",
+ InAlpha>
+ ]>,
Documentation<HasDocumentation>;
def SimpleStreamChecker : Checker<"SimpleStream">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 069e3a633c1214..31c756ab0c5812 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -297,6 +297,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
/// If true, evaluate special testing stream functions.
bool TestMode = false;
+ /// If true, generate failure branches for cases that are often not checked.
+ bool PedanticMode = false;
+
private:
CallDescriptionMap<FnDescription> 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<NonLoc>();
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<StreamChecker>();
+ auto *Checker = Mgr.registerChecker<StreamChecker>();
+ 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 2167a2b32f5962..23e37a856d09f7 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 281fbaaffe7034..cac3fe5c5151cd 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 2411a2d9a00a72..fb12f0bace937f 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 5f0a58032fa263..08382eaf6abf9f 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 7f9116ff401445..2abf4b900a047f 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 54ea699f46674e..03a8ff4e468f66 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 00000000000000..2bbea81d47ef60
--- /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 0053510163efc0..2ea6a8c472c613 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 ba5e66a4102e3c..93ed555c89ebd1 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"
More information about the cfe-commits
mailing list