[clang analysis][thread safety] Warn when returning rvalue references.

[legrosbuffle]

We're missing T&& Consume() && { return std::move(member); }.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Clement Courbet (legrosbuffle)

Changes

We're missing T&& Consume() && { return std::move(member); }.

---

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

2 Files Affected:

• (modified) clang/lib/Analysis/ThreadSafety.cpp (+7-1)
• (modified) clang/test/SemaCXX/warn-thread-safety-analysis.cpp (+25)

diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index e25b843c9bf83e..ce2074a5922e32 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1725,6 +1725,12 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
       checkAccess(FSet, ME->getBase(), AK, POK);
   }
 
+  if (const auto *C = dyn_cast<CallExpr>(Exp); C && C->isCallToStdMove()) {
+    // Changing rvalue-ness of a reference does not change anything w.r.t
+    // thread-safety.
+    checkAccess(FSet, C->getArg(0), AK, POK);
+  }
+
   const ValueDecl *D = getValueDecl(Exp);
   if (!D || !D->hasAttrs())
     return;
@@ -2160,7 +2166,7 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
   // capabilities.
   const QualType ReturnType =
       Analyzer->CurrentFunction->getReturnType().getCanonicalType();
-  if (ReturnType->isLValueReferenceType()) {
+  if (ReturnType->isReferenceType()) {
     Analyzer->checkAccess(
         FunctionExitFSet, RetVal,
         ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index dfb966d3b5902d..03f63227f515cd 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -8,6 +8,26 @@
 
 #include "thread-safety-annotations.h"
 
+
+namespace std {
+
+template <typename T> struct remove_reference {
+  using type = T;
+};
+template <typename T> struct remove_reference<T &> {
+  using type = T;
+};
+template <typename T> struct remove_reference<T &&> {
+  using type = T;
+};
+
+template <typename T>
+constexpr typename std::remove_reference<T>::type &&move(T &&t) noexcept {
+  return static_cast<typename std::remove_reference<T>::type &&>(t);
+}
+
+} // namespace std
+
 class LOCKABLE Mutex {
  public:
   void Lock() EXCLUSIVE_LOCK_FUNCTION();
@@ -5630,6 +5650,11 @@ class Return {
     return foo;               // expected-warning {{returning variable 'foo' by reference requires holding mutex 'mu'}}
   }
 
+  Foo &&returns_refref_locked() {
+    MutexLock lock(&mu);
+    return std::move(foo);    // expected-warning {{returning variable 'foo' by reference requires holding mutex 'mu'}}
+  }
+
   Foo &returns_ref_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
     return foo;               // expected-warning {{returning variable 'foo' by reference requires holding mutex 'mu' exclusively}}
   }