Skip to content
55 changes: 50 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 Expand Up @@ -321,6 +321,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) {
return result && *result;
}

bool isAllocInit(const Expr *E, const Expr **InnerExpr) {
auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) {
if (unsigned ExprCount = POE->getNumSemanticExprs()) {
auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts();
ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr);
if (InnerExpr)
*InnerExpr = ObjCMsgExpr;
}
}
if (!ObjCMsgExpr)
return false;
auto Selector = ObjCMsgExpr->getSelector();
auto NameForFirstSlot = Selector.getNameForSlot(0);
if (NameForFirstSlot.starts_with("alloc") ||
NameForFirstSlot.starts_with("copy") ||
NameForFirstSlot.starts_with("mutableCopy"))
return true;
if (!NameForFirstSlot.starts_with("init") &&
!NameForFirstSlot.starts_with("_init"))
return false;
if (!ObjCMsgExpr->isInstanceMessage())
return false;
auto *Receiver = ObjCMsgExpr->getInstanceReceiver();
if (!Receiver)
return false;
Receiver = Receiver->IgnoreParenCasts();
if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) {
if (InnerExpr)
*InnerExpr = Inner;
auto InnerSelector = Inner->getSelector();
return InnerSelector.getNameForSlot(0).starts_with("alloc");
} else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
if (InnerExpr)
*InnerExpr = CE;
if (auto *Callee = CE->getDirectCallee()) {
if (Callee->getDeclName().isIdentifier()) {
auto CalleeName = Callee->getName();
return CalleeName.starts_with("alloc");
}
}
}
return false;
}

class EnsureFunctionVisitor
: public ConstStmtVisitor<EnsureFunctionVisitor, bool> {
public:
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
/// supports CheckedPtr.
bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E);

/// \returns true if \p E is a [[alloc] init] pattern expression.
/// Sets \p InnerExpr to the inner function call or selector invocation.
bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr);

/// \returns true if E is a CXXMemberCallExpr which returns a const smart
/// pointer type.
class EnsureFunctionAnalysis {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,11 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
while (ArgExpr) {
ArgExpr = ArgExpr->IgnoreParenCasts();
if (auto *InnerCE = dyn_cast<CallExpr>(ArgExpr)) {
auto *InnerCallee = InnerCE->getDirectCallee();
if (InnerCallee && InnerCallee->isInStdNamespace() &&
safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
ArgExpr = InnerCE->getArg(0);
continue;
if (auto *InnerCallee = InnerCE->getDirectCallee()) {
if (isStdOrWTFMove(InnerCallee) && InnerCE->getNumArgs() == 1) {
ArgExpr = InnerCE->getArg(0);
continue;
}
}
}
if (auto *UO = dyn_cast<UnaryOperator>(ArgExpr)) {
Expand Down
75 changes: 22 additions & 53 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,18 @@ 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 *TUDeck = Namespace->getParent();
if (!isa_and_nonnull<TranslationUnitDecl>(TUDeck))
return false;
auto NsName = safeGetName(Namespace);
return (NsName == "WTF" || NsName == "std") && FnName == "move";
}

template <typename Predicate>
static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
QualType type = T;
Expand Down Expand Up @@ -288,37 +300,6 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
return RT && CFPointees.contains(RT);
}

std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
if (auto *Decl = Subst->getAssociatedDecl()) {
if (isRetainPtrOrOSPtr(safeGetName(Decl)))
return false;
}
}
if ((ento::cocoa::isCocoaObjectRef(T) && !IsARCEnabled) ||
ento::coreFoundation::isCFObjectRef(T))
return true;

// RetainPtr strips typedef for CF*Ref. Manually check for struct __CF* types.
auto CanonicalType = T.getCanonicalType();
auto *Type = CanonicalType.getTypePtrOrNull();
if (!Type)
return false;
auto Pointee = Type->getPointeeType();
auto *PointeeType = Pointee.getTypePtrOrNull();
if (!PointeeType)
return false;
auto *Record = PointeeType->getAsStructureType();
if (!Record)
return false;
auto *Decl = Record->getDecl();
if (!Decl)
return false;
auto TypeName = Decl->getName();
return TypeName.starts_with("__CF") || TypeName.starts_with("__CG") ||
TypeName.starts_with("__CM");
}

std::optional<bool> isUncounted(const CXXRecordDecl* Class)
{
// Keep isRefCounted first as it's cheaper.
Expand Down Expand Up @@ -354,25 +335,6 @@ std::optional<bool> isUncheckedPtr(const QualType T) {
return false;
}

std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled) {
if (T->isPointerType() || T->isReferenceType()) {
if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
auto isUncountedPtr = isUncounted(CXXRD);
auto isUncheckedPtr = isUnchecked(CXXRD);
auto isUnretainedPtr = isUnretained(T, IsArcEnabled);
std::optional<bool> result;
if (isUncountedPtr)
result = *isUncountedPtr;
if (isUncheckedPtr)
result = result ? *result || *isUncheckedPtr : *isUncheckedPtr;
if (isUnretainedPtr)
result = result ? *result || *isUnretainedPtr : *isUnretainedPtr;
return result;
}
}
return false;
}

