diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -34,7 +34,9 @@ } if (auto *call = dyn_cast(E)) { if (auto *memberCall = dyn_cast(call)) { - if (isGetterOfRefCounted(memberCall->getMethodDecl())) { + Optional 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 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp @@ -76,8 +76,11 @@ 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 isRCAble = isRefCountable(MemberCXXRD); + if (isRCAble && *isRCAble) + reportBug(Member, MemberType, MemberCXXRD, RD); + } } } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h --- 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 @@ // Ref. /// \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 +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 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 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 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 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/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp --- 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 @@ bool hasPublicRefAndDeref(const CXXRecordDecl *R) { assert(R); + assert(R->hasDefinition()); bool hasRef = false; bool hasDeref = false; @@ -43,25 +45,29 @@ namespace clang { -const CXXRecordDecl *isRefCountable(const CXXBaseSpecifier *Base) { +llvm::Optional +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 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 @@ CXXBasePaths Paths; Paths.setOrigin(const_cast(R)); - const auto isRefCountableBase = [](const CXXBaseSpecifier *Base, - CXXBasePath &) { - return clang::isRefCountable(Base); - }; + bool AnyInconclusiveBase = false; + const auto isRefCountableBase = + [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) { + Optional 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 @@ || FunctionName == "Identifier"; } -bool isUncounted(const CXXRecordDecl *Class) { +llvm::Optional isUncounted(const CXXRecordDecl *Class) { // Keep isRefCounted first as it's cheaper. - return !isRefCounted(Class) && isRefCountable(Class); + if (isRefCounted(Class)) + return false; + + llvm::Optional IsRefCountable = isRefCountable(Class); + if (!IsRefCountable) + return llvm::None; + + return (*IsRefCountable); } -bool isUncountedPtr(const Type *T) { +llvm::Optional isUncountedPtr(const Type *T) { assert(T); if (T->isPointerType() || T->isReferenceType()) { @@ -111,7 +135,7 @@ return false; } -bool isGetterOfRefCounted(const CXXMethodDecl *M) { +Optional isGetterOfRefCounted(const CXXMethodDecl *M) { assert(M); if (isa(M)) { @@ -133,9 +157,7 @@ if (auto *maybeRefToRawOperator = dyn_cast(M)) { if (auto *targetConversionType = maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) { - if (isUncountedPtr(targetConversionType)) { - return true; - } + return isUncountedPtr(targetConversionType); } } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -76,19 +76,15 @@ (AccSpec == AS_none && RD->isClass())) return false; - llvm::Optional MaybeRefCntblBaseRD = + llvm::Optional 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 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -86,7 +86,8 @@ continue; // FIXME? Should we bail? // FIXME: more complex types (arrays, references to raw pointers, etc) - if (!isUncountedPtr(ArgType)) + Optional 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 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp @@ -59,7 +59,8 @@ if (C.capturesVariable()) { VarDecl *CapturedVar = C.getCapturedVar(); if (auto *CapturedVarType = CapturedVar->getType().getTypePtrOrNull()) { - if (isUncountedPtr(CapturedVarType)) { + Optional 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 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp @@ -169,7 +169,8 @@ if (!ArgType) return; - if (isUncountedPtr(ArgType)) { + Optional 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