Author: Jan Korous Date: 2020-09-22T21:57:24-07:00 New Revision: 47e6851423fd32f0685a643236ad946e23ab14ff
URL: https://github.com/llvm/llvm-project/commit/47e6851423fd32f0685a643236ad946e23ab14ff DIFF: https://github.com/llvm/llvm-project/commit/47e6851423fd32f0685a643236ad946e23ab14ff.diff LOG: [Analyzer][WebKit] Use tri-state types for relevant predicates Some of the predicates can't always be decided - for example when a type definition isn't available. At the same time it's necessary to let client code decide what to do about such cases - specifically we can't just use true or false values as there are callees with conflicting strategies how to handle this. This is a speculative fix for PR47276. Differential Revision: https://reviews.llvm.org/D88133 Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 34c072ac2241..9c7a59971763 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -34,7 +34,9 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) { } if (auto *call = dyn_cast<CallExpr>(E)) { if (auto *memberCall = dyn_cast<CXXMemberCallExpr>(call)) { - if (isGetterOfRefCounted(memberCall->getMethodDecl())) { + Optional<bool> IsGetterOfRefCt = + isGetterOfRefCounted(memberCall->getMethodDecl()); + if (IsGetterOfRefCt && *IsGetterOfRefCt) { E = memberCall->getImplicitObjectArgument(); if (StopAtFirstRefCountedObj) { return {E, true}; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp index 3956db933b35..97f75135bf92 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp @@ -76,8 +76,11 @@ class NoUncountedMemberChecker if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) { // If we don't see the definition we just don't know. - if (MemberCXXRD->hasDefinition() && isRefCountable(MemberCXXRD)) - reportBug(Member, MemberType, MemberCXXRD, RD); + if (MemberCXXRD->hasDefinition()) { + llvm::Optional<bool> isRCAble = isRefCountable(MemberCXXRD); + if (isRCAble && *isRCAble) + reportBug(Member, MemberType, MemberCXXRD, RD); + } } } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 168cfd511170..a198943c9433 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -12,6 +12,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" +#include "llvm/ADT/Optional.h" using llvm::Optional; using namespace clang; @@ -20,6 +21,7 @@ namespace { bool hasPublicRefAndDeref(const CXXRecordDecl *R) { assert(R); + assert(R->hasDefinition()); bool hasRef = false; bool hasDeref = false; @@ -43,25 +45,29 @@ bool hasPublicRefAndDeref(const CXXRecordDecl *R) { namespace clang { -const CXXRecordDecl *isRefCountable(const CXXBaseSpecifier *Base) { +llvm::Optional<const clang::CXXRecordDecl *> +isRefCountable(const CXXBaseSpecifier *Base) { assert(Base); const Type *T = Base->getType().getTypePtrOrNull(); if (!T) - return nullptr; + return llvm::None; const CXXRecordDecl *R = T->getAsCXXRecordDecl(); if (!R) - return nullptr; + return llvm::None; + if (!R->hasDefinition()) + return llvm::None; return hasPublicRefAndDeref(R) ? R : nullptr; } -bool isRefCountable(const CXXRecordDecl *R) { +llvm::Optional<bool> isRefCountable(const CXXRecordDecl *R) { assert(R); R = R->getDefinition(); - assert(R); + if (!R) + return llvm::None; if (hasPublicRefAndDeref(R)) return true; @@ -69,13 +75,24 @@ bool isRefCountable(const CXXRecordDecl *R) { CXXBasePaths Paths; Paths.setOrigin(const_cast<CXXRecordDecl *>(R)); - const auto isRefCountableBase = [](const CXXBaseSpecifier *Base, - CXXBasePath &) { - return clang::isRefCountable(Base); - }; + bool AnyInconclusiveBase = false; + const auto isRefCountableBase = + [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) { + Optional<const clang::CXXRecordDecl *> IsRefCountable = + clang::isRefCountable(Base); + if (!IsRefCountable) { + AnyInconclusiveBase = true; + return false; + } + return (*IsRefCountable) != nullptr; + }; + + bool BasesResult = R->lookupInBases(isRefCountableBase, Paths, + /*LookupInDependent =*/true); + if (AnyInconclusiveBase) + return llvm::None; - return R->lookupInBases(isRefCountableBase, Paths, - /*LookupInDependent =*/true); + return BasesResult; } bool isCtorOfRefCounted(const clang::FunctionDecl *F) { @@ -95,12 +112,19 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) { || FunctionName == "Identifier"; } -bool isUncounted(const CXXRecordDecl *Class) { +llvm::Optional<bool> isUncounted(const CXXRecordDecl *Class) { // Keep isRefCounted first as it's cheaper. - return !isRefCounted(Class) && isRefCountable(Class); + if (isRefCounted(Class)) + return false; + + llvm::Optional<bool> IsRefCountable = isRefCountable(Class); + if (!IsRefCountable) + return llvm::None; + + return (*IsRefCountable); } -bool isUncountedPtr(const Type *T) { +llvm::Optional<bool> isUncountedPtr(const Type *T) { assert(T); if (T->isPointerType() || T->isReferenceType()) { @@ -111,7 +135,7 @@ bool isUncountedPtr(const Type *T) { return false; } -bool isGetterOfRefCounted(const CXXMethodDecl *M) { +Optional<bool> isGetterOfRefCounted(const CXXMethodDecl *M) { assert(M); if (isa<CXXMethodDecl>(M)) { @@ -133,9 +157,7 @@ bool isGetterOfRefCounted(const CXXMethodDecl *M) { if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) { if (auto *targetConversionType = maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) { - if (isUncountedPtr(targetConversionType)) { - return true; - } + return isUncountedPtr(targetConversionType); } } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 83d9c0bcc13b..730a59977175 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -9,6 +9,8 @@ #ifndef LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H #define LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H +#include "llvm/ADT/APInt.h" + namespace clang { class CXXBaseSpecifier; class CXXMethodDecl; @@ -25,30 +27,31 @@ class Type; // Ref<T>. /// \returns CXXRecordDecl of the base if the type is ref-countable, nullptr if -/// not. -const clang::CXXRecordDecl *isRefCountable(const clang::CXXBaseSpecifier *Base); +/// not, None if inconclusive. +llvm::Optional<const clang::CXXRecordDecl *> +isRefCountable(const clang::CXXBaseSpecifier *Base); -/// \returns true if \p Class is ref-countable, false if not. -/// Asserts that \p Class IS a definition. -bool isRefCountable(const clang::CXXRecordDecl *Class); +/// \returns true if \p Class is ref-countable, false if not, None if +/// inconclusive. +llvm::Optional<bool> isRefCountable(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is ref-counted, false if not. bool isRefCounted(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is ref-countable AND not ref-counted, false if -/// not. Asserts that \p Class IS a definition. -bool isUncounted(const clang::CXXRecordDecl *Class); +/// not, None if inconclusive. +llvm::Optional<bool> isUncounted(const clang::CXXRecordDecl *Class); /// \returns true if \p T is either a raw pointer or reference to an uncounted -/// class, false if not. -bool isUncountedPtr(const clang::Type *T); +/// class, false if not, None if inconclusive. +llvm::Optional<bool> isUncountedPtr(const clang::Type *T); /// \returns true if \p F creates ref-countable object from uncounted parameter, /// false if not. bool isCtorOfRefCounted(const clang::FunctionDecl *F); /// \returns true if \p M is getter of a ref-counted class, false if not. -bool isGetterOfRefCounted(const clang::CXXMethodDecl *Method); +llvm::Optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl *Method); /// \returns true if \p F is a conversion between ref-countable or ref-counted /// pointer types. diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp index 81ce284c2dc7..fa9ece217cc0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -76,19 +76,15 @@ class RefCntblBaseVirtualDtorChecker (AccSpec == AS_none && RD->isClass())) return false; - llvm::Optional<const clang::CXXRecordDecl *> MaybeRefCntblBaseRD = + llvm::Optional<const CXXRecordDecl *> RefCntblBaseRD = isRefCountable(Base); - if (!MaybeRefCntblBaseRD.hasValue()) + if (!RefCntblBaseRD || !(*RefCntblBaseRD)) return false; - const CXXRecordDecl *RefCntblBaseRD = MaybeRefCntblBaseRD.getValue(); - if (!RefCntblBaseRD) - return false; - - const auto *Dtor = RefCntblBaseRD->getDestructor(); + const auto *Dtor = (*RefCntblBaseRD)->getDestructor(); if (!Dtor || !Dtor->isVirtual()) { ProblematicBaseSpecifier = Base; - ProblematicBaseClass = RefCntblBaseRD; + ProblematicBaseClass = *RefCntblBaseRD; return true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 940a1f349831..d70bd9489d2c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -86,7 +86,8 @@ class UncountedCallArgsChecker continue; // FIXME? Should we bail? // FIXME: more complex types (arrays, references to raw pointers, etc) - if (!isUncountedPtr(ArgType)) + Optional<bool> IsUncounted = isUncountedPtr(ArgType); + if (!IsUncounted || !(*IsUncounted)) continue; const auto *Arg = CE->getArg(ArgIdx); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp index 0a387592d350..deebbd603b2c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp @@ -59,7 +59,8 @@ class UncountedLambdaCapturesChecker if (C.capturesVariable()) { VarDecl *CapturedVar = C.getCapturedVar(); if (auto *CapturedVarType = CapturedVar->getType().getTypePtrOrNull()) { - if (isUncountedPtr(CapturedVarType)) { + Optional<bool> IsUncountedPtr = isUncountedPtr(CapturedVarType); + if (IsUncountedPtr && *IsUncountedPtr) { reportBug(C, CapturedVar, CapturedVarType); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp index 101aba732aea..7e86f28cb70f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp @@ -169,7 +169,8 @@ class UncountedLocalVarsChecker if (!ArgType) return; - if (isUncountedPtr(ArgType)) { + Optional<bool> IsUncountedPtr = isUncountedPtr(ArgType); + if (IsUncountedPtr && *IsUncountedPtr) { const Expr *const InitExpr = V->getInit(); if (!InitExpr) return; // FIXME: later on we might warn on uninitialized vars too _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