std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
assert(M);

Expand Down Expand Up @@ -574,6 +536,15 @@ class TrivialFunctionAnalysisVisitor
});
}

bool IsStatementTrivial(const Stmt *S) {
auto CacheIt = Cache.find(S);
if (CacheIt != Cache.end())
return CacheIt->second;
bool Result = Visit(S);
Cache[S] = Result;
return Result;
}

bool VisitStmt(const Stmt *S) {
// All statements are non-trivial unless overriden later.
// Don't even recurse into children by default.
Expand Down Expand Up @@ -877,9 +848,7 @@ bool TrivialFunctionAnalysis::isTrivialImpl(
bool TrivialFunctionAnalysis::isTrivialImpl(
const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) {
TrivialFunctionAnalysisVisitor V(Cache);
bool Result = V.Visit(S);
assert(Cache.contains(S) && "Top-level statement not properly cached!");
return Result;
return V.IsStatementTrivial(S);
}

} // namespace clang
11 changes: 3 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ class RetainTypeChecker {
bool defaultSynthProperties() const { return DefaultSynthProperties; }
};

/// \returns true if \p Class is NS or CF objects AND not retained, false if
/// not, std::nullopt if inconclusive.
std::optional<bool> isUnretained(const clang::QualType T, bool IsARCEnabled);

/// \returns true if \p Class is ref-countable AND not ref-counted, false if
/// not, std::nullopt if inconclusive.
std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
Expand All @@ -107,10 +103,6 @@ std::optional<bool> isUncountedPtr(const clang::QualType T);
/// class, false if not, std::nullopt if inconclusive.
std::optional<bool> isUncheckedPtr(const clang::QualType T);

/// \returns true if \p T is either a raw pointer or reference to an uncounted
/// or unchecked class, false if not, std::nullopt if inconclusive.
std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled);

/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
/// variant, false if not.
bool isRefOrCheckedPtrType(const clang::QualType T);
Expand All @@ -134,6 +126,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 @@ -177,16 +177,11 @@ class RawPtrRefCallArgsChecker
if (BR->getSourceManager().isInSystemHeader(E->getExprLoc()))
return;

auto Selector = E->getSelector();
if (auto *Receiver = E->getInstanceReceiver()) {
std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType());
if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) {
if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(Receiver)) {
auto InnerSelector = InnerMsg->getSelector();
if (InnerSelector.getNameForSlot(0) == "alloc" &&
Selector.getNameForSlot(0).starts_with("init"))
return;
}
if (isAllocInit(E))
return;
reportBugOnReceiver(Receiver, D);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,17 @@ class RawPtrRefLambdaCapturesChecker
auto *Callee = CE->getCallee();
if (!Callee)
return;
Callee = Callee->IgnoreParenCasts();
if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Callee)) {
Callee = MTE->getSubExpr();
if (!Callee)
return;
Callee = Callee->IgnoreParenCasts();
}
if (auto *L = dyn_cast<LambdaExpr>(Callee)) {
LambdasToIgnore.insert(L); // Calling a lambda upon creation is safe.
return;
}
auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());
if (!DRE)
return;
Expand Down Expand Up @@ -416,12 +427,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 Expand Up @@ -587,6 +595,8 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
}

std::optional<bool> isUnsafePtr(QualType QT) const final {
if (QT.hasStrongOrWeakObjCLifetime())
return false;
return RTC->isUnretained(QT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ class RawPtrRefLocalVarsChecker
}

bool TraverseIfStmt(IfStmt *IS) override {
if (IS->getConditionVariable()) {
// This code currently does not explicitly check the "else" statement
// since getConditionVariable returns nullptr when there is a
// condition defined after ";" as in "if (auto foo = ~; !foo)". If
// this semantics change, we should add an explicit check for "else".
if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then))
return true;
}
if (!TFA.isTrivial(IS))
return DynamicRecursiveASTVisitor::TraverseIfStmt(IS);
return true;
Expand Down Expand Up @@ -433,6 +441,8 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
RTC = RetainTypeChecker();
}
std::optional<bool> isUnsafePtr(const QualType T) const final {
if (T.hasStrongOrWeakObjCLifetime())
return false;
return RTC->isUnretained(T);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,11 @@ class RawPtrRefMemberChecker
// "assign" property doesn't retain even under ARC so treat it as unsafe.
bool ignoreARC =
!PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign;
bool IsWeak =
PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak;
bool HasSafeAttr = PD->isRetaining() || IsWeak;
auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC);
return {IsUnsafePtr && *IsUnsafePtr && !PD->isRetaining(), PropType};
return {IsUnsafePtr && *IsUnsafePtr && !HasSafeAttr, PropType};
}

bool shouldSkipDecl(const RecordDecl *RD) const {
Expand Down Expand Up @@ -363,6 +366,8 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
}

std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final {
if (QT.hasStrongOrWeakObjCLifetime())
return false;
return RTC->isUnretained(QT, ignoreARC);
}

Expand Down
Loading