[clang] [clang analysis][thread safety] Warn when returning rvalue references. (PR #91229)

[Clement Courbet via cfe-commits]

https://github.com/legrosbuffle created https://github.com/llvm/llvm-project/pull/91229

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

>From 86733474a1bf1486b807c95792781aa2d869c2ca Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Mon, 6 May 2024 14:15:47 +0000
Subject: [PATCH] [clang analysis][thread safety] Warn when returning rvalue
 references too.

We're missing `T&& Consume() && { return std::move(member); }`.
---
 clang/lib/Analysis/ThreadSafety.cpp           |  8 +++++-
 .../SemaCXX/warn-thread-safety-analysis.cpp   | 25 +++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

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}}
   }