https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/171764
>From ef9a5c9e9593fdb1fc0f414ba8f73be108f46c1a Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Wed, 10 Dec 2025 21:21:11 -0800 Subject: [PATCH 1/6] [alpha.webkit.UncountedLocalVarsChecker] Ignore a VarDecl in "if" with trivial "then" Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial. --- .../WebKit/RawPtrRefLocalVarsChecker.cpp | 4 ++ .../local-vars-checked-const-member.cpp | 4 +- .../local-vars-counted-const-member.cpp | 4 +- .../Checkers/WebKit/uncounted-local-vars.cpp | 40 ++++++++++++++++++- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index f2235e7c25ab2..404ec0783dd64 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -234,6 +234,10 @@ class RawPtrRefLocalVarsChecker } bool TraverseIfStmt(IfStmt *IS) override { + if (IS->getConditionVariable()) { + if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then)) + return true; + } if (!TFA.isTrivial(IS)) return DynamicRecursiveASTVisitor::TraverseIfStmt(IS); return true; diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp index be04cf26be2e8..bf58e676f3bb0 100644 --- a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp @@ -21,10 +21,8 @@ class Foo { CheckedObj& ensureObj4() { if (!m_obj4) const_cast<CheckedPtr<CheckedObj>&>(m_obj4) = new CheckedObj; - if (auto* next = m_obj4->next()) { - // expected-warning@-1{{Local variable 'next' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}} + if (auto* next = m_obj4->next()) return *next; - } return *m_obj4; } diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp index e12c9b900a423..5f35535bde3aa 100644 --- a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp @@ -21,10 +21,8 @@ class Foo { RefCountable& ensureObj4() { if (!m_obj4) const_cast<RefPtr<RefCountable>&>(m_obj4) = RefCountable::create(); - if (auto* next = m_obj4->next()) { - // expected-warning@-1{{Local variable 'next' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + if (auto* next = m_obj4->next()) return *next; - } return *m_obj4; } diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 3364637369799..bf093dcf3fdd3 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -492,4 +492,42 @@ namespace virtual_function { auto* baz = &*obj; // expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} } -} \ No newline at end of file +} + +namespace vardecl_in_if_condition { + RefCountable* provide(); + + RefCountable* get() { + if (auto* obj = provide()) + return obj; // no warning + return nullptr; + } + + RefCountable* get_non_trivial_then() { + if (auto* obj = provide()) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return obj->next(); + return nullptr; + } + + RefCountable* get_non_trivial_else() { + if (auto* obj = provide()) + return obj; + else + return provide()->next(); + return nullptr; + } + + RefCountable& get_ref() { + if (auto* obj = provide()) + return *obj; // no warning + static Ref<RefCountable> empty = RefCountable::create(); + return empty.get(); + } + + RefCountable* get_non_trivial_condition() { + if (auto* obj = provide(); obj && obj->next()) + return obj; // expected-warning@-1{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return nullptr; + } + +} >From f65b25fabc65b1fdf72fcee859ff25c37bcd0c09 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Thu, 11 Dec 2025 08:01:38 -0800 Subject: [PATCH 2/6] Add one more test case per a review comment --- .../test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index bf093dcf3fdd3..e1536be0b53c9 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -530,4 +530,11 @@ namespace vardecl_in_if_condition { return nullptr; } + RefCountable* get_non_trivial_else() { + if (auto* obj = provide(); !obj) // expected-warning@{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return nullptr; + else + return obj; + } + } >From 08a0a84dc825fdf3191f9800800b62c0561a8078 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Thu, 11 Dec 2025 08:03:20 -0800 Subject: [PATCH 3/6] Fix the last test case so that else is non-trivial. --- clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index e1536be0b53c9..1abb7d63984ba 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -534,7 +534,7 @@ namespace vardecl_in_if_condition { if (auto* obj = provide(); !obj) // expected-warning@{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} return nullptr; else - return obj; + return obj->next(); } } >From e3807bc1c277deaff88b47b87e45e82e064f9964 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Thu, 11 Dec 2025 08:17:44 -0800 Subject: [PATCH 4/6] Add a comment clarifying that the current code does not explicitly check "else" statement. --- .../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 404ec0783dd64..f720f8e15ed03 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -235,6 +235,10 @@ 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; } >From e23a6e3abc9417a9cd759b557bc9528ef1447479 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Fri, 12 Dec 2025 12:42:52 -0800 Subject: [PATCH 5/6] Fix the latest test case in uncounted-local-vars.cpp. --- clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 1abb7d63984ba..e8022b7fe8ba0 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -530,8 +530,8 @@ namespace vardecl_in_if_condition { return nullptr; } - RefCountable* get_non_trivial_else() { - if (auto* obj = provide(); !obj) // expected-warning@{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + RefCountable* get_non_trivial_else2() { + if (auto* obj = provide(); !obj) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} return nullptr; else return obj->next(); >From aa9a32154f1df680748be5a153a8ead99e8e10c7 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Fri, 12 Dec 2025 13:41:03 -0800 Subject: [PATCH 6/6] Fix the failing tests in debug builds --- .../Checkers/WebKit/PtrTypesSemantics.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 3c5d472720336..3bacc50bd6b58 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -573,6 +573,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. @@ -876,9 +885,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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
