Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ bool tryToFindPtrOrigin(
}
}

if (call->isCallToStdMove() && call->getNumArgs() == 1) {
E = call->getArg(0)->IgnoreParenCasts();
continue;
}

if (auto *callee = call->getDirectCallee()) {
if (isCtorOfSafePtr(callee)) {
if (StopAtFirstRefCountedObj)
Expand All @@ -146,6 +141,11 @@ bool tryToFindPtrOrigin(
continue;
}

if (isStdOrWTFMove(callee) && call->getNumArgs() == 1) {
E = call->getArg(0)->IgnoreParenCasts();
continue;
}

if (isSafePtrType(callee->getReturnType()))
return callback(E, true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
ArgExpr = ArgExpr->IgnoreParenCasts();
if (auto *InnerCE = dyn_cast<CallExpr>(ArgExpr)) {
auto *InnerCallee = InnerCE->getDirectCallee();
if (InnerCallee && InnerCallee->isInStdNamespace() &&
safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
if (isStdOrWTFMove(InnerCallee) && InnerCE->getNumArgs() == 1) {
ArgExpr = InnerCE->getArg(0);
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
isCtorOfRetainPtrOrOSPtr(F);
}

bool isStdOrWTFMove(const clang::FunctionDecl *F) {
auto FnName = safeGetName(F);
auto *Namespace = F->getParent();
if (!Namespace)
return false;
auto NsName = safeGetName(Namespace);
return (NsName == "WTF" || NsName == "std") && FnName == "move";
}
Comment on lines 188 to 198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we ensure here that the parent of this parent is actually the TUDecl?
This way I think we would match: my::internal::std::move etc. I'd say it's not too likely to have something like this...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can check that. I'll just add:

  if (Namespace->getParent())
    return false;

before auto NsName = safeGetName(Namespace);.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oops, I need to check that it's TranslationUnitDecl instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add this instead:

  auto* TUDeck = Namespace->getParent();
  if (!TUDeck || !isa<TranslationUnitDecl>(TUDeck))
    return false;

Copy link
Contributor

@steakhal steakhal Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using isa_and_nonnull? Fusing the disjunction?

I'm not sure it makes sense. Its too late to think about demorgan rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, that's neat. will use that instead.


template <typename Predicate>
static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
QualType type = T;
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F);
/// uncounted parameter, false if not.
bool isCtorOfSafePtr(const clang::FunctionDecl *F);

/// \returns true if \p F is std::move or WTF::move.
bool isStdOrWTFMove(const clang::FunctionDecl *F);

/// \returns true if \p Name is RefPtr, Ref, or its variant, false if not.
bool isRefType(const std::string &Name);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,9 @@ class RawPtrRefLambdaCapturesChecker
return false;
}
if (auto *CE = dyn_cast<CallExpr>(Arg)) {
if (CE->isCallToStdMove() && CE->getNumArgs() == 1) {
Arg = CE->getArg(0)->IgnoreParenCasts();
continue;
}
if (auto *Callee = CE->getDirectCallee()) {
if (isCtorOfSafePtr(Callee) && CE->getNumArgs() == 1) {
if ((isStdOrWTFMove(Callee) || isCtorOfSafePtr(Callee)) &&
CE->getNumArgs() == 1) {
Arg = CE->getArg(0)->IgnoreParenCasts();
continue;
}
Expand Down
1 change: 1 addition & 0 deletions clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ namespace call_with_std_move {
void consume(CheckedObj&&);
void foo(CheckedObj&& obj) {
consume(std::move(obj));
consume(WTF::move(obj));
}

}
3 changes: 3 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr<Obj>& safeObj, WeakP
receive_obj_ref(*obj);
receive_obj_ptr(&*obj);
receive_obj_rref(std::move(otherObj));
receive_obj_rref(WTF::move(otherObj));
receive_obj_ref(*safeObj.get());
receive_obj_ptr(weakObj.get());
// expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}}
Expand All @@ -59,6 +60,7 @@ void rval(Obj&& arg) {
auto &&obj = provide_obj_rval();
// expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj &&'}}
receive_obj_rval(std::move(arg));
receive_obj_rval(WTF::move(arg));
}

ObjCObj *provide_objcobj();
Expand All @@ -84,6 +86,7 @@ void construct_ptr(Obj&& arg) {
WrapperObj wrapper2(provide_obj_ref());
// expected-warning@-1{{Call argument for parameter 'obj' uses a forward declared type 'Obj &'}}
WrapperObj wrapper3(std::move(arg));
WrapperObj wrapper4(WTF::move(arg));
}

JSStringRef provide_opaque_ptr();
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/mock-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ template<typename T> typename remove_reference<T>::type&& move(T&& t);

#endif

namespace WTF {

template<typename T> typename std::remove_reference<T>::type&& move(T&& t);

}

#ifndef mock_types_1103988513531
#define mock_types_1103988513531

Expand Down
10 changes: 8 additions & 2 deletions clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ template<typename T> typename remove_reference<T>::type&& move(T&& t);

#endif

namespace WTF {

template<typename T> typename std::remove_reference<T>::type&& move(T&& t);

}

namespace std {

template <bool, typename U = void> struct enable_if {
Expand Down Expand Up @@ -453,7 +459,7 @@ template<typename T> class OSObjectPtr {
}

OSObjectPtr(T ptr)
: m_ptr(std::move(ptr))
: m_ptr(WTF::move(ptr))
{
if (m_ptr)
retainOSObject(m_ptr);
Expand Down Expand Up @@ -483,7 +489,7 @@ template<typename T> class OSObjectPtr {

OSObjectPtr& operator=(T other)
{
OSObjectPtr ptr = std::move(other);
OSObjectPtr ptr = WTF::move(other);
swap(ptr);
return *this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,14 @@ struct RefCountableWithLambdaCapturingThis {
});
});
}

void method_nested_lambda4() {
callAsync([this, protectedThis = RefPtr { this }] {
callAsync([this, protectedThis = WTF::move(*protectedThis)] {
nonTrivial();
});
});
}
};

struct NonRefCountableWithLambdaCapturingThis {
Expand Down
1 change: 1 addition & 0 deletions clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ class RefCounted {
unsigned trivial69() { return offsetof(OtherObj, children); }
DerivedNumber* trivial70() { [[clang::suppress]] return static_cast<DerivedNumber*>(number); }
unsigned trivial71() { return std::bit_cast<unsigned>(nullptr); }
unsigned trivial72() { Number n { 5 }; return WTF::move(n).value(); }

static RefCounted& singleton() {
static RefCounted s_RefCounted;
Expand Down