balazske updated this revision to Diff 364787.
balazske added a comment.
Init std declarations and FILE type only once.
Make a lambda not operate on captured data.
Simplify test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106644/new/
https://reviews.llvm.org/D106644
Files:
clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp
clang/lib/StaticAnalyzer/Checkers/ASTLookup.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
#include "Inputs/system-header-simulator.h"
+void clang_analyzer_eval(int);
+
void check_fread() {
FILE *fp = tmpfile();
fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
@@ -267,3 +269,23 @@
} // expected-warning {{Opened stream never closed. Potential resource leak}}
// FIXME: This warning should be placed at the `return` above.
// See https://reviews.llvm.org/D83120 about details.
+
+void check_fopen_is_not_std() {
+ FILE *F = fopen("file", "r");
+ if (!F)
+ return;
+ clang_analyzer_eval(F != stdin); // expected-warning{{TRUE}}
+ clang_analyzer_eval(F != stdout); // expected-warning{{TRUE}}
+ clang_analyzer_eval(F != stderr); // expected-warning{{TRUE}}
+ fclose(F);
+}
+
+void check_tmpfile_is_not_std() {
+ FILE *F = tmpfile();
+ if (!F)
+ return;
+ clang_analyzer_eval(F != stdin); // expected-warning{{TRUE}}
+ clang_analyzer_eval(F != stdout); // expected-warning{{TRUE}}
+ clang_analyzer_eval(F != stderr); // expected-warning{{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,8 @@
//
//===----------------------------------------------------------------------===//
+#include "ASTLookup.h"
+#include "clang/AST/ParentMapContext.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -157,6 +159,11 @@
// StreamChecker class and utility functions.
//===----------------------------------------------------------------------===//
+// This map holds the state of a stream.
+// The stream is identified with a SymbolRef that is created when a stream
+// opening function is modeled by the checker.
+REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+
namespace {
class StreamChecker;
@@ -206,8 +213,9 @@
return State;
}
-class StreamChecker : public Checker<check::PreCall, eval::Call,
- check::DeadSymbols, check::PointerEscape> {
+class StreamChecker
+ : public Checker<check::BeginFunction, check::PreCall, eval::Call,
+ check::DeadSymbols, check::PointerEscape> {
BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
BugType BT_UseAfterOpenFailed{this, "Invalid stream",
@@ -221,6 +229,7 @@
/*SuppressOnSink =*/true};
public:
+ void checkBeginFunction(CheckerContext &C) const;
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -285,6 +294,15 @@
0}},
};
+ mutable bool StdStreamsInitialized = false;
+ mutable Optional<QualType> FILEType;
+ mutable const VarDecl *StdinDecl;
+ mutable const VarDecl *StdoutDecl;
+ mutable const VarDecl *StderrDecl;
+ mutable SymbolRef StdinSym;
+ mutable SymbolRef StdoutSym;
+ mutable SymbolRef StderrSym;
+
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
CheckerContext &C) const;
@@ -410,20 +428,23 @@
static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
SymbolRef StreamSym,
CheckerContext &C);
+
+ const VarDecl *findStdStreamDecl(StringRef StdName, CheckerContext &C) const;
+
+ SymbolRef getStdStreamSym(const VarDecl *StdDecl, CheckerContext &C) const;
};
} // end anonymous namespace
-// This map holds the state of a stream.
-// The stream is identified with a SymbolRef that is created when a stream
-// opening function is modeled by the checker.
-REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
-
inline void assertStreamStateOpened(const StreamState *SS) {
assert(SS->isOpened() &&
"Previous create of error node for non-opened stream failed?");
}
+//===----------------------------------------------------------------------===//
+// Methods of StreamChecker.
+//===----------------------------------------------------------------------===//
+
const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N,
SymbolRef StreamSym,
CheckerContext &C) {
@@ -445,9 +466,63 @@
return nullptr;
}
-//===----------------------------------------------------------------------===//
-// Methods of StreamChecker.
-//===----------------------------------------------------------------------===//
+const VarDecl *StreamChecker::findStdStreamDecl(StringRef StdName,
+ CheckerContext &C) const {
+ ASTContext &ACtx = C.getASTContext();
+
+ IdentifierInfo &II = ACtx.Idents.get(StdName);
+ auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
+
+ for (Decl *D : LookupRes) {
+ D = D->getCanonicalDecl();
+ if (!C.getSourceManager().isInSystemHeader(D->getLocation()))
+ continue;
+ if (auto *VD = dyn_cast<VarDecl>(D)) {
+ if (FILEType && !ACtx.hasSameType(*FILEType, VD->getType()))
+ continue;
+ return VD;
+ }
+ }
+
+ return nullptr;
+}
+
+SymbolRef StreamChecker::getStdStreamSym(const VarDecl *StdDecl,
+ CheckerContext &C) const {
+ if (!StdDecl)
+ return nullptr;
+
+ const LocationContext *LCtx = C.getLocationContext();
+
+ const VarRegion *RegionOfStdStreamVar =
+ C.getState()->getRegion(StdDecl, LCtx);
+ if (!RegionOfStdStreamVar)
+ return nullptr;
+ SVal StdVal = C.getState()->getSVal(RegionOfStdStreamVar, StdDecl->getType());
+ return StdVal.getAsSymbol();
+}
+
+void StreamChecker::checkBeginFunction(CheckerContext &C) const {
+ if (!C.inTopFrame())
+ return;
+
+ if (!StdStreamsInitialized) {
+ StdStreamsInitialized = true;
+
+ ast_lookup::LookupType LookupType(C.getASTContext());
+ ast_lookup::GetPointerTy GetPointer(C.getASTContext());
+ FILEType = GetPointer(LookupType("FILE"));
+
+ StdinDecl = findStdStreamDecl("stdin", C);
+ StdoutDecl = findStdStreamDecl("stdout", C);
+ StderrDecl = findStdStreamDecl("stderr", C);
+ }
+
+ // These can be nullptr if not found.
+ StdinSym = getStdStreamSym(StdinDecl, C);
+ StdoutSym = getStdStreamSym(StdoutDecl, C);
+ StderrSym = getStdStreamSym(StderrDecl, C);
+}
void StreamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
@@ -494,6 +569,23 @@
StateNull =
StateNull->set<StreamMap>(RetSym, StreamState::getOpenFailed(Desc));
+ auto AssumeRetValNotEq = [&C, RetVal](SymbolRef StdStream,
+ ProgramStateRef State) {
+ if (!StdStream)
+ return State;
+ SVal NotEqS = C.getSValBuilder().evalBinOp(
+ State, BO_NE, RetVal, C.getSValBuilder().makeSymbolVal(StdStream),
+ C.getASTContext().getLogicalOperationType());
+ Optional<DefinedOrUnknownSVal> NotEqDef =
+ NotEqS.getAs<DefinedOrUnknownSVal>();
+ if (!NotEqDef)
+ return State;
+ return State->assume(*NotEqDef, true);
+ };
+ StateNotNull = AssumeRetValNotEq(StdinSym, StateNotNull);
+ StateNotNull = AssumeRetValNotEq(StdoutSym, StateNotNull);
+ StateNotNull = AssumeRetValNotEq(StderrSym, StateNotNull);
+
C.addTransition(StateNotNull,
constructNoteTag(C, RetSym, {&BT_ResourceLeak}));
C.addTransition(StateNull, constructNoteTag(C, RetSym, {&BT_FileNull}));
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -49,6 +49,7 @@
//
//===----------------------------------------------------------------------===//
+#include "ASTLookup.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -955,90 +956,11 @@
BasicValueFactory &BVF = SVB.getBasicValueFactory();
const ASTContext &ACtx = BVF.getContext();
- // Helper class to lookup a type by its name.
- class LookupType {
- const ASTContext &ACtx;
-
- public:
- LookupType(const ASTContext &ACtx) : ACtx(ACtx) {}
-
- // Find the type. If not found then the optional is not set.
- llvm::Optional<QualType> operator()(StringRef Name) {
- IdentifierInfo &II = ACtx.Idents.get(Name);
- auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
- if (LookupRes.empty())
- return None;
-
- // Prioritze typedef declarations.
- // This is needed in case of C struct typedefs. E.g.:
- // typedef struct FILE FILE;
- // In this case, we have a RecordDecl 'struct FILE' with the name 'FILE'
- // and we have a TypedefDecl with the name 'FILE'.
- for (Decl *D : LookupRes)
- if (auto *TD = dyn_cast<TypedefNameDecl>(D))
- return ACtx.getTypeDeclType(TD).getCanonicalType();
-
- // Find the first TypeDecl.
- // There maybe cases when a function has the same name as a struct.
- // E.g. in POSIX: `struct stat` and the function `stat()`:
- // int stat(const char *restrict path, struct stat *restrict buf);
- for (Decl *D : LookupRes)
- if (auto *TD = dyn_cast<TypeDecl>(D))
- return ACtx.getTypeDeclType(TD).getCanonicalType();
- return None;
- }
- } lookupTy(ACtx);
-
- // Below are auxiliary classes to handle optional types that we get as a
- // result of the lookup.
- class GetRestrictTy {
- const ASTContext &ACtx;
-
- public:
- GetRestrictTy(const ASTContext &ACtx) : ACtx(ACtx) {}
- QualType operator()(QualType Ty) {
- return ACtx.getLangOpts().C99 ? ACtx.getRestrictType(Ty) : Ty;
- }
- Optional<QualType> operator()(Optional<QualType> Ty) {
- if (Ty)
- return operator()(*Ty);
- return None;
- }
- } getRestrictTy(ACtx);
- class GetPointerTy {
- const ASTContext &ACtx;
-
- public:
- GetPointerTy(const ASTContext &ACtx) : ACtx(ACtx) {}
- QualType operator()(QualType Ty) { return ACtx.getPointerType(Ty); }
- Optional<QualType> operator()(Optional<QualType> Ty) {
- if (Ty)
- return operator()(*Ty);
- return None;
- }
- } getPointerTy(ACtx);
- class {
- public:
- Optional<QualType> operator()(Optional<QualType> Ty) {
- return Ty ? Optional<QualType>(Ty->withConst()) : None;
- }
- QualType operator()(QualType Ty) { return Ty.withConst(); }
- } getConstTy;
- class GetMaxValue {
- BasicValueFactory &BVF;
-
- public:
- GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {}
- Optional<RangeInt> operator()(QualType Ty) {
- return BVF.getMaxValue(Ty).getLimitedValue();
- }
- Optional<RangeInt> operator()(Optional<QualType> Ty) {
- if (Ty) {
- return operator()(*Ty);
- }
- return None;
- }
- } getMaxValue(BVF);
+ ast_lookup::LookupType lookupTy(ACtx);
+ ast_lookup::GetRestrictTy getRestrictTy(ACtx);
+ ast_lookup::GetPointerTy getPointerTy(ACtx);
+ ast_lookup::GetConstTy getConstTy;
+ ast_lookup::GetMaxValue getMaxValue(BVF);
// These types are useful for writing specifications quickly,
// New specifications should probably introduce more types.
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -8,6 +8,7 @@
AnalyzerStatsChecker.cpp
ArrayBoundChecker.cpp
ArrayBoundCheckerV2.cpp
+ ASTLookup.cpp
BasicObjCFoundationChecks.cpp
BlockInCriticalSectionChecker.cpp
BoolAssignmentChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/ASTLookup.h
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ASTLookup.h
@@ -0,0 +1,83 @@
+//=== ASTLookup.h - Lookup and construct objects from AST. ---------*- C++ -*-//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines a simple interface to search for specific objects (types) in a clang
+// AST and compute derived objects (types) from these.
+// This is designed to be useful in checkers that need to lookup big amount of
+// specific types and properties of them.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ASTLOOKUP_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ASTLOOKUP_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
+
+namespace clang {
+namespace ento {
+namespace ast_lookup {
+
+/// Try to find a type with specific name.
+/// If there is a typedef with same name use this instead.
+class LookupType {
+ const ASTContext &ACtx;
+
+public:
+ LookupType(const ASTContext &ACtx) : ACtx(ACtx) {}
+
+ // Find the type. If not found then the optional is not set.
+ llvm::Optional<QualType> operator()(StringRef Name);
+};
+
+/// Construct the "restrict" (C99) version of a type.
+/// If not applicable or empty is passed in, return the same value.
+class GetRestrictTy {
+ const ASTContext &ACtx;
+
+public:
+ GetRestrictTy(const ASTContext &ACtx) : ACtx(ACtx) {}
+
+ QualType operator()(QualType Ty);
+ Optional<QualType> operator()(Optional<QualType> Ty);
+};
+
+/// Construct a pointer to a type.
+class GetPointerTy {
+ const ASTContext &ACtx;
+
+public:
+ GetPointerTy(const ASTContext &ACtx) : ACtx(ACtx) {}
+
+ QualType operator()(QualType Ty);
+ Optional<QualType> operator()(Optional<QualType> Ty);
+};
+
+/// Construct a const version of a type.
+class GetConstTy {
+public:
+ Optional<QualType> operator()(Optional<QualType> Ty);
+ QualType operator()(QualType Ty);
+};
+
+/// Get the maximum possible value of a type.
+class GetMaxValue {
+ BasicValueFactory &BVF;
+
+public:
+ GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {}
+
+ Optional<uint64_t> operator()(QualType Ty);
+ Optional<uint64_t> operator()(Optional<QualType> Ty);
+};
+
+} // namespace ast_lookup
+} // namespace ento
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ASTLOOKUP_H
Index: clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp
@@ -0,0 +1,78 @@
+//== ASTLookup.cpp ----------------------------------------------*- C++ -*--==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTLookup.h"
+
+namespace clang {
+namespace ento {
+namespace ast_lookup {
+
+llvm::Optional<QualType> LookupType::operator()(StringRef Name) {
+ IdentifierInfo &II = ACtx.Idents.get(Name);
+ auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
+ if (LookupRes.empty())
+ return None;
+
+ // Prioritze typedef declarations.
+ // This is needed in case of C struct typedefs. E.g.:
+ // typedef struct FILE FILE;
+ // In this case, we have a RecordDecl 'struct FILE' with the name 'FILE'
+ // and we have a TypedefDecl with the name 'FILE'.
+ for (const Decl *D : LookupRes)
+ if (const auto *TD = dyn_cast<TypedefNameDecl>(D))
+ return ACtx.getTypeDeclType(TD).getCanonicalType();
+
+ // Find the first TypeDecl.
+ // There maybe cases when a function has the same name as a struct.
+ // E.g. in POSIX: `struct stat` and the function `stat()`:
+ // int stat(const char *restrict path, struct stat *restrict buf);
+ for (const Decl *D : LookupRes)
+ if (const auto *TD = dyn_cast<TypeDecl>(D))
+ return ACtx.getTypeDeclType(TD).getCanonicalType();
+ return None;
+}
+
+QualType GetRestrictTy::operator()(QualType Ty) {
+ return ACtx.getLangOpts().C99 ? ACtx.getRestrictType(Ty) : Ty;
+}
+
+Optional<QualType> GetRestrictTy::operator()(Optional<QualType> Ty) {
+ if (Ty)
+ return operator()(*Ty);
+ return None;
+}
+
+QualType GetPointerTy::operator()(QualType Ty) {
+ return ACtx.getPointerType(Ty);
+}
+
+Optional<QualType> GetPointerTy::operator()(Optional<QualType> Ty) {
+ if (Ty)
+ return operator()(*Ty);
+ return None;
+}
+
+Optional<QualType> GetConstTy::operator()(Optional<QualType> Ty) {
+ return Ty ? Optional<QualType>(Ty->withConst()) : None;
+}
+
+QualType GetConstTy::operator()(QualType Ty) { return Ty.withConst(); }
+
+Optional<uint64_t> GetMaxValue::operator()(QualType Ty) {
+ return BVF.getMaxValue(Ty).getLimitedValue();
+}
+Optional<uint64_t> GetMaxValue::operator()(Optional<QualType> Ty) {
+ if (Ty) {
+ return operator()(*Ty);
+ }
+ return None;
+}
+
+} // namespace ast_lookup
+} // namespace ento
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
