diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp index 18e718e0855365..72fd6781a75615 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp @@ -205,7 +205,7 @@ void ErrnoChecker::checkPreCall(const CallEvent &Call, // Probably 'strerror'? if (CallF->isExternC() && CallF->isGlobal() && C.getSourceManager().isInSystemHeader(CallF->getLocation()) && - !isErrno(CallF)) { + !isErrnoLocationCall(Call)) { if (getErrnoState(C.getState()) == MustBeChecked) { std::optional ErrnoLoc = getErrnoLoc(C.getState()); assert(ErrnoLoc && "ErrnoLoc should exist if an errno state is set."); diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp index 1b34ea0e056e56..6ffc05f06742bd 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp @@ -39,10 +39,15 @@ namespace { // Name of the "errno" variable. // FIXME: Is there a system where it is not called "errno" but is a variable? const char *ErrnoVarName = "errno"; + // Names of functions that return a location of the "errno" value. // FIXME: Are there other similar function names? -const char *ErrnoLocationFuncNames[] = {"__errno_location", "___errno", - "__errno", "_errno", "__error"}; +CallDescriptionSet ErrnoLocationCalls{ + {CDM::CLibrary, {"__errno_location"}, 0, 0}, + {CDM::CLibrary, {"___errno"}, 0, 0}, + {CDM::CLibrary, {"__errno"}, 0, 0}, + {CDM::CLibrary, {"_errno"}, 0, 0}, + {CDM::CLibrary, {"__error"}, 0, 0}}; class ErrnoModeling : public Checker, check::BeginFunction, @@ -54,16 +59,10 @@ class ErrnoModeling void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const; bool evalCall(const CallEvent &Call, CheckerContext &C) const; - // The declaration of an "errno" variable or "errno location" function. - mutable const Decl *ErrnoDecl = nullptr; - private: - // FIXME: Names from `ErrnoLocationFuncNames` are used to build this set. - CallDescriptionSet ErrnoLocationCalls{{{"__errno_location"}, 0, 0}, - {{"___errno"}, 0, 0}, - {{"__errno"}, 0, 0}, - {{"_errno"}, 0, 0}, - {{"__error"}, 0, 0}}; + // The declaration of an "errno" variable on systems where errno is + // represented by a variable (and not a function that queries its location). + mutable const VarDecl *ErrnoDecl = nullptr; }; } // namespace @@ -74,9 +73,13 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoRegion, const MemRegion *) REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrnoState, errno_modeling::ErrnoCheckState) -/// Search for a variable called "errno" in the AST. -/// Return nullptr if not found. -static const VarDecl *getErrnoVar(ASTContext &ACtx) { +void ErrnoModeling::checkASTDecl(const TranslationUnitDecl *D, + AnalysisManager &Mgr, BugReporter &BR) const { + // Try to find the declaration of the external variable `int errno;`. + // There are also C library implementations, where the `errno` location is + // accessed via a function that returns its address; in those environments + // this callback has no effect. + ASTContext &ACtx = Mgr.getASTContext(); IdentifierInfo &II = ACtx.Idents.get(ErrnoVarName); auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II); auto Found = llvm::find_if(LookupRes, [&ACtx](const Decl *D) { @@ -86,47 +89,8 @@ static const VarDecl *getErrnoVar(ASTContext &ACtx) { VD->getType().getCanonicalType() == ACtx.IntTy; return false; }); - if (Found == LookupRes.end()) - return nullptr; - - return cast(*Found); -} - -/// Search for a function with a specific name that is used to return a pointer -/// to "errno". -/// Return nullptr if no such function was found. -static const FunctionDecl *getErrnoFunc(ASTContext &ACtx) { - SmallVector LookupRes; - for (StringRef ErrnoName : ErrnoLocationFuncNames) { - IdentifierInfo &II = ACtx.Idents.get(ErrnoName); - llvm::append_range(LookupRes, ACtx.getTranslationUnitDecl()->lookup(&II)); - } - - auto Found = llvm::find_if(LookupRes, [&ACtx](const Decl *D) { - if (auto *FD = dyn_cast(D)) - return ACtx.getSourceManager().isInSystemHeader(FD->getLocation()) && - FD->isExternC() && FD->getNumParams() == 0 && - FD->getReturnType().getCanonicalType() == - ACtx.getPointerType(ACtx.IntTy); - return false; - }); - if (Found == LookupRes.end()) - return nullptr; - - return cast(*Found); -} - -void ErrnoModeling::checkASTDecl(const TranslationUnitDecl *D, - AnalysisManager &Mgr, BugReporter &BR) const { - // Try to find an usable `errno` value. - // It can be an external variable called "errno" or a function that returns a - // pointer to the "errno" value. This function can have different names. - // The actual case is dependent on the C library implementation, we - // can only search for a match in one of these variations. - // We assume that exactly one of these cases might be true. - ErrnoDecl = getErrnoVar(Mgr.getASTContext()); - if (!ErrnoDecl) - ErrnoDecl = getErrnoFunc(Mgr.getASTContext()); + if (Found != LookupRes.end()) + ErrnoDecl = cast(*Found); } void ErrnoModeling::checkBeginFunction(CheckerContext &C) const { @@ -136,25 +100,18 @@ void ErrnoModeling::checkBeginFunction(CheckerContext &C) const { ASTContext &ACtx = C.getASTContext(); ProgramStateRef State = C.getState(); - if (const auto *ErrnoVar = dyn_cast_or_null(ErrnoDecl)) { - // There is an external 'errno' variable. - // Use its memory region. - // The memory region for an 'errno'-like variable is allocated in system - // space by MemRegionManager. - const MemRegion *ErrnoR = - State->getRegion(ErrnoVar, C.getLocationContext()); + const MemRegion *ErrnoR = nullptr; + + if (ErrnoDecl) { + // There is an external 'errno' variable, so we can simply use the memory + // region that's associated with it. + ErrnoR = State->getRegion(ErrnoDecl, C.getLocationContext()); assert(ErrnoR && "Memory region should exist for the 'errno' variable."); - State = State->set(ErrnoR); - State = - errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant); - C.addTransition(State); - } else if (ErrnoDecl) { - assert(isa(ErrnoDecl) && "Invalid errno location function."); - // There is a function that returns the location of 'errno'. - // We must create a memory region for it in system space. - // Currently a symbolic region is used with an artifical symbol. - // FIXME: It is better to have a custom (new) kind of MemRegion for such - // cases. + } else { + // There is no 'errno' variable, so create a new symbolic memory region + // that can be used to model the return value of the "get the location of + // errno" internal functions. + // NOTE: this `SVal` is created even if errno is not defined or used. SValBuilder &SVB = C.getSValBuilder(); MemRegionManager &RMgr = C.getStateManager().getRegionManager(); @@ -162,27 +119,31 @@ void ErrnoModeling::checkBeginFunction(CheckerContext &C) const { RMgr.getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind); // Create an artifical symbol for the region. - // It is not possible to associate a statement or expression in this case. + // Note that it is not possible to associate a statement or expression in + // this case and the `symbolTag` (opaque pointer tag) is just the address + // of the data member `ErrnoDecl` of the singleton `ErrnoModeling` checker + // object. const SymbolConjured *Sym = SVB.conjureSymbol( nullptr, C.getLocationContext(), ACtx.getLValueReferenceType(ACtx.IntTy), C.blockCount(), &ErrnoDecl); // The symbolic region is untyped, create a typed sub-region in it. // The ElementRegion is used to make the errno region a typed region. - const MemRegion *ErrnoR = RMgr.getElementRegion( + ErrnoR = RMgr.getElementRegion( ACtx.IntTy, SVB.makeZeroArrayIndex(), RMgr.getSymbolicRegion(Sym, GlobalSystemSpace), C.getASTContext()); - State = State->set(ErrnoR); - State = - errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant); - C.addTransition(State); } + assert(ErrnoR); + State = State->set(ErrnoR); + State = + errno_modeling::setErrnoValue(State, C, 0, errno_modeling::Irrelevant); + C.addTransition(State); } bool ErrnoModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { // Return location of "errno" at a call to an "errno address returning" // function. - if (ErrnoLocationCalls.contains(Call)) { + if (errno_modeling::isErrnoLocationCall(Call)) { ProgramStateRef State = C.getState(); const MemRegion *ErrnoR = State->get(); @@ -260,14 +221,8 @@ ProgramStateRef clearErrnoState(ProgramStateRef State) { return setErrnoState(State, Irrelevant); } -bool isErrno(const Decl *D) { - if (const auto *VD = dyn_cast_or_null(D)) - if (const IdentifierInfo *II = VD->getIdentifier()) - return II->getName() == ErrnoVarName; - if (const auto *FD = dyn_cast_or_null(D)) - if (const IdentifierInfo *II = FD->getIdentifier()) - return llvm::is_contained(ErrnoLocationFuncNames, II->getName()); - return false; +bool isErrnoLocationCall(const CallEvent &CE) { + return ErrnoLocationCalls.contains(CE); } const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) { diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h index 6b53572fe5e2d9..95da8a28d32539 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h @@ -71,12 +71,9 @@ ProgramStateRef setErrnoState(ProgramStateRef State, ErrnoCheckState EState); /// Clear state of errno (make it irrelevant). ProgramStateRef clearErrnoState(ProgramStateRef State); -/// Determine if a `Decl` node related to 'errno'. -/// This is true if the declaration is the errno variable or a function -/// that returns a pointer to the 'errno' value (usually the 'errno' macro is -/// defined with this function). \p D is not required to be a canonical -/// declaration. -bool isErrno(const Decl *D); +/// Determine if `Call` is a call to an internal function that returns the +/// location of `errno` (in environments where errno is accessed this way). +bool isErrnoLocationCall(const CallEvent &Call); /// Create a NoteTag that displays the message if the 'errno' memory region is /// marked as interesting, and resets the interestingness. diff --git a/clang/test/Analysis/memory-model.cpp b/clang/test/Analysis/memory-model.cpp index fd5a286acb60c0..cd42e8c72b8bdd 100644 --- a/clang/test/Analysis/memory-model.cpp +++ b/clang/test/Analysis/memory-model.cpp @@ -34,9 +34,9 @@ void var_simple_ref() { } void var_simple_ptr(int *a) { - clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$0}}} - clang_analyzer_dumpExtent(a); // expected-warning {{extent_$1{SymRegion{reg_$0}}}} - clang_analyzer_dumpElementCount(a); // expected-warning {{(extent_$1{SymRegion{reg_$0}}) / 4}} + clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$1}}} + clang_analyzer_dumpExtent(a); // expected-warning {{extent_$2{SymRegion{reg_$1}}}} + clang_analyzer_dumpElementCount(a); // expected-warning {{(extent_$2{SymRegion{reg_$1}}) / 4}} } void var_array() { @@ -53,9 +53,9 @@ void string() { } void struct_simple_ptr(S *a) { - clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$0}}} - clang_analyzer_dumpExtent(a); // expected-warning {{extent_$1{SymRegion{reg_$0}}}} - clang_analyzer_dumpElementCount(a); // expected-warning {{(extent_$1{SymRegion{reg_$0}}) / 4}} + clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$1}}} + clang_analyzer_dumpExtent(a); // expected-warning {{extent_$2{SymRegion{reg_$1}}}} + clang_analyzer_dumpElementCount(a); // expected-warning {{(extent_$2{SymRegion{reg_$1}}) / 4}} } void field_ref(S a) { @@ -65,9 +65,9 @@ void field_ref(S a) { } void field_ptr(S *a) { - clang_analyzer_dump(&a->f); // expected-warning {{Element{SymRegion{reg_$0},0 S64b,struct S}.f}} - clang_analyzer_dumpExtent(&a->f); // expected-warning {{extent_$1{SymRegion{reg_$0}}}} - clang_analyzer_dumpElementCount(&a->f); // expected-warning {{(extent_$1{SymRegion{reg_$0}}) / 4U}} + clang_analyzer_dump(&a->f); // expected-warning {{Element{SymRegion{reg_$1},0 S64b,struct S}.f}} + clang_analyzer_dumpExtent(&a->f); // expected-warning {{extent_$2{SymRegion{reg_$1}}}} + clang_analyzer_dumpElementCount(&a->f); // expected-warning {{(extent_$2{SymRegion{reg_$1}}) / 4U}} } void symbolic_array() {