Skip to content

Commit 8b2cf0b

Browse files
rniwarniwa
committed
[alpha.webkit.UncountedLocalVarsChecker] Allow uncounted object references within trivial statements
This PR makes alpha.webkit.UncountedLocalVarsChecker ignore raw references and pointers to a ref counted type which appears within "trival" statements. To do this, this PR extends TrivialFunctionAnalysis so that it can also analyze "triviality" of statements as well as that of functions Each Visit* function is now augmented with withCachedResult, which is responsible for looking up and updating the cache for each Visit* functions. As this PR dramatically improves the false positive rate of the checker, it also deletes the code to ignore raw pointers and references within if and for statements.
1 parent d9f9775 commit 8b2cf0b

File tree

5 files changed

+286
-119
lines changed

5 files changed

+286
-119
lines changed

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

Lines changed: 150 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -244,18 +244,41 @@ class TrivialFunctionAnalysisVisitor
244244

245245
// Returns false if at least one child is non-trivial.
246246
bool VisitChildren(const Stmt *S) {
247-
for (const Stmt *Child : S->children()) {
248-
if (Child && !Visit(Child))
247+
return withCachedResult(S, [&]() {
248+
for (const Stmt *Child : S->children()) {
249+
if (Child && !Visit(Child))
250+
return false;
251+
}
252+
return true;
253+
});
254+
}
255+
256+
bool VisitSubExpr(const Expr *Parent, const Expr *E) {
257+
return withCachedResult(Parent, [&]() {
258+
if (!Visit(E))
249259
return false;
250-
}
260+
return true;
261+
});
262+
}
251263

252-
return true;
264+
template <typename StmtType, typename CheckFunction>
265+
bool withCachedResult(const StmtType *S, CheckFunction Function) {
266+
// Insert false to the cache first to avoid infinite recursion.
267+
auto [It, IsNew] = StatementCache.insert(std::make_pair(S, false));
268+
if (!IsNew)
269+
return It->second;
270+
bool Result = Function();
271+
It->second = Result;
272+
return Result;
253273
}
254274

255275
public:
256-
using CacheTy = TrivialFunctionAnalysis::CacheTy;
276+
using FunctionCacheTy = TrivialFunctionAnalysis::FunctionCacheTy;
277+
using StatementCacheTy = TrivialFunctionAnalysis::StatementCacheTy;
257278

258-
TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {}
279+
TrivialFunctionAnalysisVisitor(FunctionCacheTy &FunctionCache,
280+
StatementCacheTy &StatementCache)
281+
: FunctionCache(FunctionCache), StatementCache(StatementCache) {}
259282

260283
bool VisitStmt(const Stmt *S) {
261284
// All statements are non-trivial unless overriden later.
@@ -271,31 +294,48 @@ class TrivialFunctionAnalysisVisitor
271294

272295
bool VisitReturnStmt(const ReturnStmt *RS) {
273296
// A return statement is allowed as long as the return value is trivial.
274-
if (auto *RV = RS->getRetValue())
275-
return Visit(RV);
276-
return true;
297+
return withCachedResult(RS, [&]() {
298+
if (auto *RV = RS->getRetValue())
299+
return Visit(RV);
300+
return true;
301+
});
302+
}
303+
304+
bool VisitCXXForRangeStmt(const CXXForRangeStmt *FS) {
305+
return VisitChildren(FS);
277306
}
278307

279308
bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); }
280309
bool VisitDoStmt(const DoStmt *DS) { return VisitChildren(DS); }
310+
bool VisitForStmt(const ForStmt *FS) { return VisitChildren(FS); }
311+
bool VisitWhileStmt(const WhileStmt *WS) { return VisitChildren(WS); }
281312
bool VisitIfStmt(const IfStmt *IS) { return VisitChildren(IS); }
282313
bool VisitSwitchStmt(const SwitchStmt *SS) { return VisitChildren(SS); }
283314
bool VisitCaseStmt(const CaseStmt *CS) { return VisitChildren(CS); }
284315
bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); }
285316

