llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to