[clang] [WebKit checkers] Treat a move constructor as non-trivial (PR #184986)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ryosuke Niwa (rniwa)

<details>
<summary>Changes</summary>

Since a move constructor conceptually destroys its argument, we need to treat it like a delete operation.

This PR updates "trivial function analysis" so that calling a move constructor is treated as non-trivial unless it's a trivially destructive type or its sole argument is a local variable or a parameter.

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


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+19-1) 
- (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+4-1) 
- (modified) clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp (+25) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 0b1f30923d49c..51305f05c0a29 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -883,7 +883,25 @@ class TrivialFunctionAnalysisVisitor
     }
 
     // Recursively descend into the callee to confirm that it's trivial.
-    return IsFunctionTrivial(CE->getConstructor());
+    auto *Ctor = CE->getConstructor();
+    if (Ctor->isMoveConstructor() && !IsFirstArgTrivialDtorOrLocalOrParm(CE))
+      return false;
+    return IsFunctionTrivial(Ctor);
+  }
+
+  bool IsFirstArgTrivialDtorOrLocalOrParm(const CXXConstructExpr *CE) {
+    if (CE->getNumArgs() != 1)
+      return false;
+    auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+    if (CanTriviallyDestruct(Arg->getType()))
+      return true;
+    auto DRE = dyn_cast<DeclRefExpr>(Arg);
+    if (!DRE)
+      return false;
+    auto *Decl = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+    if (!Decl)
+      return false;
+    return Decl->isLocalVarDeclOrParm();
   }
 
   bool VisitCXXDeleteExpr(const CXXDeleteExpr *DE) {
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index c139a5cb13de7..b6d345537408d 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -11,7 +11,10 @@ template <typename T> struct remove_reference<T&> {
 typedef T type;
 };
 
-template<typename T> typename remove_reference<T>::type&& move(T&& t);
+template<typename T> typename remove_reference<T>::type&& move(T&& t)
+{
+  return static_cast<typename remove_reference<T>::type&&>(t);
+}
 
 }
 
diff --git a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
index faf43178fae9a..ec66287eb018e 100644
--- a/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/nodelete-annotation.cpp
@@ -213,6 +213,31 @@ class SomeClass {
     m_obj = obj; // expected-warning{{A function 'setObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
   }
 
+  RefPtr<RefCountable> [[clang::annotate_type("webkit.nodelete")]] takeObj() {
+    return std::move(m_obj); // expected-warning{{A function 'takeObj' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+  }
+
+  struct Duration {
+    double seconds;
+  };
+  Duration [[clang::annotate_type("webkit.nodelete")]] returnArg(Duration duration) {
+    return duration;
+  }
+
+  Duration m_duration;
+  Duration [[clang::annotate_type("webkit.nodelete")]] returnStruct() {
+    return std::move(m_duration);
+  }
+
+  struct PtrContainer {
+    RefPtr<RefCountable> ptr;
+  };
+
+  PtrContainer m_container;
+  PtrContainer [[clang::annotate_type("webkit.nodelete")]] returnPtrContainer() {
+    return std::move(m_container); // expected-warning{{A function 'returnPtrContainer' has [[clang::annotate_type("webkit.nodelete")]] but it contains code that could destruct an object}}
+  }
+
   void [[clang::annotate_type("webkit.nodelete")]] swapObj(RefPtr<RefCountable>&& obj) {
     m_obj.swap(obj);
   }

``````````

</details>


https://github.com/llvm/llvm-project/pull/184986