[clang] [clang analysis][thread safety] Warn when returning rvalue references. (PR #91229)
llvmbot wrote:
@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
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
, const Expr *Exp,
checkAccess(FSet, ME->getBase(), AK, POK);
}
+ if (const auto *C = dyn_cast(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 struct remove_reference {
+ using type = T;
+};
+template struct remove_reference {
+ using type = T;
+};
+template struct remove_reference {
+ using type = T;
+};
+
+template
+constexpr typename std::remove_reference::type &(T &) noexcept {
+ return static_cast::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 &_refref_locked() {
+MutexLock lock();
+return std::move(foo);// expected-warning {{returning variable 'foo'
by reference requires holding mutex 'mu'}}
+ }
+
Foo _ref_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
return foo; // expected-warning {{returning variable 'foo'
by reference requires holding mutex 'mu' exclusively}}
}
``
https://github.com/llvm/llvm-project/pull/91229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang analysis][thread safety] Warn when returning rvalue references. (PR #91229)
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
Date: Mon, 6 May 2024 14:15:47 +
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
, const Expr *Exp,
checkAccess(FSet, ME->getBase(), AK, POK);
}
+ if (const auto *C = dyn_cast(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 struct remove_reference {
+ using type = T;
+};
+template struct remove_reference {
+ using type = T;
+};
+template struct remove_reference {
+ using type = T;
+};
+
+template
+constexpr typename std::remove_reference::type &(T &) noexcept {
+ return static_cast::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 &_refref_locked() {
+MutexLock lock();
+return std::move(foo);// expected-warning {{returning variable 'foo'
by reference requires holding mutex 'mu'}}
+ }
+
Foo _ref_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
return foo; // expected-warning {{returning variable 'foo'
by reference requires holding mutex 'mu' exclusively}}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
