Skip to content

Commit e7887d2

Browse files
committed
[alpha.webkit.UncountedLocalVarsChecker] Ignore a VarDecl in "if" with trivial "then" (llvm#171764)
Don't emit a warning when a variable declaration appears within the "if" condition and if its "then" statement is trivial. (cherry picked from commit 6f1e3c3)
1 parent f2dcfda commit e7887d2

File tree

5 files changed

+66
-10
lines changed

5 files changed

+66
-10
lines changed

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,15 @@ class TrivialFunctionAnalysisVisitor
536536
});
537537
}
538538

539+
bool IsStatementTrivial(const Stmt *S) {
540+
auto CacheIt = Cache.find(S);
541+
if (CacheIt != Cache.end())
542+
return CacheIt->second;
543+
bool Result = Visit(S);
544+
Cache[S] = Result;
545+
return Result;
546+
}
547+
539548
bool VisitStmt(const Stmt *S) {
540549
// All statements are non-trivial unless overriden later.
541550
// Don't even recurse into children by default.
@@ -839,9 +848,7 @@ bool TrivialFunctionAnalysis::isTrivialImpl(
839848
bool TrivialFunctionAnalysis::isTrivialImpl(
840849
const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) {
841850
TrivialFunctionAnalysisVisitor V(Cache);
842-
bool Result = V.Visit(S);
843-
assert(Cache.contains(S) && "Top-level statement not properly cached!");
844-
return Result;
851+
return V.IsStatementTrivial(S);
845852
}
846853

847854
} // namespace clang

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,14 @@ class RawPtrRefLocalVarsChecker
234234
}
235235

236236
bool TraverseIfStmt(IfStmt *IS) override {
237+
if (IS->getConditionVariable()) {
238+
// This code currently does not explicitly check the "else" statement
239+
// since getConditionVariable returns nullptr when there is a
240+
// condition defined after ";" as in "if (auto foo = ~; !foo)". If
241+
// this semantics change, we should add an explicit check for "else".
242+
if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then))
243+
return true;
244+
}
237245
if (!TFA.isTrivial(IS))
238246
return DynamicRecursiveASTVisitor::TraverseIfStmt(IS);
239247
return true;

clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ class Foo {
2121
CheckedObj& ensureObj4() {
2222
if (!m_obj4)
2323
const_cast<CheckedPtr<CheckedObj>&>(m_obj4) = new CheckedObj;
24-
if (auto* next = m_obj4->next()) {
25-
// expected-warning@-1{{Local variable 'next' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
24+
if (auto* next = m_obj4->next())
2625
return *next;
27-
}
2826
return *m_obj4;
2927
}
3028

clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ class Foo {
2121
RefCountable& ensureObj4() {
2222
if (!m_obj4)
2323
const_cast<RefPtr<RefCountable>&>(m_obj4) = RefCountable::create();
24-
if (auto* next = m_obj4->next()) {
25-
// expected-warning@-1{{Local variable 'next' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
24+
if (auto* next = m_obj4->next())
2625
return *next;
27-
}
2826
return *m_obj4;
2927
}
3028

clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,4 +492,49 @@ namespace virtual_function {
492492
auto* baz = &*obj;
493493
// expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
494494
}
495-
}
495+
}
496+
497+
namespace vardecl_in_if_condition {
498+
RefCountable* provide();
499+
500+
RefCountable* get() {
501+
if (auto* obj = provide())
502+
return obj; // no warning
503+
return nullptr;
504+
}
505+
506+
RefCountable* get_non_trivial_then() {
507+
if (auto* obj = provide()) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
508+
return obj->next();
509+
return nullptr;
510+
}
511+
512+
RefCountable* get_non_trivial_else() {
513+
if (auto* obj = provide())
514+
return obj;
515+
else
516+
return provide()->next();
517+
return nullptr;
518+
}
519+
520+
RefCountable& get_ref() {
521+
if (auto* obj = provide())
522+
return *obj; // no warning
523+
static Ref<RefCountable> empty = RefCountable::create();
524+
return empty.get();
525+
}
526+
527+
RefCountable* get_non_trivial_condition() {
528+
if (auto* obj = provide(); obj && obj->next())
529+
return obj; // expected-warning@-1{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
530+
return nullptr;
531+
}
532+
533+
RefCountable* get_non_trivial_else2() {
534+
if (auto* obj = provide(); !obj) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
535+
return nullptr;
536+
else
537+
return obj->next();
538+
}
539+
540+
}

0 commit comments

Comments
 (0)