[clang][analyzer] Simplify code of StreamChecker (NFC). #82228
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Continuation of commit 42b5037, apply changes to the remaining functions. Code for function `fflush` was not changed, because it is more special compared to the others.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) ChangesContinuation of commit 42b5037, apply changes to the remaining functions. Patch is 22.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82228.diff 1 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 354217c1e52922..82296e0b83649b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,6 +265,11 @@ struct StreamOperationEvaluator {
SVB.makeIntVal(Val, CE->getCallReturnType(ACtx)));
}
+ ProgramStateRef bindReturnValue(ProgramStateRef State, CheckerContext &C,
+ SVal Val) {
+ return State->BindExpr(CE, C.getLocationContext(), Val);
+ }
+
ProgramStateRef bindNullReturnValue(ProgramStateRef State,
CheckerContext &C) {
return State->BindExpr(CE, C.getLocationContext(),
@@ -280,6 +285,13 @@ struct StreamOperationEvaluator {
return nullptr;
return State->assume(*Cond, true);
}
+
+ ProgramStatePair makeRetValAndAssumeDual(ProgramStateRef State,
+ CheckerContext &C) {
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ return C.getConstraintManager().assumeDual(State, RetVal);
+ }
};
class StreamChecker : public Checker<check::PreCall, eval::Call,
@@ -937,68 +949,47 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalFprintf(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
if (Call.getNumArgs() < 2)
return;
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto &ACtx = C.getASTContext();
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
+ State = State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ auto Cond =
+ E.SVB
+ .evalBinOp(State, BO_GE, RetVal, E.SVB.makeZeroVal(E.ACtx.IntTy),
+ E.SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
if (!Cond)
return;
ProgramStateRef StateNotFailed, StateFailed;
std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
C.addTransition(StateNotFailed);
// Add transition for the failed state. The resulting value of the file
// position indicator for the stream is indeterminate.
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, ErrorFError, true));
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, ErrorFError, true));
C.addTransition(StateFailed);
}
void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
if (Call.getNumArgs() < 2)
return;
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
- SValBuilder &SVB = C.getSValBuilder();
- ASTContext &ACtx = C.getASTContext();
-
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
@@ -1007,19 +998,15 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
// then EOF or read error happens. Now this case is handled like a "success"
// case, and no error flags are set on the stream. This is probably not
// accurate, and the POSIX documentation does not tell more.
- if (OldSS->ErrorState != ErrorFEof) {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ if (!E.isStreamEof()) {
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto RetGeZero =
- SVB.evalBinOp(StateNotFailed, BO_GE, RetVal,
- SVB.makeZeroVal(ACtx.IntTy), SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!RetGeZero)
- return;
- StateNotFailed = StateNotFailed->assume(*RetGeZero, true);
-
- C.addTransition(StateNotFailed);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed =
+ E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ *E.SVB.makeZeroVal(E.ACtx.IntTy).getAs<NonLoc>());
+ if (StateNotFailed)
+ C.addTransition(StateNotFailed);
}
// Add transition for the failed state.
@@ -1028,14 +1015,13 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
// be set but it is not further specified if all are required to be set.
// Documentation does not mention, but file position will be set to
// indeterminate similarly as at 'fread'.
- ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
- StreamErrorState NewES = (OldSS->ErrorState == ErrorFEof)
- ? ErrorFEof
- : ErrorNone | ErrorFEof | ErrorFError;
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
+ StreamErrorState NewES =
+ E.isStreamEof() ? ErrorFEof : ErrorNone | ErrorFEof | ErrorFError;
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (!E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
else
C.addTransition(StateFailed);
}
@@ -1043,28 +1029,17 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
// Generate a transition for the success state.
std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
if (!PutVal)
return;
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), *PutVal);
+ ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, *PutVal);
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
C.addTransition(StateNotFailed);
// Add transition for the failed state.
@@ -1073,9 +1048,8 @@ void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
// the same transition as the success state.
// In this case only one state transition is added by the analyzer (the two
// new states may be similar).
- ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
- StateFailed =
- StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
+ StateFailed = E.setStreamState(StateFailed, StreamState::getOpened(Desc));
C.addTransition(StateFailed);
}
@@ -1083,20 +1057,10 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
// Upon successful completion, the getline() and getdelim() functions shall
// return the number of bytes written into the buffer.
// If the end-of-file indicator for the stream is set, the function shall
@@ -1104,18 +1068,13 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
// If an error occurs, the function shall return -1 and set 'errno'.
// Add transition for the successful state.
- if (OldSS->ErrorState != ErrorFEof) {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ if (!E.isStreamEof()) {
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto Cond =
- SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(CE->getType()),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateNotFailed = StateNotFailed->assume(*Cond, true);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed =
+ E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ *E.SVB.makeZeroVal(E.CE->getType()).getAs<NonLoc>());
if (!StateNotFailed)
return;
C.addTransition(StateNotFailed);
@@ -1124,13 +1083,13 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
// Add transition for the failed state.
// If a (non-EOF) error occurs, the resulting value of the file position
// indicator for the stream is indeterminate.
- ProgramStateRef StateFailed = bindInt(-1, State, C, CE);
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
StreamErrorState NewES =
- OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError;
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
else
C.addTransition(StateFailed);
}
@@ -1156,16 +1115,8 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- // Ignore the call if the stream is not tracked.
- if (!State->get<StreamMap>(StreamSym))
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
const llvm::APSInt *PosV =
@@ -1173,56 +1124,38 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
const llvm::APSInt *WhenceV =
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
- DefinedSVal RetVal = makeRetVal(C, CE);
-
- // Make expression result.
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
-
// Bifurcate the state into failed and non-failed.
// Return zero on success, nonzero on error.
ProgramStateRef StateNotFailed, StateFailed;
- std::tie(StateFailed, StateNotFailed) =
- C.getConstraintManager().assumeDual(State, RetVal);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
- // Reset the state to opened with no error.
+ // No failure: Reset the state to opened with no error.
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- // We get error.
- // It is possible that fseek fails but sets none of the error flags.
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+
+ // 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.
StreamErrorState NewErrS = ErrorNone | ErrorFError;
// Setting the position to start of file never produces EOF error.
if (!(PosV && *PosV == 0 && WhenceV && *WhenceV == SeekSetVal))
NewErrS = NewErrS | ErrorFEof;
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, NewErrS, true));
-
- C.addTransition(StateNotFailed);
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ StateFailed = E.setStreamState(StateFailed,
+ StreamState::getOpened(Desc, NewErrS, true));
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.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;
-
- // Do not evaluate if stream is not found.
- if (!State->get<StreamMap>(Sym))
- return;
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- DefinedSVal RetVal = makeRetVal(C, CE);
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
ProgramStateRef StateNotFailed, StateFailed;
- std::tie(StateFailed, StateNotFailed) =
- C.getConstraintManager().assumeDual(State, RetVal);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
@@ -1235,35 +1168,22 @@ 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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
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);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
- StateNotFailed = StateNotFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+ StateNotFailed = E.setStreamState(
+ StateNotFailed, StreamState::getOpened(Desc, ErrorNone, false));
// At failure ferror could be set.
// 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 = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@@ -1272,33 +1192,19 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc,
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;
-
- if (!State->get<StreamMap>(Sym))
- return;
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- SValBuilder &SVB = C.getSValBuilder();
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond =
- SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(Call.getResultType()),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateNotFailed = StateNotFailed->assume(*Cond, true);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ SVB.makeZeroVal(Call.getResultType()));
if (!StateNotFailed)
return;
- ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1, Call.getResultType()));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
@@ -1310,23 +1216,12 @@ void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(SS);
-
- State = State->set<StreamMap>(StreamSym,
- StreamState::getOpened(Desc, ErrorNone, false));
-
+ State =
+ E.setStreamState(State, StreamState::getOpened(Desc, ErrorNone, false));
C.addTransition(State);
}
@@ -1334,19 +1229,13 @@ void StreamChecker::evalClearerr(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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(SS);
-
// FilePositionIndeterminate is not cleared.
- State = State->set<Stre...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Balázs Kéri (balazske) ChangesContinuation of commit 42b5037, apply changes to the remaining functions. Patch is 22.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82228.diff 1 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 354217c1e52922..82296e0b83649b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,6 +265,11 @@ struct StreamOperationEvaluator {
SVB.makeIntVal(Val, CE->getCallReturnType(ACtx)));
}
+ ProgramStateRef bindReturnValue(ProgramStateRef State, CheckerContext &C,
+ SVal Val) {
+ return State->BindExpr(CE, C.getLocationContext(), Val);
+ }
+
ProgramStateRef bindNullReturnValue(ProgramStateRef State,
CheckerContext &C) {
return State->BindExpr(CE, C.getLocationContext(),
@@ -280,6 +285,13 @@ struct StreamOperationEvaluator {
return nullptr;
return State->assume(*Cond, true);
}
+
+ ProgramStatePair makeRetValAndAssumeDual(ProgramStateRef State,
+ CheckerContext &C) {
+ DefinedSVal RetVal = makeRetVal(C, CE);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ return C.getConstraintManager().assumeDual(State, RetVal);
+ }
};
class StreamChecker : public Checker<check::PreCall, eval::Call,
@@ -937,68 +949,47 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalFprintf(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
if (Call.getNumArgs() < 2)
return;
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto &ACtx = C.getASTContext();
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
+ State = State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ auto Cond =
+ E.SVB
+ .evalBinOp(State, BO_GE, RetVal, E.SVB.makeZeroVal(E.ACtx.IntTy),
+ E.SVB.getConditionType())
+ .getAs<DefinedOrUnknownSVal>();
if (!Cond)
return;
ProgramStateRef StateNotFailed, StateFailed;
std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
C.addTransition(StateNotFailed);
// Add transition for the failed state. The resulting value of the file
// position indicator for the stream is indeterminate.
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, ErrorFError, true));
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, ErrorFError, true));
C.addTransition(StateFailed);
}
void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
if (Call.getNumArgs() < 2)
return;
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ ProgramStateRef State = C.getState();
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
- SValBuilder &SVB = C.getSValBuilder();
- ASTContext &ACtx = C.getASTContext();
-
// Add the success state.
// In this context "success" means there is not an EOF or other read error
// before any item is matched in 'fscanf'. But there may be match failure,
@@ -1007,19 +998,15 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
// then EOF or read error happens. Now this case is handled like a "success"
// case, and no error flags are set on the stream. This is probably not
// accurate, and the POSIX documentation does not tell more.
- if (OldSS->ErrorState != ErrorFEof) {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ if (!E.isStreamEof()) {
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto RetGeZero =
- SVB.evalBinOp(StateNotFailed, BO_GE, RetVal,
- SVB.makeZeroVal(ACtx.IntTy), SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!RetGeZero)
- return;
- StateNotFailed = StateNotFailed->assume(*RetGeZero, true);
-
- C.addTransition(StateNotFailed);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed =
+ E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ *E.SVB.makeZeroVal(E.ACtx.IntTy).getAs<NonLoc>());
+ if (StateNotFailed)
+ C.addTransition(StateNotFailed);
}
// Add transition for the failed state.
@@ -1028,14 +1015,13 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
// be set but it is not further specified if all are required to be set.
// Documentation does not mention, but file position will be set to
// indeterminate similarly as at 'fread'.
- ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
- StreamErrorState NewES = (OldSS->ErrorState == ErrorFEof)
- ? ErrorFEof
- : ErrorNone | ErrorFEof | ErrorFError;
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
+ StreamErrorState NewES =
+ E.isStreamEof() ? ErrorFEof : ErrorNone | ErrorFEof | ErrorFError;
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (!E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
else
C.addTransition(StateFailed);
}
@@ -1043,28 +1029,17 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
// Generate a transition for the success state.
std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
if (!PutVal)
return;
- ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), *PutVal);
+ ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, *PutVal);
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
C.addTransition(StateNotFailed);
// Add transition for the failed state.
@@ -1073,9 +1048,8 @@ void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent &Call,
// the same transition as the success state.
// In this case only one state transition is added by the analyzer (the two
// new states may be similar).
- ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
- StateFailed =
- StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
+ StateFailed = E.setStreamState(StateFailed, StreamState::getOpened(Desc));
C.addTransition(StateFailed);
}
@@ -1083,20 +1057,10 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- const StreamState *OldSS = State->get<StreamMap>(StreamSym);
- if (!OldSS)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(OldSS);
-
// Upon successful completion, the getline() and getdelim() functions shall
// return the number of bytes written into the buffer.
// If the end-of-file indicator for the stream is set, the function shall
@@ -1104,18 +1068,13 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
// If an error occurs, the function shall return -1 and set 'errno'.
// Add transition for the successful state.
- if (OldSS->ErrorState != ErrorFEof) {
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ if (!E.isStreamEof()) {
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- SValBuilder &SVB = C.getSValBuilder();
- auto Cond =
- SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(CE->getType()),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateNotFailed = StateNotFailed->assume(*Cond, true);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed =
+ E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ *E.SVB.makeZeroVal(E.CE->getType()).getAs<NonLoc>());
if (!StateNotFailed)
return;
C.addTransition(StateNotFailed);
@@ -1124,13 +1083,13 @@ void StreamChecker::evalGetdelim(const FnDescription *Desc,
// Add transition for the failed state.
// If a (non-EOF) error occurs, the resulting value of the file position
// indicator for the stream is indeterminate.
- ProgramStateRef StateFailed = bindInt(-1, State, C, CE);
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
StreamErrorState NewES =
- OldSS->ErrorState == ErrorFEof ? ErrorFEof : ErrorFEof | ErrorFError;
- StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
- StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
- if (OldSS->ErrorState != ErrorFEof)
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ E.isStreamEof() ? ErrorFEof : ErrorFEof | ErrorFError;
+ StateFailed = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, NewES, !NewES.isFEof()));
+ if (E.isStreamEof())
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.StreamSym));
else
C.addTransition(StateFailed);
}
@@ -1156,16 +1115,8 @@ void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
- SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
- if (!StreamSym)
- return;
-
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return;
-
- // Ignore the call if the stream is not tracked.
- if (!State->get<StreamMap>(StreamSym))
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
const llvm::APSInt *PosV =
@@ -1173,56 +1124,38 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
const llvm::APSInt *WhenceV =
C.getSValBuilder().getKnownValue(State, Call.getArgSVal(2));
- DefinedSVal RetVal = makeRetVal(C, CE);
-
- // Make expression result.
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
-
// Bifurcate the state into failed and non-failed.
// Return zero on success, nonzero on error.
ProgramStateRef StateNotFailed, StateFailed;
- std::tie(StateFailed, StateNotFailed) =
- C.getConstraintManager().assumeDual(State, RetVal);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
- // Reset the state to opened with no error.
+ // No failure: Reset the state to opened with no error.
StateNotFailed =
- StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- // We get error.
- // It is possible that fseek fails but sets none of the error flags.
+ E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+
+ // 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.
StreamErrorState NewErrS = ErrorNone | ErrorFError;
// Setting the position to start of file never produces EOF error.
if (!(PosV && *PosV == 0 && WhenceV && *WhenceV == SeekSetVal))
NewErrS = NewErrS | ErrorFEof;
- StateFailed = StateFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, NewErrS, true));
-
- C.addTransition(StateNotFailed);
- C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
+ StateFailed = E.setStreamState(StateFailed,
+ StreamState::getOpened(Desc, NewErrS, true));
+ C.addTransition(StateFailed, constructSetEofNoteTag(C, E.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;
-
- // Do not evaluate if stream is not found.
- if (!State->get<StreamMap>(Sym))
- return;
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- DefinedSVal RetVal = makeRetVal(C, CE);
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
ProgramStateRef StateNotFailed, StateFailed;
- std::tie(StateFailed, StateNotFailed) =
- C.getConstraintManager().assumeDual(State, RetVal);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
@@ -1235,35 +1168,22 @@ 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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
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);
+ std::tie(StateFailed, StateNotFailed) = E.makeRetValAndAssumeDual(State, C);
- StateNotFailed = StateNotFailed->set<StreamMap>(
- StreamSym, StreamState::getOpened(Desc, ErrorNone, false));
+ StateNotFailed = E.setStreamState(
+ StateNotFailed, StreamState::getOpened(Desc, ErrorNone, false));
// At failure ferror could be set.
// 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 = E.setStreamState(
+ StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
C.addTransition(StateNotFailed);
C.addTransition(StateFailed);
@@ -1272,33 +1192,19 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc,
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;
-
- if (!State->get<StreamMap>(Sym))
- return;
-
- auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- SValBuilder &SVB = C.getSValBuilder();
- NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+ NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
ProgramStateRef StateNotFailed =
- State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond =
- SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(Call.getResultType()),
- SVB.getConditionType())
- .getAs<DefinedOrUnknownSVal>();
- if (!Cond)
- return;
- StateNotFailed = StateNotFailed->assume(*Cond, true);
+ State->BindExpr(E.CE, C.getLocationContext(), RetVal);
+ StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal,
+ SVB.makeZeroVal(Call.getResultType()));
if (!StateNotFailed)
return;
- ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1, Call.getResultType()));
+ ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
@@ -1310,23 +1216,12 @@ void StreamChecker::evalFtell(const FnDescription *Desc, const CallEvent &Call,
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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(SS);
-
- State = State->set<StreamMap>(StreamSym,
- StreamState::getOpened(Desc, ErrorNone, false));
-
+ State =
+ E.setStreamState(State, StreamState::getOpened(Desc, ErrorNone, false));
C.addTransition(State);
}
@@ -1334,19 +1229,13 @@ void StreamChecker::evalClearerr(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)
+ StreamOperationEvaluator E(C);
+ if (!E.Init(Desc, Call, C, State))
return;
- assertStreamStateOpened(SS);
-
// FilePositionIndeterminate is not cleared.
- State = State->set<Stre...
[truncated]
|
NagyDonat
approved these changes
Feb 19, 2024
There was a problem hiding this comment.
Seems to be a nice improvement, I'm happy to see this boilerplate removal and reducing the code length by 120 lines is impressive. (Disclaimer: I didn't do a line-by-line review of the changes, but I hope that testing etc. validates that the change is indeed NFC.)
However, please add "part 2" to the title line of the commit, because it is/was confusing to have two commits with identical titles that are very close to each other.
In general, the title line should be an identifier that's reasonable unique for the human readers (while the commit hash is the machine-readable exact identifier). It's completely fine if a commit title happens to be identical to a 10-year-old commit, but please try to avoid confusion between commits that might "appear" in the same context.
benshi001
approved these changes
Feb 20, 2024
|
The initial commit was somehow incomplete and code was not tested. New commit should fix the problems. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuation of commit 42b5037, apply changes to the remaining functions.
Code for function
fflushwas not changed, because it is more special compared to the others.