[clang] [clang][analyzer] Add "pedantic" mode to StreamChecker. (PR #87322)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 2 01:12:04 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
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).
---
Full diff: https://github.com/llvm/llvm-project/pull/87322.diff
9 Files Affected:
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+8)
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+26-6)
- (modified) clang/test/Analysis/stream-errno-note.c (+1)
- (modified) clang/test/Analysis/stream-errno.c (+1)
- (modified) clang/test/Analysis/stream-error.c (+1)
- (modified) clang/test/Analysis/stream-note.c (+2)
- (added) clang/test/Analysis/stream-pedantic.c (+71)
- (modified) clang/test/Analysis/stream-stdlibraryfunctionargs.c (+3)
- (modified) clang/test/Analysis/stream.c (+8-4)
``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 5fe5c9286dabb7..570e0849e0124b 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -604,6 +604,14 @@ def PthreadLockChecker : Checker<"PthreadLock">,
def StreamChecker : Checker<"Stream">,
HelpText<"Check stream handling functions">,
WeakDependencies<[NonNullParamChecker]>,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "Pedantic",
+ "If false, assume that stream write operations do never "
+ "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..337420c3b25df3 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);
@@ -1059,6 +1066,9 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
// Add transition for the failed state. The resulting value of the file
// position indicator for the stream is indeterminate.
+ if (!PedanticMode)
+ return;
+
ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
StateFailed = E.setStreamState(
StateFailed, StreamState::getOpened(Desc, ErrorFError, true));
@@ -1094,6 +1104,9 @@ void StreamChecker::evalFprintf(const FnDescription *Desc,
// Add transition for the failed state. The resulting value of the file
// position indicator for the stream is indeterminate.
+ if (!PedanticMode)
+ return;
+
StateFailed = E.setStreamState(
StateFailed, StreamState::getOpened(Desc, ErrorFError, true));
C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
@@ -1264,16 +1277,18 @@ 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);
+ // Add failure state.
+ if (!PedanticMode)
+ return;
+
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
// 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.
@@ -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/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..5cdf70f1630eb2
--- /dev/null
+++ b/clang/test/Analysis/stream-pedantic.c
@@ -0,0 +1,71 @@
+// 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 %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); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fputc(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ int Ret = fputc('A', Fp);
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fputs(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ int Ret = fputs("ABC", Fp);
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fprintf(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ int Ret = fprintf(Fp, "ABC");
+ clang_analyzer_eval(Ret < 0); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fseek(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ int Ret = fseek(Fp, 0, 0);
+ clang_analyzer_eval(Ret == -1); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fseeko(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ int Ret = fseeko(Fp, 0, 0);
+ clang_analyzer_eval(Ret == -1); // expected-warning {{FALSE}}
+ fclose(Fp);
+}
+
+void check_fsetpos(void) {
+ FILE *Fp = tmpfile();
+ if (!Fp)
+ return;
+ fpos_t Pos;
+ int Ret = fsetpos(Fp, &Pos);
+ clang_analyzer_eval(Ret); // expected-warning {{FALSE}}
+ 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"
``````````
</details>
https://github.com/llvm/llvm-project/pull/87322
More information about the cfe-commits
mailing list