Skip to content

Commit 1f0d65a

Browse files
committed
[alpha.webkit.UncountedCallArgsChecker] Fix a false negative when a call argument is a local variable.
isASafeCallArg erroneously returns true when a call argument is a local variable regardless of its type. This is incorrect. We should only allow any local variable of a safe pointer type. Fix the bug by moving the logic to check for a function parameter and local variable from isASafeCallArg to a lambda in isPtrOriginSafe, and check that the local variable is a safe pointer type. Also fix a bug in isPtrOfType that it was not recognizing DeducedTemplateSpecializationType.
1 parent a085da6 commit 1f0d65a

File tree

4 files changed

+27
-9
lines changed

4 files changed

+27
-9
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ bool isASafeCallArg(const Expr *E) {
141141
assert(E);
142142
if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
143143
if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
144-
if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
144+
if (isa<ParmVarDecl>(D))
145145
return true;
146146
}
147147
}

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,16 @@ static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
162162
type = elaboratedT->desugar();
163163
continue;
164164
}
165-
auto *SpecialT = type->getAs<TemplateSpecializationType>();
166-
if (!SpecialT)
167-
return false;
168-
auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl();
169-
if (!Decl)
170-
return false;
171-
return Pred(Decl->getNameAsString());
165+
if (auto *SpecialT = type->getAs<TemplateSpecializationType>()) {
166+
auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl();
167+
if (!Decl)
168+
return false;
169+
return Pred(Decl->getNameAsString());
170+
} else if (auto *DTS = type->getAs<DeducedTemplateSpecializationType>()) {
171+
auto *Decl = DTS->getTemplateName().getAsTemplateDecl();
172+
return Pred(Decl->getNameAsString());
173+
} else
174+
break;
172175
}
173176
return false;
174177
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,14 @@ class RawPtrRefCallArgsChecker
158158
// foo(NULL)
159159
return true;
160160
}
161+
if (auto *Ref = dyn_cast<DeclRefExpr>(ArgOrigin)) {
162+
if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
163+
if (isa<ParmVarDecl>(D))
164+
return true; // Parameters are transitively safe.
165+
if (D->isLocalVarDecl() && isSafePtrType(D->getType()))
166+
return true;
167+
}
168+
}
161169
if (isASafeCallArg(ArgOrigin))
162170
return true;
163171
if (EFA.isACallToEnsureFn(ArgOrigin))

clang/test/Analysis/Checkers/WebKit/call-args.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,14 @@ namespace call_with_explicit_temporary_obj {
374374
}
375375
}
376376

377-
namespace call_with_explicit_construct {
377+
namespace call_via_local_var {
378+
RefCountable* provide();
379+
void bar(RefCountable*);
380+
void foo() {
381+
auto* obj = provide();
382+
bar(obj);
383+
// expected-warning@-1{{Call argument is uncounted and unsafe}}
384+
}
378385
}
379386

380387
namespace call_with_adopt_ref {

0 commit comments

Comments
 (0)