llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> GuardianVisitor treated `guardian->method()` as a mutation of the guardian because the implicit object argument of a member operator call had no corresponding parameter, and the check fell back to the non-const type of the guardian itself. Skip the implicit object argument when the operator is a const member function, and only apply the argument offset for implicit object member operators so that arguments of non-member operators map to the right parameters. Also treat passing a guardian to a const reference parameter as non-mutating by checking the constness of the referenced type instead of the reference type. --- Full diff: https://github.com/llvm/llvm-project/pull/230900.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp (+10-2) - (modified) clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp (+9) - (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+22) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 232cca1c7a2c4..b3ddbb858044c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -96,14 +96,22 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor { if (isGetterOfSafePtr(Method).value_or(false)) return true; } + // For a member operator call, the first argument is the implicit object. + auto *OperatorMethod = dyn_cast<CXXMethodDecl>(Callee); + if (!isa<CXXOperatorCallExpr>(CE) || !OperatorMethod || + !OperatorMethod->isImplicitObjectMemberFunction()) + OperatorMethod = nullptr; unsigned ArgIndex = 0; - unsigned ArgOffset = isa<CXXOperatorCallExpr>(CE); + unsigned ArgOffset = OperatorMethod ? 1 : 0; for (auto *Arg : CE->arguments()) { ParmVarDecl *Parm = nullptr; if (ArgIndex >= ArgOffset) { unsigned ParmIndex = ArgIndex - ArgOffset; if (ParmIndex < Callee->getNumParams()) Parm = Callee->getParamDecl(ParmIndex); + } else if (OperatorMethod->isConst()) { + ArgIndex++; + continue; } if (mutatesGuardian(Arg, Parm)) return false; @@ -152,7 +160,7 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor { return false; return !ArgType.getNonReferenceType().isConstQualified(); } - return !ArgType.isConstQualified(); + return !ArgType.getNonReferenceType().isConstQualified(); } }; diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp index a0ce0ffbff90d..a3f7d23328e52 100644 --- a/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/unchecked-local-vars.cpp @@ -360,6 +360,15 @@ void local_var_with_guardian_checked_ref() { } } +void local_var_with_guardian_used_via_arrow() { + Ref<RefCountableAndCheckable> a = *provide_obj(); + { + RefCountableAndCheckable* b = a.ptr(); + a->method(); + b->method(); + } +} + void static_var() { static RefCountableAndCheckable* a = nullptr; // expected-warning@-1{{Static local variable 'a' is a raw pointer to CheckedPtr-capable type 'RefCountableAndCheckable' [alpha.webkit.UncheckedLocalVarsChecker]}} diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 7f086f13f68e4..a7a5e9ae19746 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -214,6 +214,28 @@ void foo9(RefCountable& o) { } } +bool operator==(const Ref<RefCountable>&, const Ref<RefCountable>&); +void consumeConstRef(const Ref<RefCountable>&); + +void guardian_const_operator_call(RefCountable& o, Ref<RefCountable>& other) { + Ref<RefCountable> guardian(o); + { + RefCountable *bar = guardian.ptr(); + guardian->method(); + bar->method(); + } + { + RefCountable *bar = guardian.ptr(); + if (guardian == other) + bar->method(); + } + { + RefCountable *bar = guardian.ptr(); + consumeConstRef(guardian); + bar->method(); + } +} + RefCountable* provide(); struct Derived : public RefCountable { `````````` </details> https://github.com/llvm/llvm-project/pull/230900 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
