balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Functions `fsetpos`, `fgetpos`, `ftell`, `rewind` are added.
Function `fileno` is handled by `StdLibraryFunctionsChecker` too,
tests are added for it but eval function is not needed.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132249
Files:
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-errno-note.c
clang/test/Analysis/stream-errno.c
clang/test/Analysis/stream-error.c
Index: clang/test/Analysis/stream-error.c
===================================================================
--- clang/test/Analysis/stream-error.c
+++ clang/test/Analysis/stream-error.c
@@ -11,6 +11,9 @@
void StreamTesterChecker_make_feof_stream(FILE *);
void StreamTesterChecker_make_ferror_stream(FILE *);
+const char *WBuf = "123456789";
+char RBuf[10];
+
void error_fopen(void) {
FILE *F = fopen("file", "r");
if (!F)
@@ -233,3 +236,37 @@
}
fclose(F);
}
+
+void error_fsetpos(void) {
+ FILE *F = fopen("file", "w");
+ if (!F)
+ return;
+ fpos_t Pos;
+ int rc = fsetpos(F, &Pos);
+ clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}}
+ if (rc) {
+ if (ferror(F))
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ else
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ } else {
+ clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}}
+ }
+ fwrite(WBuf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
+ fclose(F);
+}
+
+void error_rewind(void) {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ fread(RBuf, 1, 10, F);
+ if (feof(F)) {
+ rewind(F);
+ clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}}
+ } else if (ferror(F)) {
+ rewind(F);
+ clang_analyzer_eval(ferror(F)); // expected-warning{{FALSE}}
+ }
+ fclose(F);
+}
Index: clang/test/Analysis/stream-errno.c
===================================================================
--- clang/test/Analysis/stream-errno.c
+++ clang/test/Analysis/stream-errno.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions,debug.ExprInspection \
-// RUN: -verify %s
+// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -verify %s
#include "Inputs/system-header-simulator.h"
#include "Inputs/errno_func.h"
@@ -121,3 +121,75 @@
clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}}
fclose(F);
}
+
+void check_fgetpos(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fgetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_fsetpos(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ fpos_t Pos;
+ int Ret = fsetpos(F, &Pos);
+ if (Ret)
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ else
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_ftell(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ long Ret = ftell(F);
+ if (Ret == -1) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ } else {
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+ }
+ if (errno) {} // no-warning
+ fclose(F);
+}
+
+void check_rewind(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ errno = 0;
+ rewind(F);
+ clang_analyzer_eval(errno == 0);
+ // expected-warning@-1{{FALSE}}
+ // expected-warning@-2{{TRUE}}
+ fclose(F);
+}
+
+void check_fileno(void) {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ int N = fileno(F);
+ if (N == -1) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ if (errno) {} // no-warning
+ fclose(F);
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+}
Index: clang/test/Analysis/stream-errno-note.c
===================================================================
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,apiModeling.StdCLibraryFunctions \
-// RUN: -analyzer-output text -verify %s
+// RUN: -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true -analyzer-output text -verify %s
#include "Inputs/system-header-simulator.h"
#include "Inputs/errno_func.h"
@@ -97,3 +97,29 @@
// expected-note@-1{{An undefined value may be read from 'errno'}}
(void)fclose(F);
}
+
+void check_rewind_errnocheck(void) {
+ FILE *F = tmpfile();
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ errno = 0;
+ rewind(F); // expected-note{{Function 'rewind' indicates failure only by setting of 'errno'}}
+ fclose(F); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'fclose' [alpha.unix.Errno]}}
+ // expected-note@-1{{Value of 'errno' was not checked and may be overwritten by function 'fclose'}}
+}
+
+// 'fileno' is modeled by StdCLibraryFunctions checker (the errno related parts)
+void check_fileno(void) {
+ FILE *F = tmpfile();
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ fileno(F);
+ // expected-note@-1{{Assuming that function 'fileno' is successful}}
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+ (void)fclose(F);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -251,10 +251,14 @@
{&StreamChecker::preFwrite,
std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
{{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
- {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
- {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+ {{"ftell", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+ {{"rewind", 1},
+ {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
+ {{"fgetpos", 2},
+ {&StreamChecker::preDefault, &StreamChecker::evalFgetpos, 0}},
+ {{"fsetpos", 2},
+ {&StreamChecker::preDefault, &StreamChecker::evalFsetpos, 0}},
{{"clearerr", 1},
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
{{"feof", 1},
@@ -265,6 +269,8 @@
{&StreamChecker::preDefault,
std::bind(&StreamChecker::evalFeofFerror, _1, _2, _3, _4, ErrorFError),
0}},
+ // 'fileno' is checked sufficiently by StdLibraryFunctionsChecker.
+ // This check additionally checks if the file is in opened state.
{{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
};
@@ -307,6 +313,18 @@
void evalFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
+ void evalFgetpos(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFsetpos(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalFtell(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+ void evalRewind(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
void preDefault(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -835,6 +853,153 @@
C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
}
+void StreamChecker::evalFgetpos(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!Sym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(Sym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ // In the success case, do not change 'errno' and errno state.
+ StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+ getErrnoSVal(C, Call));
+ if (!StateFailed)
+ return;
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFsetpos(const FnDescription *Desc,
+ const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateFailed, StateNotFailed) =
+ C.getConstraintManager().assumeDual(State, RetVal);
+
+ // In the success case, do not change 'errno' and errno state.
+ StateNotFailed = StateNotFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+
+ // At failure it may happen that ferror is set but not always.
+ // The standards do not tell what happens with the file position at failure.
+ // But we can assume that it is dangerous to make a next I/O operation after
+ // the position was not set correctly (similar to 'fseek').
+ StateFailed = StateFailed->set<StreamMap>(
+ StreamSym, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
+ StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+ getErrnoSVal(C, Call));
+ if (!StateFailed)
+ return;
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef Sym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!Sym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(Sym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ SValBuilder &SVB = C.getSValBuilder();
+ NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), RetVal);
+ auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
+ SVB.makeZeroVal(C.getASTContext().LongTy),
+ SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cond)
+ return;
+ StateNotFailed = StateNotFailed->assume(*Cond, true);
+ if (!StateNotFailed)
+ return;
+
+ ProgramStateRef StateFailed = State->BindExpr(
+ CE, C.getLocationContext(), SVB.makeIntVal(-1, C.getASTContext().LongTy));
+
+ StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+ getErrnoSVal(C, Call));
+ if (!StateFailed)
+ return;
+
+ C.addTransition(StateNotFailed);
+ C.addTransition(StateFailed);
+}
+
+void StreamChecker::evalRewind(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+ return;
+
+ const StreamState *SS = State->get<StreamMap>(StreamSym);
+ if (!SS)
+ return;
+
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
+ assertStreamStateOpened(SS);
+
+ State = State->set<StreamMap>(StreamSym,
+ StreamState::getOpened(Desc, ErrorNone, false));
+
+ State =
+ errno_modeling::setErrnoStdMustBeChecked(State, C, Call.getOriginExpr());
+
+ C.addTransition(State,
+ errno_modeling::getNoteTagForStdMustBeChecked(C, "rewind"));
+}
+
void StreamChecker::evalClearerr(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -101,11 +101,26 @@
ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
NonLoc ErrnoSym);
+/// Set errno state for the common case when a standard function indicates
+/// failure only by \c errno. Sets \c ErrnoCheckState to \c MustBeChecked, and
+/// invalidates the errno region (clear of previous value).
+/// At the state transition a note tag created by
+/// \c getNoteTagForStdMustBeChecked can be used.
+/// \arg \c InvalE Expression that causes invalidation of \c errno.
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C, const Expr *InvalE);
+
/// Generate the note tag that can be applied at the state generated by
/// \c setErrnoForStdSuccess .
/// \arg \c Fn Name of the (standard) function that is modeled.
const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn);
+/// Generate the note tag that can be applied at the state generated by
+/// \c setErrnoStdMustBeChecked .
+/// \arg \c Fn Name of the (standard) function that is modeled.
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+ llvm::StringRef Fn);
+
} // namespace errno_modeling
} // namespace ento
} // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -299,6 +299,19 @@
return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
}
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C,
+ const Expr *InvalE) {
+ const MemRegion *ErrnoR = State->get<ErrnoRegion>();
+ if (!ErrnoR)
+ return State;
+ State = State->invalidateRegions(ErrnoR, InvalE, C.blockCount(),
+ C.getLocationContext(), false);
+ if (!State)
+ return nullptr;
+ return setErrnoState(State, MustBeChecked);
+}
+
const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
return getErrnoNoteTag(
C, (Twine("Assuming that function '") + Twine(Fn) +
@@ -307,6 +320,14 @@
.str());
}
+const NoteTag *getNoteTagForStdMustBeChecked(CheckerContext &C,
+ llvm::StringRef Fn) {
+ return getErrnoNoteTag(
+ C, (Twine("Function '") + Twine(Fn) +
+ Twine("' indicates failure only by setting of 'errno'"))
+ .str());
+}
+
} // namespace errno_modeling
} // namespace ento
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
