diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 94eaa81af5177..1a9d6d3127fb7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -23,6 +23,10 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) { E = tempExpr->getSubExpr(); continue; } + if (auto *tempExpr = dyn_cast(E)) { + E = tempExpr->getSubExpr(); + continue; + } if (auto *cast = dyn_cast(E)) { if (StopAtFirstRefCountedObj) { if (auto *ConversionFunc = diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index bf6f9a64877c6..287f6a5287005 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -103,15 +103,18 @@ std::optional isRefCountable(const CXXRecordDecl* R) return hasRef && hasDeref; } +bool isRefType(const std::string &Name) { + return Name == "Ref" || Name == "RefAllowingPartiallyDestroyed" || + Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed"; +} + bool isCtorOfRefCounted(const clang::FunctionDecl *F) { assert(F); - const auto &FunctionName = safeGetName(F); - - return FunctionName == "Ref" || FunctionName == "makeRef" + const std::string &FunctionName = safeGetName(F); - || FunctionName == "RefPtr" || FunctionName == "makeRefPtr" - - || FunctionName == "UniqueRef" || FunctionName == "makeUniqueRef" || + return isRefType(FunctionName) || FunctionName == "makeRef" || + FunctionName == "makeRefPtr" || FunctionName == "UniqueRef" || + FunctionName == "makeUniqueRef" || FunctionName == "makeUniqueRefWithoutFastMallocCheck" || FunctionName == "String" || FunctionName == "AtomString" || @@ -131,7 +134,7 @@ bool isReturnValueRefCounted(const clang::FunctionDecl *F) { if (auto *specialT = type->getAs()) { if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) { auto name = decl->getNameAsString(); - return name == "Ref" || name == "RefPtr"; + return isRefType(name); } return false; } @@ -172,20 +175,18 @@ std::optional isGetterOfRefCounted(const CXXMethodDecl* M) if (isa(M)) { const CXXRecordDecl *calleeMethodsClass = M->getParent(); auto className = safeGetName(calleeMethodsClass); - auto methodName = safeGetName(M); + auto method = safeGetName(M); - if (((className == "Ref" || className == "RefPtr") && - methodName == "get") || - (className == "Ref" && methodName == "ptr") || + if ((isRefType(className) && (method == "get" || method == "ptr")) || ((className == "String" || className == "AtomString" || className == "AtomStringImpl" || className == "UniqueString" || className == "UniqueStringImpl" || className == "Identifier") && - methodName == "impl")) + method == "impl")) return true; // Ref -> T conversion // FIXME: Currently allowing any Ref -> whatever cast. - if (className == "Ref" || className == "RefPtr") { + if (isRefType(className)) { if (auto *maybeRefToRawOperator = dyn_cast(M)) { if (auto *targetConversionType = maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) { @@ -202,7 +203,7 @@ bool isRefCounted(const CXXRecordDecl *R) { if (auto *TmplR = R->getTemplateInstantiationPattern()) { // FIXME: String/AtomString/UniqueString const auto &ClassName = safeGetName(TmplR); - return ClassName == "RefPtr" || ClassName == "Ref"; + return isRefType(ClassName); } return false; } @@ -252,6 +253,19 @@ class TrivialFunctionAnalysisVisitor return true; } + template + bool WithCachedResult(const Stmt *S, CheckFunction Function) { + // If the statement isn't in the cache, conservatively assume that + // it's not trivial until analysis completes. Insert false to the cache + // first to avoid infinite recursion. + auto [It, IsNew] = Cache.insert(std::make_pair(S, false)); + if (!IsNew) + return It->second; + bool Result = Function(); + Cache[S] = Result; + return Result; + } + public: using CacheTy = TrivialFunctionAnalysis::CacheTy; @@ -266,7 +280,7 @@ class TrivialFunctionAnalysisVisitor bool VisitCompoundStmt(const CompoundStmt *CS) { // A compound statement is allowed as long each individual sub-statement // is trivial. - return VisitChildren(CS); + return WithCachedResult(CS, [&]() { return VisitChildren(CS); }); } bool VisitReturnStmt(const ReturnStmt *RS) { @@ -278,16 +292,36 @@ class TrivialFunctionAnalysisVisitor bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); } bool VisitDoStmt(const DoStmt *DS) { return VisitChildren(DS); } - bool VisitIfStmt(const IfStmt *IS) { return VisitChildren(IS); } + bool VisitIfStmt(const IfStmt *IS) { + return WithCachedResult(IS, [&]() { return VisitChildren(IS); }); + } + bool VisitForStmt(const ForStmt *FS) { + return WithCachedResult(FS, [&]() { return VisitChildren(FS); }); + } + bool VisitCXXForRangeStmt(const CXXForRangeStmt *FS) { + return WithCachedResult(FS, [&]() { return VisitChildren(FS); }); + } + bool VisitWhileStmt(const WhileStmt *WS) { + return WithCachedResult(WS, [&]() { return VisitChildren(WS); }); + } bool VisitSwitchStmt(const SwitchStmt *SS) { return VisitChildren(SS); } bool VisitCaseStmt(const CaseStmt *CS) { return VisitChildren(CS); } bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); } bool VisitUnaryOperator(const UnaryOperator *UO) { // Operator '*' and '!' are allowed as long as the operand is trivial. - if (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_LNot) + auto op = UO->getOpcode(); + if (op == UO_Deref || op == UO_AddrOf || op == UO_LNot) return Visit(UO->getSubExpr()); + if (UO->isIncrementOp() || UO->isDecrementOp()) { + // Allow increment or decrement of a POD type. + if (auto *RefExpr = dyn_cast(UO->getSubExpr())) { + if (auto *Decl = dyn_cast(RefExpr->getDecl())) + return Decl->isLocalVarDeclOrParm() && + Decl->getType().isPODType(Decl->getASTContext()); + } + } // Other operators are non-trivial. return false; } @@ -302,13 +336,7 @@ class TrivialFunctionAnalysisVisitor return VisitChildren(CO); } - bool VisitDeclRefExpr(const DeclRefExpr *DRE) { - if (auto *decl = DRE->getDecl()) { - if (isa(decl)) - return true; - } - return false; - } + bool VisitAtomicExpr(const AtomicExpr *E) { return VisitChildren(E); } bool VisitStaticAssertDecl(const StaticAssertDecl *SAD) { // Any static_assert is considered trivial. @@ -325,12 +353,18 @@ class TrivialFunctionAnalysisVisitor const auto &Name = safeGetName(Callee); if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" || + Name == "WTFReportAssertionFailure" || Name == "compilerFenceForCrash" || Name == "__builtin_unreachable") return true; return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache); } + bool VisitPredefinedExpr(const PredefinedExpr *E) { + // A predefined identifier such as "func" is considered trivial. + return true; + } + bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) { if (!checkArguments(MCE)) return false; @@ -351,6 +385,14 @@ class TrivialFunctionAnalysisVisitor return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache); } + bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) { + if (auto *Expr = E->getExpr()) { + if (!Visit(Expr)) + return false; + } + return true; + } + bool checkArguments(const CallExpr *CE) { for (const Expr *Arg : CE->arguments()) { if (Arg && !Visit(Arg)) @@ -377,6 +419,14 @@ class TrivialFunctionAnalysisVisitor return Visit(ECE->getSubExpr()); } + bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *VMT) { + return Visit(VMT->getSubExpr()); + } + + bool VisitExprWithCleanups(const ExprWithCleanups *EWC) { + return Visit(EWC->getSubExpr()); + } + bool VisitParenExpr(const ParenExpr *PE) { return Visit(PE->getSubExpr()); } bool VisitInitListExpr(const InitListExpr *ILE) { @@ -397,6 +447,16 @@ class TrivialFunctionAnalysisVisitor return true; } + bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E) { + // nullptr is trivial. + return true; + } + + bool VisitDeclRefExpr(const DeclRefExpr *DRE) { + // The use of a variable is trivial. + return true; + } + // Constant literal expressions are always trivial bool VisitIntegerLiteral(const IntegerLiteral *E) { return true; } bool VisitFloatingLiteral(const FloatingLiteral *E) { return true; } @@ -410,7 +470,7 @@ class TrivialFunctionAnalysisVisitor } private: - CacheTy Cache; + CacheTy &Cache; }; bool TrivialFunctionAnalysis::isTrivialImpl( @@ -435,4 +495,17 @@ bool TrivialFunctionAnalysis::isTrivialImpl( return Result; } +bool TrivialFunctionAnalysis::isTrivialImpl( + const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) { + // If the statement isn't in the cache, conservatively assume that + // it's not trivial until analysis completes. Unlike a function case, + // we don't insert an entry into the cache until Visit returns + // since Visit* functions themselves make use of the cache. + + TrivialFunctionAnalysisVisitor V(Cache); + bool Result = V.Visit(S); + assert(Cache.contains(S) && "Top-level statement not properly cached!"); + return Result; +} + } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index e07cd31395747..9ed8e7cab6abb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -11,6 +11,7 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/PointerUnion.h" #include namespace clang { @@ -19,6 +20,7 @@ class CXXMethodDecl; class CXXRecordDecl; class Decl; class FunctionDecl; +class Stmt; class Type; // Ref-countability of a type is implicitly defined by Ref and RefPtr @@ -71,14 +73,17 @@ class TrivialFunctionAnalysis { public: /// \returns true if \p D is a "trivial" function. bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); } + bool isTrivial(const Stmt *S) const { return isTrivialImpl(S, TheCache); } private: friend class TrivialFunctionAnalysisVisitor; - using CacheTy = llvm::DenseMap; + using CacheTy = + llvm::DenseMap, bool>; mutable CacheTy TheCache{}; static bool isTrivialImpl(const Decl *D, CacheTy &Cache); + static bool isTrivialImpl(const Stmt *S, CacheTy &Cache); }; } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 31d05ee880a13..5a5d48b109f1a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -74,6 +74,11 @@ class UncountedCallArgsChecker unsigned ArgIdx = isa(CE) && isa_and_nonnull(F); if (auto *MemberCallExpr = dyn_cast(CE)) { + if (auto *MD = MemberCallExpr->getMethodDecl()) { + auto name = safeGetName(MD); + if (name == "ref" || name == "deref") + return; + } auto *E = MemberCallExpr->getImplicitObjectArgument(); QualType ArgType = MemberCallExpr->getObjectType(); std::optional IsUncounted = @@ -166,6 +171,9 @@ class UncountedCallArgsChecker if (!Callee) return false; + if (isMethodOnWTFContainerType(Callee)) + return true; + auto overloadedOperatorType = Callee->getOverloadedOperator(); if (overloadedOperatorType == OO_EqualEqual || overloadedOperatorType == OO_ExclaimEqual || @@ -194,6 +202,31 @@ class UncountedCallArgsChecker return false; } + bool isMethodOnWTFContainerType(const FunctionDecl *Decl) const { + if (!isa(Decl)) + return false; + auto *ClassDecl = Decl->getParent(); + if (!ClassDecl || !isa(ClassDecl)) + return false; + + auto *NsDecl = ClassDecl->getParent(); + if (!NsDecl || !isa(NsDecl)) + return false; + + auto MethodName = safeGetName(Decl); + auto ClsNameStr = safeGetName(ClassDecl); + StringRef ClsName = ClsNameStr; // FIXME: Make safeGetName return StringRef. + auto NamespaceName = safeGetName(NsDecl); + // FIXME: These should be implemented via attributes. + return NamespaceName == "WTF" && + (MethodName == "find" || MethodName == "findIf" || + MethodName == "reverseFind" || MethodName == "reverseFindIf" || + MethodName == "get" || MethodName == "inlineGet" || + MethodName == "contains" || MethodName == "containsIf") && + (ClsName.ends_with("Vector") || ClsName.ends_with("Set") || + ClsName.ends_with("Map")); + } + void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const { assert(CallArg); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp index fa74759349810..4caaa17f81647 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp @@ -27,28 +27,6 @@ using namespace ento; namespace { -// for ( int a = ...) ... true -// for ( int a : ...) ... true -// if ( int* a = ) ... true -// anything else ... false -bool isDeclaredInForOrIf(const VarDecl *Var) { - assert(Var); - auto &ASTCtx = Var->getASTContext(); - auto parent = ASTCtx.getParents(*Var); - - if (parent.size() == 1) { - if (auto *DS = parent.begin()->get()) { - DynTypedNodeList grandParent = ASTCtx.getParents(*DS); - if (grandParent.size() == 1) { - return grandParent.begin()->get() || - grandParent.begin()->get() || - grandParent.begin()->get(); - } - } - } - return false; -} - // FIXME: should be defined by anotations in the future bool isRefcountedStringsHack(const VarDecl *V) { assert(V); @@ -144,6 +122,11 @@ class UncountedLocalVarsChecker // want to visit those, so we make our own RecursiveASTVisitor. struct LocalVisitor : public RecursiveASTVisitor { const UncountedLocalVarsChecker *Checker; + + TrivialFunctionAnalysis TFA; + + using Base = RecursiveASTVisitor; + explicit LocalVisitor(const UncountedLocalVarsChecker *Checker) : Checker(Checker) { assert(Checker); @@ -156,6 +139,36 @@ class UncountedLocalVarsChecker Checker->visitVarDecl(V); return true; } + + bool TraverseIfStmt(IfStmt *IS) { + if (!TFA.isTrivial(IS)) + return Base::TraverseIfStmt(IS); + return true; + } + + bool TraverseForStmt(ForStmt *FS) { + if (!TFA.isTrivial(FS)) + return Base::TraverseForStmt(FS); + return true; + } + + bool TraverseCXXForRangeStmt(CXXForRangeStmt *FRS) { + if (!TFA.isTrivial(FRS)) + return Base::TraverseCXXForRangeStmt(FRS); + return true; + } + + bool TraverseWhileStmt(WhileStmt *WS) { + if (!TFA.isTrivial(WS)) + return Base::TraverseWhileStmt(WS); + return true; + } + + bool TraverseCompoundStmt(CompoundStmt *CS) { + if (!TFA.isTrivial(CS)) + return Base::TraverseCompoundStmt(CS); + return true; + } }; LocalVisitor visitor(this); @@ -190,18 +203,16 @@ class UncountedLocalVarsChecker dyn_cast_or_null(Ref->getFoundDecl())) { const auto *MaybeGuardianArgType = MaybeGuardian->getType().getTypePtr(); - if (!MaybeGuardianArgType) - return; - const CXXRecordDecl *const MaybeGuardianArgCXXRecord = - MaybeGuardianArgType->getAsCXXRecordDecl(); - if (!MaybeGuardianArgCXXRecord) - return; - - if (MaybeGuardian->isLocalVarDecl() && - (isRefCounted(MaybeGuardianArgCXXRecord) || - isRefcountedStringsHack(MaybeGuardian)) && - isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian)) { - return; + if (MaybeGuardianArgType) { + const CXXRecordDecl *const MaybeGuardianArgCXXRecord = + MaybeGuardianArgType->getAsCXXRecordDecl(); + if (MaybeGuardianArgCXXRecord) { + if (MaybeGuardian->isLocalVarDecl() && + (isRefCounted(MaybeGuardianArgCXXRecord) || + isRefcountedStringsHack(MaybeGuardian)) && + isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian)) + return; + } } // Parameters are guaranteed to be safe for the duration of the call @@ -220,9 +231,6 @@ class UncountedLocalVarsChecker if (!V->isLocalVarDecl()) return true; - if (isDeclaredInForOrIf(V)) - return true; - return false; } diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp index 1c4b3df211b1e..6a8b7464845a2 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp @@ -5,12 +5,14 @@ class RefCounted { public: - void ref(); - void deref(); + void ref() const; + void deref() const; }; class Object { public: + void ref() const; + void deref() const; void someFunction(RefCounted&); }; diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp new file mode 100644 index 0000000000000..0a63a78985612 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp @@ -0,0 +1,146 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +namespace WTF { + + template + class HashSet { + public: + template T* find(U&) const; + template bool contains(U&) const; + unsigned size() { return m_size; } + template void add(U&) const; + template void remove(U&) const; + + private: + T* m_table { nullptr }; + unsigned m_size { 0 }; + }; + + template + class HashMap { + public: + struct Item { + T key; + S value; + }; + + template Item* find(U&) const; + template bool contains(U&) const; + template S* get(U&) const; + template S* inlineGet(U&) const; + template void add(U&) const; + template void remove(U&) const; + + private: + Item* m_table { nullptr }; + }; + + template + class WeakHashSet { + public: + template T* find(U&) const; + template bool contains(U&) const; + template void add(U&) const; + template void remove(U&) const; + }; + + template + class Vector { + public: + unsigned size() { return m_size; } + T& at(unsigned i) { return m_buffer[i]; } + T& operator[](unsigned i) { return m_buffer[i]; } + template unsigned find(U&); + template unsigned reverseFind(U&); + template bool contains(U&); + template unsigned findIf(const MatchFunction& match) + { + for (unsigned i = 0; i < m_size; ++i) { + if (match(at(i))) + return i; + } + return static_cast(-1); + } + template unsigned reverseFindIf(const MatchFunction& match) + { + for (unsigned i = 0; i < m_size; ++i) { + if (match(at(m_size - i))) + return i; + } + return static_cast(-1); + } + template bool containsIf(const MatchFunction& match) + { + for (unsigned i = 0; i < m_size; ++i) { + if (match(at(m_size - i))) + return true; + } + return false; + } + template void append(U&) const; + template void remove(U&) const; + + private: + T* m_buffer { nullptr }; + unsigned m_size { 0 }; + }; + +} + +using WTF::HashSet; +using WTF::HashMap; +using WTF::WeakHashSet; +using WTF::Vector; + +class RefCounted { +public: + void ref() const; + void deref() const; +}; + +RefCounted* object(); + +void test() { + HashSet> set; + set.find(*object()); + set.contains(*object()); + set.add(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + set.remove(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + + HashMap, unsigned> map; + map.find(*object()); + map.contains(*object()); + map.inlineGet(*object()); + map.add(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + map.remove(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + + WeakHashSet> weakSet; + weakSet.find(*object()); + weakSet.contains(*object()); + weakSet.add(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + weakSet.remove(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + + Vector> vector; + vector.at(0); + vector[0]; + vector.find(*object()); + vector.reverseFind(*object()); + vector.contains(*object()); + vector.append(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + vector.remove(*object()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + + auto* obj = object(); + vector.findIf([&](Ref key) { return key.ptr() == obj; }); + vector.reverseFindIf([&](Ref key) { return key.ptr() == obj; }); + vector.containsIf([&](Ref key) { return key.ptr() == obj; }); +} \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index cc40487614a83..aab99197dfa49 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -5,9 +5,18 @@ template struct Ref { T *t; Ref() : t{} {}; - Ref(T *) {} + Ref(T &t) + : t(t) { + if (t) + t->ref(); + } + ~Ref() { + if (t) + t->deref(); + } T *get() { return t; } T *ptr() { return t; } + T *operator->() { return t; } operator const T &() const { return *t; } operator T &() { return *t; } }; @@ -16,9 +25,18 @@ template struct RefPtr { T *t; RefPtr() : t(new T) {} - RefPtr(T *t) : t(t) {} + RefPtr(T *t) + : t(t) { + if (t) + t->ref(); + } + ~RefPtr() { + if (t) + t->deref(); + } T *get() { return t; } T *operator->() { return t; } + const T *operator->() const { return t; } T &operator*() { return *t; } RefPtr &operator=(T *) { return *this; } operator bool() { return t; } @@ -44,6 +62,8 @@ struct RefCountable { static Ref create(); void ref() {} void deref() {} + void method(); + int trivial() { return 123; } }; template T *downcast(T *t) { return t; } diff --git a/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp b/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp new file mode 100644 index 0000000000000..6d96c14102a90 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/ref-allowing-partially-destroyed.cpp @@ -0,0 +1,44 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s +// expected-no-diagnostics + +#include "mock-types.h" + +template struct RefAllowingPartiallyDestroyed { + T *t; + + RefAllowingPartiallyDestroyed() : t{} {}; + RefAllowingPartiallyDestroyed(T &) {} + T *get() { return t; } + T *ptr() { return t; } + T *operator->() { return t; } + operator const T &() const { return *t; } + operator T &() { return *t; } +}; + +template struct RefPtrAllowingPartiallyDestroyed { + T *t; + + RefPtrAllowingPartiallyDestroyed() : t(new T) {} + RefPtrAllowingPartiallyDestroyed(T *t) : t(t) {} + T *get() { return t; } + T *operator->() { return t; } + const T *operator->() const { return t; } + T &operator*() { return *t; } + RefPtrAllowingPartiallyDestroyed &operator=(T *) { return *this; } + operator bool() { return t; } +}; + +class RefCounted { +public: + void ref() const; + void deref() const; + void someFunction(); +}; + +RefAllowingPartiallyDestroyed object1(); +RefPtrAllowingPartiallyDestroyed object2(); + +void testFunction() { + object1()->someFunction(); + object2()->someFunction(); +} diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 0fcd3b21376ca..00673e91f471e 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -2,6 +2,8 @@ #include "mock-types.h" +void someFunction(); + namespace raw_ptr { void foo() { RefCountable *bar; @@ -16,6 +18,13 @@ void foo_ref() { RefCountable automatic; RefCountable &bar = automatic; // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + someFunction(); + bar.method(); +} + +void foo_ref_trivial() { + RefCountable automatic; + RefCountable &bar = automatic; } void bar_ref(RefCountable &) {} @@ -32,6 +41,8 @@ void foo2() { // missing embedded scope here RefCountable *bar = foo.get(); // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + someFunction(); + bar->method(); } void foo3() { @@ -47,11 +58,35 @@ void foo4() { { RefCountable *bar = foo.get(); } } } + +void foo5() { + RefPtr foo; + auto* bar = foo.get(); + bar->trivial(); +} + +void foo6() { + RefPtr foo; + auto* bar = foo.get(); + // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + bar->method(); +} + +struct SelfReferencingStruct { + SelfReferencingStruct* ptr; + RefCountable* obj { nullptr }; +}; + +void foo7(RefCountable* obj) { + SelfReferencingStruct bar = { &bar, obj }; + bar.obj->method(); +} + } // namespace guardian_scopes namespace auto_keyword { class Foo { - RefCountable *provide_ref_ctnbl() { return nullptr; } + RefCountable *provide_ref_ctnbl(); void evil_func() { RefCountable *bar = provide_ref_ctnbl(); @@ -62,13 +97,24 @@ class Foo { // expected-warning@-1{{Local variable 'baz2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} [[clang::suppress]] auto *baz_suppressed = provide_ref_ctnbl(); // no-warning } + + void func() { + RefCountable *bar = provide_ref_ctnbl(); + // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + if (bar) + bar->method(); + } }; } // namespace auto_keyword namespace guardian_casts { void foo1() { RefPtr foo; - { RefCountable *bar = downcast(foo.get()); } + { + RefCountable *bar = downcast(foo.get()); + bar->method(); + } + foo->method(); } void foo2() { @@ -76,6 +122,7 @@ void foo2() { { RefCountable *bar = static_cast(downcast(foo.get())); + someFunction(); } } } // namespace guardian_casts @@ -83,7 +130,11 @@ void foo2() { namespace guardian_ref_conversion_operator { void foo() { Ref rc; - { RefCountable &rr = rc; } + { + RefCountable &rr = rc; + rr.method(); + someFunction(); + } } } // namespace guardian_ref_conversion_operator @@ -92,9 +143,47 @@ RefCountable *provide_ref_ctnbl() { return nullptr; } void foo() { // no warnings - if (RefCountable *a = provide_ref_ctnbl()) { } - for (RefCountable *a = provide_ref_ctnbl(); a != nullptr;) { } + if (RefCountable *a = provide_ref_ctnbl()) + a->trivial(); + for (RefCountable *b = provide_ref_ctnbl(); b != nullptr;) + b->trivial(); RefCountable *array[1]; - for (RefCountable *a : array) { } + for (RefCountable *c : array) + c->trivial(); + while (RefCountable *d = provide_ref_ctnbl()) + d->trivial(); + do { + RefCountable *e = provide_ref_ctnbl(); + e->trivial(); + } while (1); + someFunction(); } + +void bar() { + if (RefCountable *a = provide_ref_ctnbl()) { + // expected-warning@-1{{Local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + a->method(); + } + for (RefCountable *b = provide_ref_ctnbl(); b != nullptr;) { + // expected-warning@-1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + b->method(); + } + RefCountable *array[1]; + for (RefCountable *c : array) { + // expected-warning@-1{{Local variable 'c' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + c->method(); + } + + while (RefCountable *d = provide_ref_ctnbl()) { + // expected-warning@-1{{Local variable 'd' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + d->method(); + } + do { + RefCountable *e = provide_ref_ctnbl(); + // expected-warning@-1{{Local variable 'e' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + e->method(); + } while (1); + someFunction(); +} + } // namespace ignore_for_if diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index 156a2480901bf..8aee51f6e5fc8 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -1,10 +1,10 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s #include "mock-types.h" -//#include void WTFBreakpointTrap(); void WTFCrashWithInfo(int, const char*, const char*, int); +void WTFReportAssertionFailure(const char* file, int line, const char* function, const char* assertion); inline void compilerFenceForCrash() { @@ -32,21 +32,19 @@ void isIntegralOrPointerType(T, Types... types) CRASH_WITH_INFO(__VA_ARGS__); \ } while (0) -#if !defined(NOT_TAIL_CALLED) -#if __has_attribute(not_tail_called) -#define NOT_TAIL_CALLED __attribute__((not_tail_called)) -#else -#define NOT_TAIL_CALLED -#endif -#endif -#define NO_RETURN_DUE_TO_CRASH +#define ASSERT(assertion, ...) do { \ + if (!(assertion)) { \ + WTFReportAssertionFailure(__FILE__, __LINE__, __PRETTY_FUNCTION__, #assertion); \ + CRASH_WITH_INFO(__VA_ARGS__); \ + } \ +} while (0) #if !defined(ALWAYS_INLINE) #define ALWAYS_INLINE inline #endif -NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfoImpl(int line, const char* file, const char* function, int counter, unsigned long reason); -NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfo(int line, const char* file, const char* function, int counter); +void WTFCrashWithInfoImpl(int line, const char* file, const char* function, int counter, unsigned long reason); +void WTFCrashWithInfo(int line, const char* file, const char* function, int counter); template ALWAYS_INLINE unsigned long wtfCrashArg(T* arg) { return reinterpret_cast(arg); } @@ -55,16 +53,91 @@ template ALWAYS_INLINE unsigned long wtfCrashArg(T arg) { return arg; } template -NO_RETURN_DUE_TO_CRASH ALWAYS_INLINE void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, T reason) +void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, T reason) { WTFCrashWithInfoImpl(line, file, function, counter, wtfCrashArg(reason)); } +enum class Flags : unsigned short { + Flag1 = 1 << 0, + Flag2 = 1 << 1, + Flag3 = 1 << 2, +}; + +template class OptionSet { +public: + using StorageType = unsigned short; + + static constexpr OptionSet fromRaw(StorageType rawValue) { + return OptionSet(static_cast(rawValue), FromRawValue); + } + + constexpr OptionSet() = default; + + constexpr OptionSet(E e) + : m_storage(static_cast(e)) { + } + + constexpr StorageType toRaw() const { return m_storage; } + + constexpr bool isEmpty() const { return !m_storage; } + + constexpr explicit operator bool() const { return !isEmpty(); } + + constexpr bool contains(E option) const { return containsAny(option); } + constexpr bool containsAny(OptionSet optionSet) const { + return !!(*this & optionSet); + } + + constexpr bool containsAll(OptionSet optionSet) const { + return (*this & optionSet) == optionSet; + } + + constexpr void add(OptionSet optionSet) { m_storage |= optionSet.m_storage; } + + constexpr void remove(OptionSet optionSet) + { + m_storage &= ~optionSet.m_storage; + } + + constexpr void set(OptionSet optionSet, bool value) + { + if (value) + add(optionSet); + else + remove(optionSet); + } + + constexpr friend OptionSet operator|(OptionSet lhs, OptionSet rhs) { + return fromRaw(lhs.m_storage | rhs.m_storage); + } + + constexpr friend OptionSet operator&(OptionSet lhs, OptionSet rhs) { + return fromRaw(lhs.m_storage & rhs.m_storage); + } + + constexpr friend OptionSet operator-(OptionSet lhs, OptionSet rhs) { + return fromRaw(lhs.m_storage & ~rhs.m_storage); + } + + constexpr friend OptionSet operator^(OptionSet lhs, OptionSet rhs) { + return fromRaw(lhs.m_storage ^ rhs.m_storage); + } + +private: + enum InitializationTag { FromRawValue }; + constexpr OptionSet(E e, InitializationTag) + : m_storage(static_cast(e)) { + } + StorageType m_storage { 0 }; +}; + class Number { public: Number(int v) : v(v) { } Number(double); Number operator+(const Number&); + const int& value() const { return v; } private: int v; }; @@ -112,6 +185,23 @@ class RefCounted { RefCounted& trivial18() const { RELEASE_ASSERT(this, "this must be not null"); return const_cast(*this); } void trivial19() const { return; } + static constexpr unsigned numBits = 4; + int trivial20() { return v >> numBits; } + + const int* trivial21() { return number ? &number->value() : nullptr; } + + enum class Enum : unsigned short { + Value1 = 1, + Value2 = 2, + }; + bool trivial22() { return enumValue == Enum::Value1; } + + bool trivial23() const { return OptionSet::fromRaw(v).contains(Flags::Flag1); } + int trivial24() const { ASSERT(v); return v; } + unsigned trivial25() const { return __c11_atomic_load((volatile _Atomic(unsigned) *)&v, __ATOMIC_RELAXED); } + bool trivial26() { bool hasValue = v; return !hasValue; } + bool trivial27(int v) { bool value; value = v ? 1 : 0; return value; } + static RefCounted& singleton() { static RefCounted s_RefCounted; s_RefCounted.ref(); @@ -169,7 +259,23 @@ class RefCounted { } } + static unsigned* another(); + unsigned nonTrivial10() const { + return __c11_atomic_load((volatile _Atomic(unsigned) *)another(), __ATOMIC_RELAXED); + } + + void nonTrivial11() { + Number num(0.3); + } + + bool nonTrivial12() { + bool val = otherFunction(); + return val; + } + unsigned v { 0 }; + Number* number { nullptr }; + Enum enumValue { Enum::Value1 }; }; RefCounted* refCountedObj(); @@ -208,6 +314,14 @@ class UnrelatedClass { getFieldTrivial().trivial17(); // no-warning getFieldTrivial().trivial18(); // no-warning getFieldTrivial().trivial19(); // no-warning + getFieldTrivial().trivial20(); // no-warning + getFieldTrivial().trivial21(); // no-warning + getFieldTrivial().trivial22(); // no-warning + getFieldTrivial().trivial23(); // no-warning + getFieldTrivial().trivial24(); // no-warning + getFieldTrivial().trivial25(); // no-warning + getFieldTrivial().trivial26(); // no-warning + getFieldTrivial().trivial27(5); // no-warning RefCounted::singleton().trivial18(); // no-warning RefCounted::singleton().someFunction(); // no-warning @@ -231,6 +345,12 @@ class UnrelatedClass { // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} getFieldTrivial().nonTrivial9(); // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + getFieldTrivial().nonTrivial10(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + getFieldTrivial().nonTrivial11(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + getFieldTrivial().nonTrivial12(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} } }; @@ -249,3 +369,10 @@ class UnrelatedClass2 { // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} } }; + +RefPtr object(); +void someFunction(const RefCounted&); + +void test2() { + someFunction(*object()); +}