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.
Update the existing modeling of stream functions in StreamChecker
with 'errno' modeling. The not (yet) evaluated stream functions
are not updated.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132017
Files:
clang/docs/ReleaseNotes.rst
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-errno.c
Index: clang/test/Analysis/stream-errno.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/stream-errno.c
@@ -0,0 +1,196 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,alpha.unix.Errno,debug.ExprInspection -analyzer-output text -verify %s
+
+#include "Inputs/system-header-simulator.h"
+#include "Inputs/errno_func.h"
+
+extern void clang_analyzer_eval(int);
+extern void clang_analyzer_dump(int);
+
+void check_fopen(void) {
+ FILE *F = fopen("xxx", "r");
+ // expected-note@-1{{Assuming that function 'fopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+ // expected-note@+4{{'F' is null}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_tmpfile(void) {
+ FILE *F = tmpfile();
+ // expected-note@-1{{Assuming that function 'tmpfile' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+ // expected-note@+4{{'F' is null}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_freopen(void) {
+ FILE *F = tmpfile();
+ // expected-note@+4{{'F' is non-null}}
+ // expected-note@+3{{Taking false branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ F = freopen("xxx", "w", F);
+ // expected-note@-1{{Assuming that function 'freopen' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+ // expected-note@+4{{'F' is null}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fclose(void) {
+ FILE *F = tmpfile();
+ // expected-note@+4{{'F' is non-null}}
+ // expected-note@+3{{Taking false branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ int Ret = fclose(F);
+ // expected-note@-1{{Assuming that function 'fclose' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+ // expected-note@+2{{Taking true branch}}
+ // expected-note@+1{{Taking false branch}}
+ if (Ret == EOF) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fread(void) {
+ char Buf[10];
+ FILE *F = tmpfile();
+ // expected-note@+4{{'F' is non-null}}
+ // expected-note@+3{{Taking false branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ fread(Buf, 0, 1, F);
+ // expected-note@+2{{Taking false branch}}
+ // expected-note@+1{{Taking false branch}}
+ if (errno) {} // no-warning
+ fread(Buf, 1, 0, F);
+ // expected-note@+2{{Taking false branch}}
+ // expected-note@+1{{Taking false branch}}
+ if (errno) {} // no-warning
+
+ int R = fread(Buf, 1, 10, F);
+ // expected-note@-1{{function 'fread' is successful}}
+ // expected-note@+4{{'R' is < 10}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'R' is >= 10}}
+ // expected-note@+1{{Taking false branch}}
+ if (R < 10) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ fclose(F);
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fwrite(void) {
+ char Buf[] = "0123456789";
+ FILE *F = tmpfile();
+ // expected-note@+4{{'F' is non-null}}
+ // expected-note@+3{{Taking false branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ fwrite(Buf, 0, 1, F);
+ // expected-note@+2{{Taking false branch}}
+ // expected-note@+1{{Taking false branch}}
+ if (errno) {} // no-warning
+ fwrite(Buf, 1, 0, F);
+ // expected-note@+2{{Taking false branch}}
+ // expected-note@+1{{Taking false branch}}
+ if (errno) {} // no-warning
+
+ int R = fwrite(Buf, 1, 10, F);
+ // expected-note@-1{{function 'fwrite' is successful}}
+ // expected-note@+4{{'R' is < 10}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'R' is >= 10}}
+ // expected-note@+1{{Taking false branch}}
+ if (R < 10) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ fclose(F);
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_fseek(void) {
+ FILE *F = tmpfile();
+ // expected-note@+4{{'F' is non-null}}
+ // expected-note@+3{{Taking false branch}}
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ int S = fseek(F, 11, SEEK_SET);
+ // expected-note@-1{{Assuming that function 'fseek' is successful}}
+ // expected-note@+4{{'S' is not equal to 0}}
+ // expected-note@+3{{Taking true branch}}
+ // expected-note@+2{{'S' is equal to 0}}
+ // expected-note@+1{{Taking false branch}}
+ if (S != 0) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ if (errno) {}
+ fclose(F);
+ return;
+ }
+ if (errno) {} // expected-warning{{An undefined value may be read from 'errno'}}
+ // expected-note@-1{{An undefined value may be read from 'errno'}}
+}
+
+void check_no_errno_change(void) {
+ FILE *F = tmpfile();
+ // expected-note@+2{{'F' is non-null}}
+ // expected-note@+1{{Taking false branch}}
+ if (!F)
+ return;
+ errno = 1;
+ clearerr(F);
+ // expected-note@+1{{Taking true branch}}
+ if (errno) {} // no-warning
+ feof(F);
+ // expected-note@+1{{Taking true branch}}
+ if (errno) {} // no-warning
+ ferror(F);
+ // expected-note@+1{{Taking true branch}}
+ if (errno) {} // no-warning
+ clang_analyzer_eval(errno == 1); // expected-warning{{TRUE}} expected-note{{TRUE}}
+ fclose(F);
+}
+
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "ErrnoModeling.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -17,6 +18,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -278,6 +280,9 @@
0}},
};
+ mutable bool EofInitialized = false;
+ mutable int EofVal = -1;
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -411,6 +416,25 @@
});
}
+ /// Get a SVal to be used as new errno value at failure cases of stream
+ /// functions.
+ NonLoc getErrnoSVal(CheckerContext &C, const CallEvent &Call) const {
+ return C.getSValBuilder()
+ .conjureSymbolVal(this, Call.getOriginExpr(), C.getLocationContext(),
+ C.getASTContext().IntTy, C.blockCount())
+ .castAs<NonLoc>();
+ }
+
+ void initEof(CheckerContext &C) const {
+ if (EofInitialized)
+ return;
+
+ if (const llvm::Optional<int> OptInt =
+ tryExpandAsInteger("EOF", C.getPreprocessor()))
+ EofVal = *OptInt;
+ EofInitialized = true;
+ }
+
/// Searches for the ExplodedNode where the file descriptor was acquired for
/// StreamSym.
static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
@@ -457,6 +481,8 @@
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
+ initEof(C);
+
const FnDescription *Desc = lookupFn(Call);
if (!Desc || !Desc->PreFn)
return;
@@ -497,11 +523,18 @@
StateNotNull =
StateNotNull->set<StreamMap>(RetSym, StreamState::getOpened(Desc));
+ StateNotNull = errno_modeling::setErrnoForStdSuccess(StateNotNull, C);
+
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
-
- C.addTransition(StateNotNull,
- constructNoteTag(C, RetSym, "Stream opened here"));
+ StateNull = errno_modeling::setErrnoForStdFailure(StateNull, C,
+ getErrnoSVal(C, Call));
+
+ ExplodedNode *N = C.addTransition(
+ StateNotNull, constructNoteTag(C, RetSym, "Stream opened here"));
+ C.addTransition(N->getState(), N,
+ errno_modeling::getNoteTagForStdSuccess(
+ C, cast<NamedDecl>(Call.getDecl())->getNameAsString()));
C.addTransition(StateNull);
}
@@ -555,11 +588,17 @@
StateRetNotNull =
StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ StateRetNotNull = errno_modeling::setErrnoForStdSuccess(StateRetNotNull, C);
+
StateRetNull =
StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
+ StateRetNull = errno_modeling::setErrnoForStdFailure(StateRetNull, C,
+ getErrnoSVal(C, Call));
- C.addTransition(StateRetNotNull,
- constructNoteTag(C, StreamSym, "Stream reopened here"));
+ ExplodedNode *N = C.addTransition(
+ StateRetNotNull, constructNoteTag(C, StreamSym, "Stream reopened here"));
+ C.addTransition(N->getState(), N,
+ errno_modeling::getNoteTagForStdSuccess(C, "freopen"));
C.addTransition(StateRetNull);
}
@@ -574,6 +613,10 @@
if (!SS)
return;
+ auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return;
+
assertStreamStateOpened(SS);
// Close the File Descriptor.
@@ -581,7 +624,21 @@
// and can not be used any more.
State = State->set<StreamMap>(Sym, StreamState::getClosed(Desc));
- C.addTransition(State);
+ // Return 0 on success, EOF on failure.
+ SValBuilder &SVB = C.getSValBuilder();
+ ProgramStateRef StateSuccess = State->BindExpr(
+ CE, C.getLocationContext(), SVB.makeIntVal(0, C.getASTContext().IntTy));
+ ProgramStateRef StateFailure =
+ State->BindExpr(CE, C.getLocationContext(),
+ SVB.makeIntVal(EofVal, C.getASTContext().IntTy));
+
+ StateSuccess = errno_modeling::setErrnoForStdSuccess(StateSuccess, C);
+ StateFailure = errno_modeling::setErrnoForStdFailure(StateFailure, C,
+ getErrnoSVal(C, Call));
+
+ C.addTransition(StateSuccess,
+ errno_modeling::getNoteTagForStdSuccess(C, "fclose"));
+ C.addTransition(StateFailure);
}
void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
@@ -663,6 +720,8 @@
// This is the "size or nmemb is zero" case.
// Just return 0, do nothing more (not clear the error flags).
State = bindInt(0, State, C, CE);
+ // It is unspecified what can happen with 'errno'.
+ State = errno_modeling::setErrnoState(State, errno_modeling::Irrelevant);
C.addTransition(State);
return;
}
@@ -674,7 +733,10 @@
State->BindExpr(CE, C.getLocationContext(), *NMembVal);
StateNotFailed =
StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
- C.addTransition(StateNotFailed);
+ StateNotFailed = errno_modeling::setErrnoForStdSuccess(StateNotFailed, C);
+ C.addTransition(StateNotFailed,
+ errno_modeling::getNoteTagForStdSuccess(
+ C, cast<NamedDecl>(Call.getDecl())->getNameAsString()));
}
// Add transition for the failed state.
@@ -701,6 +763,8 @@
// indicator for the stream is indeterminate.
StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof());
StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
+ StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+ getErrnoSVal(C, Call));
if (IsFread && OldSS->ErrorState != ErrorFEof)
C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
else
@@ -762,7 +826,12 @@
StreamSym,
StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true));
- C.addTransition(StateNotFailed);
+ StateNotFailed = errno_modeling::setErrnoForStdSuccess(StateNotFailed, C);
+ StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,
+ getErrnoSVal(C, Call));
+
+ C.addTransition(StateNotFailed,
+ errno_modeling::getNoteTagForStdSuccess(C, "fseek"));
C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym));
}
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -207,6 +207,10 @@
Static Analyzer
---------------
+- Checker ``alpha.unix.Stream`` is improved to be able to detect bugs related
+ to bad ``errno`` usage. The effects are visible if checker
+ ``alpha.unix.Errno`` is turned on additionally.
+
.. _release-notes-ubsan:
Undefined Behavior Sanitizer (UBSan)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
