Skip to content

Commit 74b16c1

Browse files
committed
Fix the checker for WeakPtr and unique_ptr
1 parent bc6988f commit 74b16c1

File tree

5 files changed

+107
-10
lines changed

5 files changed

+107
-10
lines changed

clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,19 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
283283
break;
284284
}
285285

286+
if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(ArgExpr)) {
287+
if (isOwnerPtrType(MemberCallExpr->getObjectType()))
288+
return;
289+
}
290+
291+
if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(ArgExpr)) {
292+
auto *Method = dyn_cast_or_null<CXXMethodDecl>(OpCE->getDirectCallee());
293+
if (Method && isOwnerPtr(safeGetName(Method->getParent()))) {
294+
if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1)
295+
return;
296+
}
297+
}
298+
286299
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
287300
isa<CXXDefaultArgExpr>(ArgExpr))
288301
return;

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ bool isCheckedPtr(const std::string &Name) {
138138
return Name == "CheckedPtr" || Name == "CheckedRef";
139139
}
140140

141+
bool isOwnerPtr(const std::string &Name) {
142+
return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
143+
Name == "UniqueRef" || Name == "LazyUniqueRef";
144+
}
145+
141146
bool isSmartPtrClass(const std::string &Name) {
142147
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
143148
Name == "WeakPtr" || Name == "WeakPtrFactory" ||
@@ -206,10 +211,7 @@ bool isRetainPtrOrOSPtrType(const clang::QualType T) {
206211
}
207212

208213
bool isOwnerPtrType(const clang::QualType T) {
209-
return isPtrOfType(T, [](auto Name) {
210-
return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
211-
Name == "UniqueRef" || Name == "LazyUniqueRef";
212-
});
214+
return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); });
213215
}
214216

215217
std::optional<bool> isUncounted(const QualType T) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ bool isCheckedPtr(const std::string &Name);
143143
/// \returns true if \p Name is RetainPtr or its variant, false if not.
144144
bool isRetainPtrOrOSPtr(const std::string &Name);
145145

146+
/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr,
147+
/// and unique_ptr.
148+
bool isOwnerPtr(const std::string &Name);
149+
146150
/// \returns true if \p Name is a smart pointer type name, false if not.
147151
bool isSmartPtrClass(const std::string &Name);
148152

clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,14 @@
4040
return obj;
4141
}
4242

43-
void opaque_call_arg(Obj* obj, Obj&& otherObj) {
43+
void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr<Obj>& safeObj, WeakPtr<Obj> weakObj, std::unique_ptr<Obj> uniqObj) {
4444
receive_obj_ref(*obj);
4545
receive_obj_ptr(&*obj);
4646
receive_obj_rref(std::move(otherObj));
47+
receive_obj_ref(*safeObj.get());
48+
receive_obj_ptr(weakObj.get());
49+
// expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}}
50+
receive_obj_ref(*uniqObj);
4751
}
4852

4953
Obj&& provide_obj_rval();

clang/test/Analysis/Checkers/WebKit/mock-types.h

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,23 @@ namespace std {
2525
template <typename T>
2626
class unique_ptr {
2727
private:
28-
T *t;
28+
void *t;
2929

3030
public:
3131
unique_ptr() : t(nullptr) { }
3232
unique_ptr(T *t) : t(t) { }
3333
~unique_ptr() {
3434
if (t)
35-
delete t;
35+
delete static_cast<T*>(t);
3636
}
3737
template <typename U> unique_ptr(unique_ptr<U>&& u)
3838
: t(u.t)
3939
{
4040
u.t = nullptr;
4141
}
42-
T *get() const { return t; }
43-
T *operator->() const { return t; }
44-
T &operator*() const { return *t; }
42+
T *get() const { return static_cast<T*>(t); }
43+
T *operator->() const { return get(); }
44+
T &operator*() const { return *get(); }
4545
unique_ptr &operator=(T *) { return *this; }
4646
explicit operator bool() const { return !!t; }
4747
};
@@ -313,4 +313,78 @@ class UniqueRef {
313313
UniqueRef &operator=(T &) { return *this; }
314314
};
315315

316+
template <typename T>
317+
class WeakPtrImpl {
318+
private:
319+
void* ptr;
320+
321+
template <typename U> friend class CanMakeWeakPtr;
322+
template <typename U> friend class WeakPtr;
323+
324+
Ref<WeakPtrImpl<T>> create(T& t)
325+
{
326+
return adoptNS(new WeakPtrImpl<T>(t));
327+
}
328+
329+
T* get() { return static_cast<T*>(ptr); }
330+
operator bool() const { return !!ptr; }
331+
void clear() { ptr = nullptr; }
332+
333+
WeakPtrImpl(T& t)
334+
: ptr(static_cast<void*>(t))
335+
{ }
336+
};
337+
338+
template <typename T>
339+
class CanMakeWeakPtr {
340+
private:
341+
RefPtr<WeakPtrImpl<T>> impl;
342+
343+
template <typename U> friend class CanMakeWeakPtr;
344+
template <typename U> friend class WeakPtr;
345+
346+
Ref<WeakPtrImpl<T>> createWeakPtrImpl() {
347+
if (!impl)
348+
impl = WeakPtrImpl<T>::create(static_cast<T>(*this));
349+
return *impl;
350+
}
351+
352+
public:
353+
~CanMakeWeakPtr() {
354+
if (!impl)
355+
return;
356+
impl->ptr = nullptr;
357+
impl = nullptr;
358+
}
359+
};
360+
361+
template <typename T>
362+
class WeakPtr {
363+
private:
364+
RefPtr<WeakPtrImpl<T>> impl;
365+
366+
public:
367+
WeakPtr(T& t) {
368+
*this = t;
369+
}
370+
WeakPtr(T* t) {
371+
*this = t;
372+
}
373+
374+
template <typename U>
375+
WeakPtr<T> operator=(U& obj) {
376+
impl = obj.createWeakPtrImpl();
377+
}
378+
379+
template <typename U>
380+
WeakPtr<T> operator=(U* obj) {
381+
impl = obj ? obj->createWeakPtrImpl() : nullptr;
382+
}
383+
384+
T* get() {
385+
return impl ? impl->get() : nullptr;
386+
}
387+
388+
};
389+
316390
#endif

0 commit comments

Comments
 (0)