diff --git a/clang/lib/StaticAnalyzer/Checkers/ASTLookup.h b/clang/lib/StaticAnalyzer/Checkers/ASTLookup.h new file mode 100644 --- /dev/null +++ b/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 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 operator()(Optional Ty); +}; + +/// Construct a pointer to a type. +class GetPointerTy { + const ASTContext &ACtx; + +public: + GetPointerTy(const ASTContext &ACtx) : ACtx(ACtx) {} + + QualType operator()(QualType Ty); + Optional operator()(Optional Ty); +}; + +/// Construct a const version of a type. +class GetConstTy { +public: + Optional operator()(Optional Ty); + QualType operator()(QualType Ty); +}; + +/// Get the maximum possible value of a type. +class GetMaxValue { + BasicValueFactory &BVF; + +public: + GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {} + + Optional operator()(QualType Ty); + Optional operator()(Optional Ty); +}; + +} // namespace ast_lookup +} // namespace ento +} // namespace clang + +#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ASTLOOKUP_H diff --git a/clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp b/clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp new file mode 100644 --- /dev/null +++ b/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 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(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(D)) + return ACtx.getTypeDeclType(TD).getCanonicalType(); + return None; +} + +QualType GetRestrictTy::operator()(QualType Ty) { + return ACtx.getLangOpts().C99 ? ACtx.getRestrictType(Ty) : Ty; +} + +Optional GetRestrictTy::operator()(Optional Ty) { + if (Ty) + return operator()(*Ty); + return None; +} + +QualType GetPointerTy::operator()(QualType Ty) { + return ACtx.getPointerType(Ty); +} + +Optional GetPointerTy::operator()(Optional Ty) { + if (Ty) + return operator()(*Ty); + return None; +} + +Optional GetConstTy::operator()(Optional Ty) { + return Ty ? Optional(Ty->withConst()) : None; +} + +QualType GetConstTy::operator()(QualType Ty) { return Ty.withConst(); } + +Optional GetMaxValue::operator()(QualType Ty) { + return BVF.getMaxValue(Ty).getLimitedValue(); +} +Optional GetMaxValue::operator()(Optional Ty) { + if (Ty) { + return operator()(*Ty); + } + return None; +} + +} // namespace ast_lookup +} // namespace ento +} // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -8,6 +8,7 @@ AnalyzerStatsChecker.cpp ArrayBoundChecker.cpp ArrayBoundCheckerV2.cpp + ASTLookup.cpp BasicObjCFoundationChecks.cpp BlockInCriticalSectionChecker.cpp BoolAssignmentChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/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 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(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(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 operator()(Optional 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 operator()(Optional Ty) { - if (Ty) - return operator()(*Ty); - return None; - } - } getPointerTy(ACtx); - class { - public: - Optional operator()(Optional Ty) { - return Ty ? Optional(Ty->withConst()) : None; - } - QualType operator()(QualType Ty) { return Ty.withConst(); } - } getConstTy; - class GetMaxValue { - BasicValueFactory &BVF; - - public: - GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {} - Optional operator()(QualType Ty) { - return BVF.getMaxValue(Ty).getLimitedValue(); - } - Optional operator()(Optional 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. diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/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; @@ -172,6 +179,23 @@ ArgNoTy StreamArgNo; }; +/// Find symbolic value of a global "standard stream" +/// (stdin, stdout, stderr) variable. +/// The StdDecl value is allowed to be nullptr. +SymbolRef getStdStreamSym(const VarDecl *StdDecl, CheckerContext &C) { + 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(); +} + /// Get the value of the stream argument out of the passed call event. /// The call should contain a function that is described by Desc. SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) { @@ -206,8 +230,9 @@ return State; } -class StreamChecker : public Checker { +class StreamChecker + : public Checker { 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 +246,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 +311,17 @@ 0}}, }; + mutable bool StdStreamsInitialized = false; + mutable Optional FILEType; + // These can be nullptr if not found. + mutable const VarDecl *StdinDecl; + mutable const VarDecl *StdoutDecl; + mutable const VarDecl *StderrDecl; + // These can be nullptr if not found. + mutable SymbolRef StdinSym; + mutable SymbolRef StdoutSym; + mutable SymbolRef StderrSym; + void evalFopen(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -410,20 +447,21 @@ static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N, SymbolRef StreamSym, CheckerContext &C); + + const VarDecl *findStdStreamDecl(StringRef StdName, 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 +483,47 @@ 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(D)) { + if (FILEType && !ACtx.hasSameType(*FILEType, VD->getType())) + continue; + return VD; + } + } + + return nullptr; +} + +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); + } + + StdinSym = getStdStreamSym(StdinDecl, C); + StdoutSym = getStdStreamSym(StdoutDecl, C); + StderrSym = getStdStreamSym(StderrDecl, C); +} void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { @@ -494,6 +570,21 @@ StateNull = StateNull->set(RetSym, StreamState::getOpenFailed(Desc)); + SValBuilder &SVB = C.getSValBuilder(); + auto AssumeRetValNotEq = [&SVB, RetVal](SymbolRef StdSym, + ProgramStateRef State) { + if (!StdSym) + return State; + SVal StdVal = SVB.makeSymbolVal(StdSym); + SVal NotEqS = + SVB.evalBinOp(State, BO_NE, RetVal, StdVal, SVB.getConditionType()); + DefinedOrUnknownSVal NotEqDef = NotEqS.castAs(); + 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})); @@ -1129,4 +1220,4 @@ bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; -} \ No newline at end of file +} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c --- a/clang/test/Analysis/stream.c +++ b/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); +}