286317
bool VisitUnaryOperator(const UnaryOperator *UO) {
287318
// Operator '*' and '!' are allowed as long as the operand is trivial.
288-
if (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_AddrOf ||
289-
UO->getOpcode() == UO_LNot)
290-
return Visit(UO->getSubExpr());
291-
292-
// Other operators are non-trivial.
293-
return false;
319+
return withCachedResult(UO, [&]() {
320+
auto op = UO->getOpcode();
321+
if (op == UO_Deref || op == UO_AddrOf || op == UO_LNot)
322+
return Visit(UO->getSubExpr());
323+
if (UO->isIncrementOp() || UO->isDecrementOp()) {
324+
if (auto *RefExpr = dyn_cast<DeclRefExpr>(UO->getSubExpr())) {
325+
if (auto *Decl = dyn_cast<VarDecl>(RefExpr->getDecl()))
326+
return Decl->isLocalVarDeclOrParm() &&
327+
Decl->getType().isPODType(Decl->getASTContext());
328+
}
329+
}
330+
// Other operators are non-trivial.
331+
return false;
332+
});
294333
}
295334

296335
bool VisitBinaryOperator(const BinaryOperator *BO) {
297336
// Binary operators are trivial if their operands are trivial.
298-
return Visit(BO->getLHS()) && Visit(BO->getRHS());
337+
return withCachedResult(
338+
BO, [&]() { return Visit(BO->getLHS()) && Visit(BO->getRHS()); });
299339
}
300340

301341
bool VisitConditionalOperator(const ConditionalOperator *CO) {
@@ -304,15 +344,20 @@ class TrivialFunctionAnalysisVisitor
304344
}
305345

306346
bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
307-
if (auto *decl = DRE->getDecl()) {
308-
if (isa<ParmVarDecl>(decl))
309-
return true;
310-
if (isa<EnumConstantDecl>(decl))
311-
return true;
312-
if (auto *VD = dyn_cast<VarDecl>(decl))
313-
return VD->hasConstantInitialization() && VD->getEvaluatedValue();
314-
}
315-
return false;
347+
return withCachedResult(DRE, [&]() {
348+
if (auto *decl = DRE->getDecl()) {
349+
if (isa<ParmVarDecl>(decl))
350+
return true;
351+
if (isa<EnumConstantDecl>(decl))
352+
return true;
353+
if (auto *VD = dyn_cast<VarDecl>(decl)) {
354+
if (VD->hasConstantInitialization() && VD->getEvaluatedValue())
355+
return true;
356+
return VD->getInit() ? Visit(VD->getInit()) : true;
357+
}
358+
}
359+
return false;
360+
});
316361
}
317362

318363
bool VisitAtomicExpr(const AtomicExpr *E) { return VisitChildren(E); }
@@ -323,20 +368,23 @@ class TrivialFunctionAnalysisVisitor
323368
}
324369

325370
bool VisitCallExpr(const CallExpr *CE) {
326-
if (!checkArguments(CE))
327-
return false;
371+
return withCachedResult(CE, [&]() {
372+
if (!checkArguments(CE))
373+
return false;
328374

329-
auto *Callee = CE->getDirectCallee();
330-
if (!Callee)
331-
return false;
332-
const auto &Name = safeGetName(Callee);
375+
auto *Callee = CE->getDirectCallee();
376+
if (!Callee)
377+
return false;
378+
const auto &Name = safeGetName(Callee);
333379

334-
if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
335-
Name == "WTFReportAssertionFailure" ||
336-
Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
337-
return true;
380+
if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
381+
Name == "WTFReportAssertionFailure" ||
382+
Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
383+
return true;
338384

339-
return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
385+
return TrivialFunctionAnalysis::isTrivialImpl(Callee, FunctionCache,
386+
StatementCache);
387+
});
340388
}
341389

342390
bool VisitPredefinedExpr(const PredefinedExpr *E) {
@@ -345,23 +393,26 @@ class TrivialFunctionAnalysisVisitor
345393
}
346394

