[clang] [WebKit checkers] Trivial function analysis ignores some nodelete annotation (PR #183970)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

This PR fixes the bug that TrivialFunctionAnalysis can ignore nodelete 
annotation set on some but not all function declarations because it does not 
check the annotation on prior declarations unlike alpha.webkit.NoDeleteChecker 
which checks it on any declaration by replacing isNoDeleteFunction with 
NoDeleteChecker's hasNoDeleteAnnotation.

---
Full diff: https://github.com/llvm/llvm-project/pull/183970.diff


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp 
(+1-20) 
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp 
(+20-1) 
- (modified) clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp (+10) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
index 075c46c79f139..b3b43361a25ef 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoDeleteChecker.cpp
@@ -66,30 +66,11 @@ class NoDeleteChecker : public 
Checker<check::ASTDecl<TranslationUnitDecl>> {
     visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
   }
 
-  static bool hasNoDeleteAnnotation(const FunctionDecl *FD) {
-    if (llvm::any_of(FD->redecls(), isNoDeleteFunction))
-      return true;
-
-    const auto *MD = dyn_cast<CXXMethodDecl>(FD);
-    if (!MD || !MD->isVirtual())
-      return false;
-
-    auto Overriders = llvm::to_vector(MD->overridden_methods());
-    while (!Overriders.empty()) {
-      const auto *Fn = Overriders.pop_back_val();
-      llvm::append_range(Overriders, Fn->overridden_methods());
-      if (isNoDeleteFunction(Fn))
-        return true;
-    }
-
-    return false;
-  }
-
   void visitFunctionDecl(const FunctionDecl *FD) const {
     if (!FD->doesThisDeclarationHaveABody() || FD->isDependentContext())
       return;
 
-    if (!hasNoDeleteAnnotation(FD))
+    if (!isNoDeleteFunction(FD))
       return;
 
     auto Body = FD->getBody();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index b694742ee2b1a..47f97bc1d9d67 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -454,10 +454,29 @@ bool isPtrConversion(const FunctionDecl *F) {
   return false;
 }
 
-bool isNoDeleteFunction(const FunctionDecl *F) {
+static bool isNoDeleteFunctionDecl(const FunctionDecl *F) {
   return typeAnnotationForReturnType(F) == WebKitAnnotation::NoDelete;
 }
 
+bool isNoDeleteFunction(const FunctionDecl *F) {
+  if (llvm::any_of(F->redecls(), isNoDeleteFunctionDecl))
+    return true;
+
+  const auto *MD = dyn_cast<CXXMethodDecl>(F);
+  if (!MD || !MD->isVirtual())
+    return false;
+
+  auto Overriders = llvm::to_vector(MD->overridden_methods());
+  while (!Overriders.empty()) {
+    const auto *Fn = Overriders.pop_back_val();
+    llvm::append_range(Overriders, Fn->overridden_methods());
+    if (isNoDeleteFunctionDecl(Fn))
+      return true;
+  }
+
+  return false;
+}
+
 bool isTrivialBuiltinFunction(const FunctionDecl *F) {
   if (!F || !F->getDeclName().isIdentifier())
     return false;
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp 
b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index abe4aba18899e..04d1cb34ce789 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -13,6 +13,16 @@ void [[clang::annotate_type("webkit.nodelete")]] 
callsUnsafe() {
   someFunction(); // expected-warning{{A function 'callsUnsafe' has 
[[clang::annotate_type("webkit.nodelete")]] but it contains code that could 
destruct an object}}
 }
 
+void [[clang::annotate_type("webkit.nodelete")]] callsUnsafeWithSuppress();
+
+[[clang::suppress]] void callsUnsafeWithSuppress() {
+  someFunction();
+}
+
+void [[clang::annotate_type("webkit.nodelete")]] callsNoDeleteFunction() {
+  callsUnsafeWithSuppress();
+}
+
 #define EXPORT_IMPORT __attribute__((visibility("default")))
 EXPORT_IMPORT unsigned [[clang::annotate_type("webkit.nodelete")]] 
safeFunctionWithAttr();
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/183970
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits