llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/129974.diff 4 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+1-1) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+10-7) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp (+8) - (modified) clang/test/Analysis/Checkers/WebKit/call-args.cpp (+8-1) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index dc86c4fcc64b1..73b6afd3642c7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -141,7 +141,7 @@ bool isASafeCallArg(const Expr *E) { assert(E); if (auto *Ref = dyn_cast<DeclRefExpr>(E)) { if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) { - if (isa<ParmVarDecl>(D) || D->isLocalVarDecl()) + if (isa<ParmVarDecl>(D)) return true; } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 7899b19854806..cbb207bcb9df0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -162,13 +162,16 @@ static bool isPtrOfType(const clang::QualType T, Predicate Pred) { type = elaboratedT->desugar(); continue; } - auto *SpecialT = type->getAs<TemplateSpecializationType>(); - if (!SpecialT) - return false; - auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl(); - if (!Decl) - return false; - return Pred(Decl->getNameAsString()); + if (auto *SpecialT = type->getAs<TemplateSpecializationType>()) { + auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl(); + if (!Decl) + return false; + return Pred(Decl->getNameAsString()); + } else if (auto *DTS = type->getAs<DeducedTemplateSpecializationType>()) { + auto *Decl = DTS->getTemplateName().getAsTemplateDecl(); + return Pred(Decl->getNameAsString()); + } else + break; } return false; } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index d633fbcbd798b..fbb6a06afbdac 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -158,6 +158,14 @@ class RawPtrRefCallArgsChecker // foo(NULL) return true; } + if (auto *Ref = dyn_cast<DeclRefExpr>(ArgOrigin)) { + if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) { + if (isa<ParmVarDecl>(D)) + return true; // Parameters are transitively safe. + if (D->isLocalVarDecl() && isSafePtrType(D->getType())) + return true; + } + } if (isASafeCallArg(ArgOrigin)) return true; if (EFA.isACallToEnsureFn(ArgOrigin)) diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index b4613d5090f29..4fc5a574f9e69 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -374,7 +374,14 @@ namespace call_with_explicit_temporary_obj { } } -namespace call_with_explicit_construct { +namespace call_via_local_var { + RefCountable* provide(); + void bar(RefCountable*); + void foo() { + auto* obj = provide(); + bar(obj); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + } } namespace call_with_adopt_ref { `````````` </details> https://github.com/llvm/llvm-project/pull/129974 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
