llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> This PR fixes a bug in UncountedCallArgsChecker that calling a function with a member variable which is Ref/RefPtr is erroneously treated as safe by canoniclizing the type before checking whether it's ref counted or not. --- Full diff: https://github.com/llvm/llvm-project/pull/109393.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+1-1) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+5-4) - (added) clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp (+27) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 9da3e54e454317..54c99c3c1b37f9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -155,7 +155,7 @@ std::optional<bool> isUncounted(const QualType T) { std::optional<bool> isUncounted(const CXXRecordDecl* Class) { // Keep isRefCounted first as it's cheaper. - if (isRefCounted(Class)) + if (!Class || isRefCounted(Class)) return false; std::optional<bool> IsRefCountable = isRefCountable(Class); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index 81c2434ce64775..31e9b3c4b9d412 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -86,7 +86,7 @@ class UncountedCallArgsChecker return; } auto *E = MemberCallExpr->getImplicitObjectArgument(); - QualType ArgType = MemberCallExpr->getObjectType(); + QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType(); std::optional<bool> IsUncounted = isUncounted(ArgType); if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E)) reportBugOnThis(E); @@ -102,12 +102,13 @@ class UncountedCallArgsChecker // if ((*P)->hasAttr<SafeRefCntblRawPtrAttr>()) // continue; - const auto *ArgType = (*P)->getType().getTypePtrOrNull(); - if (!ArgType) + QualType ArgType = (*P)->getType().getCanonicalType(); + const auto *TypePtr = ArgType.getTypePtrOrNull(); + if (!TypePtr) continue; // FIXME? Should we bail? // FIXME: more complex types (arrays, references to raw pointers, etc) - std::optional<bool> IsUncounted = isUncountedPtr(ArgType); + std::optional<bool> IsUncounted = isUncountedPtr(TypePtr); if (!IsUncounted || !(*IsUncounted)) continue; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp new file mode 100644 index 00000000000000..2721cd8474e1b4 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +class Object { +public: + void ref() const; + void deref() const; + + bool constFunc() const; + void mutableFunc(); +}; + +class Caller { + void someFunction(); + void otherFunction(); +private: + RefPtr<Object> m_obj; +}; + +void Caller::someFunction() +{ + m_obj->constFunc(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + m_obj->mutableFunc(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/109393 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