347395
bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
348-
if (!checkArguments(MCE))
349-
return false;
396+
return withCachedResult(MCE, [&]() {
397+
if (!checkArguments(MCE))
398+
return false;
350399

351-
bool TrivialThis = Visit(MCE->getImplicitObjectArgument());
352-
if (!TrivialThis)
353-
return false;
400+
bool TrivialThis = Visit(MCE->getImplicitObjectArgument());
401+
if (!TrivialThis)
402+
return false;
354403

355-
auto *Callee = MCE->getMethodDecl();
356-
if (!Callee)
357-
return false;
404+
auto *Callee = MCE->getMethodDecl();
405+
if (!Callee)
406+
return false;
358407

359-
std::optional<bool> IsGetterOfRefCounted = isGetterOfRefCounted(Callee);
360-
if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
361-
return true;
408+
std::optional<bool> IsGetterOfRefCounted = isGetterOfRefCounted(Callee);
409+
if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
410+
return true;
362411

363-
// Recursively descend into the callee to confirm that it's trivial as well.
364-
return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
412+
// Recursively descend into the callee to confirm it's trivial as well.
413+
return TrivialFunctionAnalysis::isTrivialImpl(Callee, FunctionCache,
414+
StatementCache);
415+
});
365416
}
366417

367418
bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) {
@@ -381,44 +432,51 @@ class TrivialFunctionAnalysisVisitor
381432
}
382433

383434
bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
384-
for (const Expr *Arg : CE->arguments()) {
385-
if (Arg && !Visit(Arg))
386-
return false;
387-
}
435+
return withCachedResult(CE, [&]() {
436+
for (const Expr *Arg : CE->arguments()) {
437+
if (Arg && !Visit(Arg))
438+
return false;
439+
}
388440

389-
// Recursively descend into the callee to confirm that it's trivial.
390-
return TrivialFunctionAnalysis::isTrivialImpl(CE->getConstructor(), Cache);
441+
// Recursively descend into the callee to confirm that it's trivial.
442+
return TrivialFunctionAnalysis::isTrivialImpl(
443+
CE->getConstructor(), FunctionCache, StatementCache);
444+
});
391445
}
392446

393447
bool VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
394-
return Visit(ICE->getSubExpr());
448+
return VisitSubExpr(ICE, ICE->getSubExpr());
395449
}
396450

397451
bool VisitExplicitCastExpr(const ExplicitCastExpr *ECE) {
398-
return Visit(ECE->getSubExpr());
452+
return VisitSubExpr(ECE, ECE->getSubExpr());
399453
}
400454

401455
bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *VMT) {
402-
return Visit(VMT->getSubExpr());
456+
return VisitSubExpr(VMT, VMT->getSubExpr());
403457
}
404458

405459
bool VisitExprWithCleanups(const ExprWithCleanups *EWC) {
406-
return Visit(EWC->getSubExpr());
460+
return VisitSubExpr(EWC, EWC->getSubExpr());
407461
}
408462

409-
bool VisitParenExpr(const ParenExpr *PE) { return Visit(PE->getSubExpr()); }
463+
bool VisitParenExpr(const ParenExpr *PE) {
464+
return VisitSubExpr(PE, PE->getSubExpr());
465+
}
410466

411467
bool VisitInitListExpr(const InitListExpr *ILE) {
412-
for (const Expr *Child : ILE->inits()) {
413-
if (Child && !Visit(Child))
414-
return false;
415-
}
416-
return true;
468+
return withCachedResult(ILE, [&]() {
469+
for (const Expr *Child : ILE->inits()) {
470+
if (Child && !Visit(Child))
471+
return false;
472+
}
473+
return true;
474+
});
417475
}
418476

419477
bool VisitMemberExpr(const MemberExpr *ME) {
420478
// Field access is allowed but the base pointer may itself be non-trivial.
421-
return Visit(ME->getBase());
479+
return VisitSubExpr(ME, ME->getBase());
422480
}
423481

424482
bool VisitCXXThisExpr(const CXXThisExpr *CTE) {
@@ -444,27 +502,48 @@ class TrivialFunctionAnalysisVisitor
444502
}
445503

