[clang] 47e6851 - [Analyzer][WebKit] Use tri-state types for relevant predicates
Jan Korous via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 22 21:57:36 PDT 2020
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
More information about the cfe-commits
mailing list