llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Fady Farag (iidmsa) <details> <summary>Changes</summary> Previously, when an `if` statement had a condition variable and a trivial then branch, `TraverseIfStmt` skipped the entire statement. That is correct for the condition variable, which is null in the else branch, but it also skipped the else branch, which caused a missing warning for any raw pointer/reference local variable declared there. This still exempts the condition variable but traverses the else branch when it is not trivial. --- Full diff: https://github.com/llvm/llvm-project/pull/227262.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp (+8-5) - (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+31) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index d0191bd0ccc62..232cca1c7a2c4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -339,12 +339,15 @@ 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)) + // This code does not check the condition variable in 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 check it in "else" as well. + if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then)) { + if (auto *Else = IS->getElse(); Else && !TFA.isTrivial(Else)) + return TraverseStmt(Else); return true; + } } if (!TFA.isTrivial(IS)) return DynamicRecursiveASTVisitor::TraverseIfStmt(IS); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 96ff48b9605b3..7f086f13f68e4 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -704,6 +704,37 @@ namespace vardecl_in_if_condition { return obj->next(); } + RefCountable* trivialProvide() { return nullptr; } + + void local_in_non_trivial_else() { + if (auto* obj = provide()) + obj->trivial(); + else { + auto* other = provide(); // expected-warning{{Local variable 'other' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + someFunction(); + other->method(); + } + } + + void local_in_non_trivial_else_if(bool flag) { + if (auto* obj = provide()) + obj->trivial(); + else if (flag) { + auto* other = provide(); // expected-warning{{Local variable 'other' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + someFunction(); + other->method(); + } + } + + void local_in_trivial_else() { + if (auto* obj = provide()) + obj->trivial(); + else { + auto* other = trivialProvide(); // no warning + other->trivial(); + } + } + } namespace delete_unresolved_type { `````````` </details> https://github.com/llvm/llvm-project/pull/227262 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