446504
private:
447-
CacheTy Cache;
505+
FunctionCacheTy FunctionCache;
506+
StatementCacheTy StatementCache;
448507
};
449508

450509
bool TrivialFunctionAnalysis::isTrivialImpl(
451-
const Decl *D, TrivialFunctionAnalysis::CacheTy &Cache) {
510+
const Decl *D, TrivialFunctionAnalysis::FunctionCacheTy &FunctionCache,
511+
TrivialFunctionAnalysis::StatementCacheTy &StatementCache) {
452512
// If the function isn't in the cache, conservatively assume that
453513
// it's not trivial until analysis completes. This makes every recursive
454514
// function non-trivial. This also guarantees that each function
455515
// will be scanned at most once.
456-
auto [It, IsNew] = Cache.insert(std::make_pair(D, false));
516+
auto [It, IsNew] = FunctionCache.insert(std::make_pair(D, false));
457517
if (!IsNew)
458518
return It->second;
459519

460520
const Stmt *Body = D->getBody();
461521
if (!Body)
462522
return false;
463523

464-
TrivialFunctionAnalysisVisitor V(Cache);
524+
TrivialFunctionAnalysisVisitor V(FunctionCache, StatementCache);
465525
bool Result = V.Visit(Body);
466526
if (Result)
467-
Cache[D] = true;
527+
FunctionCache[D] = true;
528+
529+
return Result;
530+
}
531+
532+
bool TrivialFunctionAnalysis::isTrivialImpl(
533+
const Stmt *S, TrivialFunctionAnalysis::FunctionCacheTy &FunctionCache,
534+
TrivialFunctionAnalysis::StatementCacheTy &StatementCache) {
535+
// If the statement isn't in the cache, conservatively assume that
536+
// it's not trivial until analysis completes. Unlike a function case,
537+
// we don't insert an entry into the cache until Visit returns
538+
// since Visit* functions themselves make use of the cache.
539+
540+
auto It = StatementCache.find(S);
541+
if (It != StatementCache.end())
542+
return It->second;
543+
544+
TrivialFunctionAnalysisVisitor V(FunctionCache, StatementCache);
545+
bool Result = V.Visit(S);
546+
StatementCache[S] = Result;
468547

469548
return Result;
470549
}

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class CXXMethodDecl;
1919
class CXXRecordDecl;
2020
class Decl;
2121
class FunctionDecl;
22+
class Stmt;
2223
class Type;
2324

2425
// Ref-countability of a type is implicitly defined by Ref<T> and RefPtr<T>
@@ -70,15 +71,27 @@ bool isSingleton(const FunctionDecl *F);
7071
class TrivialFunctionAnalysis {
7172
public:
7273
/// \returns true if \p D is a "trivial" function.
73-
bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
74+
bool isTrivial(const Decl *D) const {
75+
return isTrivialImpl(D, TheFunctionCache, TheStatementCache);
76+
}
77+
78+
bool isTrivial(const Stmt *S) const {
79+
return isTrivialImpl(S, TheFunctionCache, TheStatementCache);
80+
}
7481

7582
private:
7683
friend class TrivialFunctionAnalysisVisitor;
7784

78-
using CacheTy = llvm::DenseMap<const Decl *, bool>;
79-
mutable CacheTy TheCache{};
85+
using FunctionCacheTy = llvm::DenseMap<const Decl *, bool>;
86+
mutable FunctionCacheTy TheFunctionCache{};
87+
88+
using StatementCacheTy = llvm::DenseMap<const Stmt *, bool>;
89+
mutable StatementCacheTy TheStatementCache{};
8090

81-
static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
91+
static bool isTrivialImpl(const Decl *D, FunctionCacheTy &FunctionCache,
92+
StatementCacheTy &StatementCache);
93+
static bool isTrivialImpl(const Stmt *S, FunctionCacheTy &FunctionCache,
94+
StatementCacheTy &StatementCache);
8295
};
8396

8497
} // namespace clang

0 commit comments

Comments
 (0)