[Analyzer][StreamChecker] Add check for pointer escape.

Summary:
After an escaped FILE* stream handle it is not possible to make
reliable checks on it because any function call can have effect
on it.

Reviewers: Szelethus, baloghadamsoftware, martong, NoQ

Reviewed By: NoQ

Subscribers: NoQ, rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D80699
This commit is contained in:
Balázs Kéri 2020-06-15 14:59:54 +02:00
parent 669066de65
commit efa8b6e884
2 changed files with 81 additions and 4 deletions

View File

@ -193,8 +193,8 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
return State;
}
class StreamChecker
: public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
class StreamChecker : public Checker<check::PreCall, eval::Call,
check::DeadSymbols, check::PointerEscape> {
BuiltinBug BT_FileNull{this, "NULL stream pointer",
"Stream pointer might be NULL."};
BuiltinBug BT_UseAfterClose{
@ -223,6 +223,10 @@ public:
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
ProgramStateRef checkPointerEscape(ProgramStateRef State,
const InvalidatedSymbols &Escaped,
const CallEvent *Call,
PointerEscapeKind Kind) const;
/// If true, evaluate special testing stream functions.
bool TestMode = false;
@ -448,10 +452,14 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
SymbolRef StreamSym = StreamVal->getAsSymbol();
// Do not care about concrete values for stream ("(FILE *)0x12345"?).
// FIXME: Are stdin, stdout, stderr such values?
// FIXME: Can be stdin, stdout, stderr such values?
if (!StreamSym)
return;
// Do not handle untracked stream. It is probably escaped.
if (!State->get<StreamMap>(StreamSym))
return;
// Generate state for non-failed case.
// Return value is the passed stream pointer.
// According to the documentations, the stream is closed first
@ -918,6 +926,28 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
}
}
ProgramStateRef StreamChecker::checkPointerEscape(
ProgramStateRef State, const InvalidatedSymbols &Escaped,
const CallEvent *Call, PointerEscapeKind Kind) const {
// Check for file-handling system call that is not handled by the checker.
// FIXME: The checker should be updated to handle all system calls that take
// 'FILE*' argument. These are now ignored.
if (Kind == PSK_DirectEscapeOnCall && Call->isInSystemHeader())
return State;
for (SymbolRef Sym : Escaped) {
// The symbol escaped.
// From now the stream can be manipulated in unknown way to the checker,
// it is not possible to handle it any more.
// Optimistically, assume that the corresponding file handle will be closed
// somewhere else.
// Remove symbol from state so the following stream calls on this symbol are
// not handled by the checker.
State = State->remove<StreamMap>(Sym);
}
return State;
}
void ento::registerStreamChecker(CheckerManager &Mgr) {
Mgr.registerChecker<StreamChecker>();
}

View File

@ -192,4 +192,51 @@ void check_freopen_3() {
rewind(f1); // expected-warning {{Stream might be invalid}}
fclose(f1);
}
}
}
extern FILE *GlobalF;
extern void takeFile(FILE *);
void check_escape1() {
FILE *F = tmpfile();
if (!F)
return;
fwrite("1", 1, 1, F); // may fail
GlobalF = F;
fwrite("1", 1, 1, F); // no warning
}
void check_escape2() {
FILE *F = tmpfile();
if (!F)
return;
fwrite("1", 1, 1, F); // may fail
takeFile(F);
fwrite("1", 1, 1, F); // no warning
}
void check_escape3() {
FILE *F = tmpfile();
if (!F)
return;
takeFile(F);
F = freopen(0, "w", F);
if (!F)
return;
fwrite("1", 1, 1, F); // may fail
fwrite("1", 1, 1, F); // no warning
}
void check_escape4() {
FILE *F = tmpfile();
if (!F)
return;
fwrite("1", 1, 1, F); // may fail
// no escape at (non-StreamChecker-handled) system call
// FIXME: all such calls should be handled by the checker
fprintf(F, "0");
fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}}
fclose(F);
}