[clang] [clang][analyzer] Simplify code of StreamChecker (NFC). (PR #82228)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 19 00:49:39 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
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.
---
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:
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+112-233)
``````````diff
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]
``````````
</details>
https://github.com/llvm/llvm-project/pull/82228
More information about the cfe-commits
mailing list