Skip to content

Commit ef50e3e

Browse files
jkorous-applejkorous-apple
authored
Merge pull request #2037 from jkorous-apple/analyzer-webkit-fix
[Analyzer][WebKit] Use tri-state types for relevant predicates
2 parents bf5cff7 + 3fbf5c0 commit ef50e3e

8 files changed

+71
-42
lines changed

‎clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
3434
}
3535
if (auto *call = dyn_cast<CallExpr>(E)) {
3636
if (auto *memberCall = dyn_cast<CXXMemberCallExpr>(call)) {
37-
if (isGetterOfRefCounted(memberCall->getMethodDecl())) {
37+
Optional<bool> IsGetterOfRefCt =
38+
isGetterOfRefCounted(memberCall->getMethodDecl());
39+
if (IsGetterOfRefCt && *IsGetterOfRefCt) {
3840
E = memberCall->getImplicitObjectArgument();
3941
if (StopAtFirstRefCountedObj) {
4042
return {E, true};

‎clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ class NoUncountedMemberChecker
7676

7777
if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
7878
// If we don't see the definition we just don't know.
79-
if (MemberCXXRD->hasDefinition() && isRefCountable(MemberCXXRD))
80-
reportBug(Member, MemberType, MemberCXXRD, RD);
79+
if (MemberCXXRD->hasDefinition()) {
80+
llvm::Optional<bool> isRCAble = isRefCountable(MemberCXXRD);
81+
if (isRCAble && *isRCAble)
82+
reportBug(Member, MemberType, MemberCXXRD, RD);
83+
}
8184
}
8285
}
8386
}

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "clang/AST/Decl.h"
1313
#include "clang/AST/DeclCXX.h"
1414
#include "clang/AST/ExprCXX.h"
15+
#include "llvm/ADT/Optional.h"
1516

1617
using llvm::Optional;
1718
using namespace clang;
@@ -20,6 +21,7 @@ namespace {
2021

2122
bool hasPublicRefAndDeref(const CXXRecordDecl *R) {
2223
assert(R);
24+
assert(R->hasDefinition());
2325

2426
bool hasRef = false;
2527
bool hasDeref = false;
@@ -43,39 +45,54 @@ bool hasPublicRefAndDeref(const CXXRecordDecl *R) {
4345

4446
namespace clang {
4547

46-
const CXXRecordDecl *isRefCountable(const CXXBaseSpecifier *Base) {
48+
llvm::Optional<const clang::CXXRecordDecl *>
49+
isRefCountable(const CXXBaseSpecifier *Base) {
4750
assert(Base);
4851

4952
const Type *T = Base->getType().getTypePtrOrNull();
5053
if (!T)
51-
return nullptr;
54+
return llvm::None;
5255

5356
const CXXRecordDecl *R = T->getAsCXXRecordDecl();
5457
if (!R)
55-
return nullptr;
58+
return llvm::None;
59+
if (!R->hasDefinition())
60+
return llvm::None;
5661

5762
return hasPublicRefAndDeref(R) ? R : nullptr;
5863
}
5964

60-
bool isRefCountable(const CXXRecordDecl *R) {
65+
llvm::Optional<bool> isRefCountable(const CXXRecordDecl *R) {
6166
assert(R);
6267

6368
R = R->getDefinition();
64-
assert(R);
69+
if (!R)
70+
return llvm::None;
6571

6672
if (hasPublicRefAndDeref(R))
6773
return true;
6874

6975
CXXBasePaths Paths;
7076
Paths.setOrigin(const_cast<CXXRecordDecl *>(R));
7177

72-
const auto isRefCountableBase = [](const CXXBaseSpecifier *Base,
73-
CXXBasePath &) {
74-
return clang::isRefCountable(Base);
75-
};
78+
bool AnyInconclusiveBase = false;
79+
const auto isRefCountableBase =
80+
[&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
81+
Optional<const clang::CXXRecordDecl *> IsRefCountable =
82+
clang::isRefCountable(Base);
83+
if (!IsRefCountable) {
84+
AnyInconclusiveBase = true;
85+
return false;
86+
}
87+
return (*IsRefCountable) != nullptr;
88+
};
89+
90+
bool BasesResult = R->lookupInBases(isRefCountableBase, Paths,
91+
/*LookupInDependent =*/true);
92+
if (AnyInconclusiveBase)
93+
return llvm::None;
7694

77-
return R->lookupInBases(isRefCountableBase, Paths,
78-
/*LookupInDependent =*/true);
95+
return BasesResult;
7996
}
8097

8198
bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
@@ -95,12 +112,19 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
95112
|| FunctionName == "Identifier";
96113
}
97114

98-
bool isUncounted(const CXXRecordDecl *Class) {
115+
llvm::Optional<bool> isUncounted(const CXXRecordDecl *Class) {
99116
// Keep isRefCounted first as it's cheaper.
100-
return !isRefCounted(Class) && isRefCountable(Class);
117+
if (isRefCounted(Class))
118+
return false;
119+
120+
llvm::Optional<bool> IsRefCountable = isRefCountable(Class);
121+
if (!IsRefCountable)
122+
return llvm::None;
123+
124+
return (*IsRefCountable);
101125
}
102126

103-
bool isUncountedPtr(const Type *T) {
127+
llvm::Optional<bool> isUncountedPtr(const Type *T) {
104128
assert(T);
105129

106130
if (T->isPointerType() || T->isReferenceType()) {
@@ -111,7 +135,7 @@ bool isUncountedPtr(const Type *T) {
111135
return false;
112136
}
113137

114-
bool isGetterOfRefCounted(const CXXMethodDecl *M) {
138+
Optional<bool> isGetterOfRefCounted(const CXXMethodDecl *M) {
115139
assert(M);
116140

117141
if (isa<CXXMethodDecl>(M)) {
@@ -133,9 +157,7 @@ bool isGetterOfRefCounted(const CXXMethodDecl *M) {
133157
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
134158
if (auto *targetConversionType =
135159
maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
136-
if (isUncountedPtr(targetConversionType)) {
137-
return true;
138-
}
160+
return isUncountedPtr(targetConversionType);
139161
}
140162
}
141163
}

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#ifndef LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H
1010
#define LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H
1111

12+
#include "llvm/ADT/APInt.h"
13+
1214
namespace clang {
1315
class CXXBaseSpecifier;
1416
class CXXMethodDecl;
@@ -25,30 +27,31 @@ class Type;
2527
// Ref<T>.
2628

2729
/// \returns CXXRecordDecl of the base if the type is ref-countable, nullptr if
28-
/// not.
29-
const clang::CXXRecordDecl *isRefCountable(const clang::CXXBaseSpecifier *Base);
30+
/// not, None if inconclusive.
31+
llvm::Optional<const clang::CXXRecordDecl *>
32+
isRefCountable(const clang::CXXBaseSpecifier *Base);
3033

31-
/// \returns true if \p Class is ref-countable, false if not.
32-
/// Asserts that \p Class IS a definition.
33-
bool isRefCountable(const clang::CXXRecordDecl *Class);
34+
/// \returns true if \p Class is ref-countable, false if not, None if
35+
/// inconclusive.
36+
llvm::Optional<bool> isRefCountable(const clang::CXXRecordDecl *Class);
3437

3538
/// \returns true if \p Class is ref-counted, false if not.
3639
bool isRefCounted(const clang::CXXRecordDecl *Class);
3740

3841
/// \returns true if \p Class is ref-countable AND not ref-counted, false if
39-
/// not. Asserts that \p Class IS a definition.
40-
bool isUncounted(const clang::CXXRecordDecl *Class);
42+
/// not, None if inconclusive.
43+
llvm::Optional<bool> isUncounted(const clang::CXXRecordDecl *Class);
4144

4245
/// \returns true if \p T is either a raw pointer or reference to an uncounted
43-
/// class, false if not.
44-
bool isUncountedPtr(const clang::Type *T);
46+
/// class, false if not, None if inconclusive.
47+
llvm::Optional<bool> isUncountedPtr(const clang::Type *T);
4548

4649
/// \returns true if \p F creates ref-countable object from uncounted parameter,
4750
/// false if not.
4851
bool isCtorOfRefCounted(const clang::FunctionDecl *F);
4952

5053
/// \returns true if \p M is getter of a ref-counted class, false if not.
51-
bool isGetterOfRefCounted(const clang::CXXMethodDecl *Method);
54+
llvm::Optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl *Method);
5255

5356
/// \returns true if \p F is a conversion between ref-countable or ref-counted
5457
/// pointer types.

‎clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,15 @@ class RefCntblBaseVirtualDtorChecker
7676
(AccSpec == AS_none && RD->isClass()))
7777
return false;
7878

79-
llvm::Optional<const clang::CXXRecordDecl *> MaybeRefCntblBaseRD =
79+
llvm::Optional<const CXXRecordDecl *> RefCntblBaseRD =
8080
isRefCountable(Base);
81-
if (!MaybeRefCntblBaseRD.hasValue())
81+
if (!RefCntblBaseRD || !(*RefCntblBaseRD))
8282
return false;
8383

84-
const CXXRecordDecl *RefCntblBaseRD = MaybeRefCntblBaseRD.getValue();
85-
if (!RefCntblBaseRD)
86-
return false;
87-
88-
const auto *Dtor = RefCntblBaseRD->getDestructor();
84+
const auto *Dtor = (*RefCntblBaseRD)->getDestructor();
8985
if (!Dtor || !Dtor->isVirtual()) {
9086
ProblematicBaseSpecifier = Base;
91-
ProblematicBaseClass = RefCntblBaseRD;
87+
ProblematicBaseClass = *RefCntblBaseRD;
9288
return true;
9389
}
9490

‎clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ class UncountedCallArgsChecker
8686
continue; // FIXME? Should we bail?
8787

8888
// FIXME: more complex types (arrays, references to raw pointers, etc)
89-
if (!isUncountedPtr(ArgType))
89+
Optional<bool> IsUncounted = isUncountedPtr(ArgType);
90+
if (!IsUncounted || !(*IsUncounted))
9091
continue;
9192

9293
const auto *Arg = CE->getArg(ArgIdx);

‎clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ class UncountedLambdaCapturesChecker
5959
if (C.capturesVariable()) {
6060
VarDecl *CapturedVar = C.getCapturedVar();
6161
if (auto *CapturedVarType = CapturedVar->getType().getTypePtrOrNull()) {
62-
if (isUncountedPtr(CapturedVarType)) {
62+
Optional<bool> IsUncountedPtr = isUncountedPtr(CapturedVarType);
63+
if (IsUncountedPtr && *IsUncountedPtr) {
6364
reportBug(C, CapturedVar, CapturedVarType);
6465
}
6566
}

‎clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ class UncountedLocalVarsChecker
169169
if (!ArgType)
170170
return;
171171

172-
if (isUncountedPtr(ArgType)) {
172+
Optional<bool> IsUncountedPtr = isUncountedPtr(ArgType);
173+
if (IsUncountedPtr && *IsUncountedPtr) {
173174
const Expr *const InitExpr = V->getInit();
174175
if (!InitExpr)
175176
return; // FIXME: later on we might warn on uninitialized vars too

0 commit comments

Comments
 (0)