[clang] Thread Safety Analysis: Improved pointer handling (PR #127396)
Marco Elver via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 26 03:13:49 PST 2025
https://github.com/melver updated https://github.com/llvm/llvm-project/pull/127396
>From f9fec4c8415b2b9c802b1d7ecdcc9f5cb038f7be Mon Sep 17 00:00:00 2001
From: Marco Elver <elver at google.com>
Date: Sun, 16 Feb 2025 14:53:41 +0100
Subject: [PATCH 1/2] Thread Safety Analysis: Handle address-of followed by
dereference
Correctly analyze expressions where the address of a guarded variable is
taken and immediately dereferenced, such as (*(type-specifier *)&x).
Previously, such patterns would result in false negatives.
Pull Request: https://github.com/llvm/llvm-project/pull/127396
---
clang/lib/Analysis/ThreadSafety.cpp | 11 +++++++++++
clang/test/Sema/warn-thread-safety-analysis.c | 19 +++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index bfaf1a0e7c7ff..1bad4eac23c68 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1765,6 +1765,8 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
AccessKind AK,
ProtectedOperationKind POK) {
+ // Strip off paren- and cast-expressions, checking if we encounter any other
+ // operator that should be delegated to checkAccess() instead.
while (true) {
if (const auto *PE = dyn_cast<ParenExpr>(Exp)) {
Exp = PE->getSubExpr();
@@ -1783,6 +1785,15 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
break;
}
+ if (const auto *UO = dyn_cast<UnaryOperator>(Exp)) {
+ if (UO->getOpcode() == UO_AddrOf) {
+ // Pointer access via pointer taken of variable, so the dereferenced
+ // variable is not actually a pointer.
+ checkAccess(FSet, UO->getSubExpr(), AK, POK);
+ return;
+ }
+ }
+
// Pass by reference warnings are under a different flag.
ProtectedOperationKind PtPOK = POK_VarDereference;
if (POK == POK_PassByRef) PtPOK = POK_PtPassByRef;
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 73b4e4798f644..46495ad4889b4 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -24,6 +24,9 @@
__attribute__ ((shared_locks_required(__VA_ARGS__)))
#define NO_THREAD_SAFETY_ANALYSIS __attribute__ ((no_thread_safety_analysis))
+#define __READ_ONCE(x) (*(const volatile __typeof__(x) *)&(x))
+#define __WRITE_ONCE(x, val) do { *(volatile __typeof__(x) *)&(x) = (val); } while (0)
+
// Define the mutex struct.
// Simplified only for test purpose.
struct LOCKABLE Mutex {};
@@ -142,9 +145,25 @@ int main(void) {
(void)(get_value(b_) == 1);
mutex_unlock(foo_.mu_);
+ a_ = 0; // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
+ __WRITE_ONCE(a_, 0); // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
+ (void)(a_ == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
+ (void)(__READ_ONCE(a_) == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
+ *b_ = 0; // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
+ __WRITE_ONCE(*b_, 0); // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
+ (void)(*b_ == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
+ (void)(__READ_ONCE(*b_) == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
c_ = 0; // expected-warning{{writing variable 'c_' requires holding any mutex exclusively}}
(void)(*d_ == 0); // expected-warning{{reading the value pointed to by 'd_' requires holding any mutex}}
mutex_exclusive_lock(foo_.mu_);
+ a_ = 0;
+ __WRITE_ONCE(a_, 0);
+ (void)(a_ == 0);
+ (void)(__READ_ONCE(a_) == 0);
+ *b_ = 0;
+ __WRITE_ONCE(*b_, 0);
+ (void)(*b_ == 0);
+ (void)(__READ_ONCE(*b_) == 0);
c_ = 1;
(void)(*d_ == 1);
mutex_unlock(foo_.mu_);
>From de996cd590041830308f1a672e1a1f54daf13ef5 Mon Sep 17 00:00:00 2001
From: Marco Elver <elver at google.com>
Date: Sun, 16 Feb 2025 12:42:06 +0100
Subject: [PATCH 2/2] Thread Safety Analysis: Support warning on
passing/returning pointers to guarded variables
Introduce `-Wthread-safety-pointer` to warn when passing or returning
pointers to guarded variables or guarded data. This is is analogous to
`-Wthread-safety-reference`, which performs similar checks for C++
references.
Adding checks for pointer passing is required to avoid false negatives
in large C codebases, where data structures are typically implemented
through helpers that take pointers to instances of a data structure.
The feature is planned to be enabled by default under `-Wthread-safety`
in the next release cycle. This gives time for early adopters to address
new findings.
Pull Request: https://github.com/llvm/llvm-project/pull/127396
---
clang/docs/ReleaseNotes.rst | 6 +
clang/docs/ThreadSafetyAnalysis.rst | 6 +-
.../clang/Analysis/Analyses/ThreadSafety.h | 12 ++
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td | 18 ++
clang/lib/Analysis/ThreadSafety.cpp | 30 ++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 24 +++
clang/test/Sema/warn-thread-safety-analysis.c | 8 +-
.../SemaCXX/warn-thread-safety-analysis.cpp | 184 +++++++++++++++++-
9 files changed, 276 insertions(+), 13 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3a0eab66fd584..fe00bd68507e4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -196,6 +196,12 @@ Improvements to Clang's diagnostics
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
``-Wno-error=parentheses``.
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``,
+ which enables warning on passing or returning pointers to guarded variables
+ as function arguments or return value respectively. Note that
+ :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
+ feature will be default-enabled with ``-Wthread-safety`` in a future release.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
index 9c1c32e46989b..130069c5659d6 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -515,8 +515,12 @@ Warning flags
+ ``-Wthread-safety-analysis``: The core analysis.
+ ``-Wthread-safety-precise``: Requires that mutex expressions match precisely.
This warning can be disabled for code which has a lot of aliases.
- + ``-Wthread-safety-reference``: Checks when guarded members are passed by reference.
+ + ``-Wthread-safety-reference``: Checks when guarded members are passed or
+ returned by reference.
+* ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+ guarded variables, or pointers to guarded data, as function argument or
+ return value respectively.
:ref:`negative` are an experimental feature, which are enabled with:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0fcf5bed1285a..20b75c46593e0 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -54,6 +54,18 @@ enum ProtectedOperationKind {
/// Returning a pt-guarded variable by reference.
POK_PtReturnByRef,
+
+ /// Passing pointer to a guarded variable.
+ POK_PassPointer,
+
+ /// Passing a pt-guarded pointer.
+ POK_PtPassPointer,
+
+ /// Returning pointer to a guarded variable.
+ POK_ReturnPointer,
+
+ /// Returning a pt-guarded pointer.
+ POK_PtReturnPointer,
};
/// This enum distinguishes between different kinds of lock actions. For
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 05e39899e6f25..77520447b813d 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1156,6 +1156,7 @@ def ThreadSafetyPrecise : DiagGroup<"thread-safety-precise">;
def ThreadSafetyReferenceReturn : DiagGroup<"thread-safety-reference-return">;
def ThreadSafetyReference : DiagGroup<"thread-safety-reference",
[ThreadSafetyReferenceReturn]>;
+def ThreadSafetyPointer : DiagGroup<"thread-safety-pointer">;
def ThreadSafetyNegative : DiagGroup<"thread-safety-negative">;
def ThreadSafety : DiagGroup<"thread-safety",
[ThreadSafetyAttributes,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 51301d95e55b9..73b00752e6b40 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4126,6 +4126,24 @@ def warn_pt_guarded_return_by_reference : Warning<
"%select{'%2'|'%2' exclusively}3">,
InGroup<ThreadSafetyReferenceReturn>, DefaultIgnore;
+// Thread safety warnings on pointer passing
+def warn_guarded_pass_pointer : Warning<
+ "passing pointer to variable %1 requires holding %0 "
+ "%select{'%2'|'%2' exclusively}3">,
+ InGroup<ThreadSafetyPointer>, DefaultIgnore;
+def warn_pt_guarded_pass_pointer : Warning<
+ "passing pointer %1 requires holding %0 "
+ "%select{'%2'|'%2' exclusively}3">,
+ InGroup<ThreadSafetyPointer>, DefaultIgnore;
+def warn_guarded_return_pointer : Warning<
+ "returning pointer to variable %1 requires holding %0 "
+ "%select{'%2'|'%2' exclusively}3">,
+ InGroup<ThreadSafetyPointer>, DefaultIgnore;
+def warn_pt_guarded_return_pointer : Warning<
+ "returning pointer %1 requires holding %0 "
+ "%select{'%2'|'%2' exclusively}3">,
+ InGroup<ThreadSafetyPointer>, DefaultIgnore;
+
// Imprecise thread safety warnings
def warn_variable_requires_lock : Warning<
"%select{reading|writing}3 variable %1 requires holding %0 "
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 1bad4eac23c68..6b5b49377fa08 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1794,11 +1794,24 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
}
}
- // Pass by reference warnings are under a different flag.
+ // Pass by reference/pointer warnings are under a different flag.
ProtectedOperationKind PtPOK = POK_VarDereference;
- if (POK == POK_PassByRef) PtPOK = POK_PtPassByRef;
- if (POK == POK_ReturnByRef)
+ switch (POK) {
+ case POK_PassByRef:
+ PtPOK = POK_PtPassByRef;
+ break;
+ case POK_ReturnByRef:
PtPOK = POK_PtReturnByRef;
+ break;
+ case POK_PassPointer:
+ PtPOK = POK_PtPassPointer;
+ break;
+ case POK_ReturnPointer:
+ PtPOK = POK_PtReturnPointer;
+ break;
+ default:
+ break;
+ }
const ValueDecl *D = getValueDecl(Exp);
if (!D || !D->hasAttrs())
@@ -2145,6 +2158,8 @@ void BuildLockset::examineArguments(const FunctionDecl *FD,
QualType Qt = (*Param)->getType();
if (Qt->isReferenceType())
checkAccess(*Arg, AK_Read, POK_PassByRef);
+ else if (Qt->isPointerType())
+ checkPtAccess(*Arg, AK_Read, POK_PassPointer);
}
}
@@ -2286,8 +2301,8 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
if (!RetVal)
return;
- // If returning by reference, check that the function requires the appropriate
- // capabilities.
+ // If returning by reference or pointer, check that the function requires the
+ // appropriate capabilities.
const QualType ReturnType =
Analyzer->CurrentFunction->getReturnType().getCanonicalType();
if (ReturnType->isLValueReferenceType()) {
@@ -2295,6 +2310,11 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
FunctionExitFSet, RetVal,
ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
POK_ReturnByRef);
+ } else if (ReturnType->isPointerType()) {
+ Analyzer->checkPtAccess(
+ FunctionExitFSet, RetVal,
+ ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
+ POK_ReturnPointer);
}
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index afdc0eaab0a40..3d6da4f70f99e 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1977,6 +1977,18 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
case POK_PtReturnByRef:
DiagID = diag::warn_pt_guarded_return_by_reference;
break;
+ case POK_PassPointer:
+ DiagID = diag::warn_guarded_pass_pointer;
+ break;
+ case POK_PtPassPointer:
+ DiagID = diag::warn_pt_guarded_pass_pointer;
+ break;
+ case POK_ReturnPointer:
+ DiagID = diag::warn_guarded_return_pointer;
+ break;
+ case POK_PtReturnPointer:
+ DiagID = diag::warn_pt_guarded_return_pointer;
+ break;
}
PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind
<< D
@@ -2013,6 +2025,18 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler {
case POK_PtReturnByRef:
DiagID = diag::warn_pt_guarded_return_by_reference;
break;
+ case POK_PassPointer:
+ DiagID = diag::warn_guarded_pass_pointer;
+ break;
+ case POK_PtPassPointer:
+ DiagID = diag::warn_pt_guarded_pass_pointer;
+ break;
+ case POK_ReturnPointer:
+ DiagID = diag::warn_guarded_return_pointer;
+ break;
+ case POK_PtReturnPointer:
+ DiagID = diag::warn_pt_guarded_return_pointer;
+ break;
}
PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind
<< D
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 46495ad4889b4..c58b7bed61183 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -fexperimental-late-parse-attributes -DLATE_PARSING %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -fexperimental-late-parse-attributes -DLATE_PARSING %s
#define LOCKABLE __attribute__ ((lockable))
#define SCOPED_LOCKABLE __attribute__ ((scoped_lockable))
@@ -137,7 +137,9 @@ int main(void) {
Foo_func3(5);
set_value(&a_, 0); // expected-warning{{calling function 'set_value' requires holding mutex 'foo_.mu_' exclusively}}
+ // expected-warning at -1{{passing pointer to variable 'a_' requires holding mutex 'foo_.mu_'}}
get_value(b_); // expected-warning{{calling function 'get_value' requires holding mutex 'foo_.mu_'}}
+ // expected-warning at -1{{passing pointer 'b_' requires holding mutex 'foo_.mu_'}}
mutex_exclusive_lock(foo_.mu_);
set_value(&a_, 1);
mutex_unlock(foo_.mu_);
@@ -199,6 +201,8 @@ int main(void) {
#ifdef LATE_PARSING
late_parsing.a_value_defined_before = 1; // expected-warning{{writing variable 'a_value_defined_before' requires holding mutex 'a_mutex_defined_late' exclusively}}
late_parsing.a_ptr_defined_before = 0;
+ set_value(&late_parsing.a_value_defined_before, 0); // expected-warning{{calling function 'set_value' requires holding mutex 'foo_.mu_' exclusively}}
+ // expected-warning at -1{{passing pointer to variable 'a_value_defined_before' requires holding mutex 'a_mutex_defined_late'}}
mutex_exclusive_lock(late_parsing.a_mutex_defined_late);
mutex_exclusive_lock(late_parsing.a_mutex_defined_early); // expected-warning{{mutex 'a_mutex_defined_early' must be acquired before 'a_mutex_defined_late'}}
mutex_exclusive_unlock(late_parsing.a_mutex_defined_early);
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 018d6d1bb258b..ac3ca5e0c12a8 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
@@ -4863,6 +4863,11 @@ class Data {
int dat;
};
+class DataWithAddrOf : public Data {
+public:
+ const Data* operator&();
+ const Data* operator&() const;
+};
class DataCell {
public:
@@ -4873,6 +4878,7 @@ class DataCell {
};
+void showData(const Data* d);
void showDataCell(const DataCell& dc);
@@ -4944,6 +4950,11 @@ class Foo {
(*datap2_)[0] = 0; // expected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
data_(); // expected-warning {{reading variable 'data_' requires holding mutex 'mu_'}}
+
+ // Calls operator&, and does not take the address.
+ (void)&data_ao_; // expected-warning {{reading variable 'data_ao_' requires holding mutex 'mu_'}}
+ (void)__builtin_addressof(data_ao_); // expected-warning {{passing variable 'data_ao_' by reference requires holding mutex 'mu_'}}
+ showData(&data_ao_); // expected-warning {{reading variable 'data_ao_' requires holding mutex 'mu_'}}
}
// const operator tests
@@ -4955,6 +4966,9 @@ class Foo {
//showDataCell(*datap2_); // xpected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
int a = data_[0]; // expected-warning {{reading variable 'data_' requires holding mutex 'mu_'}}
+
+ (void)&data_ao_; // expected-warning {{reading variable 'data_ao_' requires holding mutex 'mu_'}}
+ showData(&data_ao_); // expected-warning {{reading variable 'data_ao_' requires holding mutex 'mu_'}}
}
private:
@@ -4962,6 +4976,7 @@ class Foo {
Data data_ GUARDED_BY(mu_);
Data* datap1_ GUARDED_BY(mu_);
Data* datap2_ PT_GUARDED_BY(mu_);
+ DataWithAddrOf data_ao_ GUARDED_BY(mu_);
};
} // end namespace GuardedNonPrimitiveTypeTest
@@ -5911,8 +5926,12 @@ T&& mymove(T& f);
void copy(Foo f);
void write1(Foo& f);
void write2(int a, Foo& f);
+void write3(Foo* f);
+void write4(Foo** f);
void read1(const Foo& f);
void read2(int a, const Foo& f);
+void read3(const Foo* f);
+void read4(Foo* const* f);
void destroy(Foo&& f);
void operator/(const Foo& f, const Foo& g);
@@ -5942,19 +5961,24 @@ class Bar {
Foo foo GUARDED_BY(mu);
Foo foo2 GUARDED_BY(mu);
Foo* foop PT_GUARDED_BY(mu);
+ Foo* foop2 GUARDED_BY(mu);
SmartPtr<Foo> foosp PT_GUARDED_BY(mu);
// test methods.
void mwrite1(Foo& f);
void mwrite2(int a, Foo& f);
+ void mwrite3(Foo* f);
void mread1(const Foo& f);
void mread2(int a, const Foo& f);
+ void mread3(const Foo* f);
// static methods
static void smwrite1(Foo& f);
static void smwrite2(int a, Foo& f);
+ static void smwrite3(Foo* f);
static void smread1(const Foo& f);
static void smread2(int a, const Foo& f);
+ static void smread3(const Foo* f);
void operator<<(const Foo& f);
@@ -6017,6 +6041,107 @@ class Bar {
read2(10, *foosp.get());
destroy(mymove(*foosp.get()));
}
+
+ void test_pass_pointer() {
+ (void)&foo; // no warning
+ (void)foop; // no warning
+
+ write3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ write3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ write3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ write4((Foo **)&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ write4(&foop); // no warning
+ read3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ read3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ read3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ read4((Foo **)&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ read4(&foop); // no warning
+ mwrite3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ mwrite3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ mwrite3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ mread3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ mread3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ mread3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ smwrite3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ smwrite3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ smwrite3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ smread3(&foo); // expected-warning {{passing pointer to variable 'foo' requires holding mutex 'mu'}}
+ smread3(foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+ smread3(&*foop); // expected-warning {{passing pointer 'foop' requires holding mutex 'mu'}}
+
+ write3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ write3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ write4(&foop2); // expected-warning {{passing pointer to variable 'foop2' requires holding mutex 'mu'}}
+ read3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ read3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ read4(&foop2); // expected-warning {{passing pointer to variable 'foop2' requires holding mutex 'mu'}}
+ mwrite3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ mwrite3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ mread3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ mread3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ smwrite3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ smwrite3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ smread3(foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+ smread3(&*foop2); // expected-warning {{reading variable 'foop2' requires holding mutex 'mu'}}
+
+ mu.Lock();
+ write3(&foo);
+ write3(foop);
+ write3(&*foop);
+ write3(foop2);
+ write4(&foop2);
+ read3(&foo);
+ read3(foop);
+ read3(&*foop);
+ read3(foop2);
+ read4(&foop2);
+ mwrite3(&foo);
+ mwrite3(foop);
+ mwrite3(&*foop);
+ mwrite3(foop2);
+ mread3(&foo);
+ mread3(foop);
+ mread3(&*foop);
+ mread3(foop2);
+ smwrite3(&foo);
+ smwrite3(foop);
+ smwrite3(&*foop);
+ smwrite3(foop2);
+ smread3(&foo);
+ smread3(foop);
+ smread3(&*foop);
+ smread3(foop2);
+ mu.Unlock();
+
+ mu.ReaderLock();
+ write3(&foo);
+ write3(foop);
+ write3(&*foop);
+ write3(foop2);
+ write4(&foop2);
+ read3(&foo);
+ read3(foop);
+ read3(&*foop);
+ read3(foop2);
+ read4(&foop2);
+ mwrite3(&foo);
+ mwrite3(foop);
+ mwrite3(&*foop);
+ mwrite3(foop2);
+ mread3(&foo);
+ mread3(foop);
+ mread3(&*foop);
+ mread3(foop2);
+ smwrite3(&foo);
+ smwrite3(foop);
+ smwrite3(&*foop);
+ smwrite3(foop2);
+ smread3(&foo);
+ smread3(foop);
+ smread3(&*foop);
+ smread3(foop2);
+ mu.ReaderUnlock();
+ }
};
class Return {
@@ -6087,15 +6212,64 @@ class Return {
const Foo &returns_constref_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
return foo;
}
+
+ Foo *returns_ptr_exclusive_locks_required() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ return &foo;
+ }
+
+ Foo *returns_pt_ptr_exclusive_locks_required() EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ return foo_ptr;
+ }
+
+ Foo *returns_ptr_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
+ return &foo; // expected-warning {{returning pointer to variable 'foo' requires holding mutex 'mu' exclusively}}
+ }
+
+ Foo *returns_pt_ptr_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
+ return foo_ptr; // expected-warning {{returning pointer 'foo_ptr' requires holding mutex 'mu' exclusively}}
+ }
+
+ const Foo *returns_constptr_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
+ return &foo;
+ }
+
+ const Foo *returns_pt_constptr_shared_locks_required() SHARED_LOCKS_REQUIRED(mu) {
+ return foo_ptr;
+ }
Foo *returns_ptr() {
- return &foo; // FIXME -- Do we want to warn on this ?
+ return &foo; // expected-warning {{returning pointer to variable 'foo' requires holding mutex 'mu' exclusively}}
+ }
+
+ Foo *returns_pt_ptr() {
+ return foo_ptr; // expected-warning {{returning pointer 'foo_ptr' requires holding mutex 'mu' exclusively}}
}
Foo &returns_ref2() {
return *foo_ptr; // expected-warning {{returning the value that 'foo_ptr' points to by reference requires holding mutex 'mu' exclusively}}
}
+ // FIXME: Basic alias analysis would help catch cases like below.
+ Foo *returns_ptr_alias() {
+ mu.Lock();
+ Foo *ret = &foo;
+ mu.Unlock();
+ return ret; // no warning (false negative)
+ }
+
+ Foo *returns_pt_ptr_alias() {
+ mu.Lock();
+ Foo *ret = foo_ptr;
+ mu.Unlock();
+ return ret; // no warning (false negative)
+ }
+
+ Foo &returns_ref2_alias() {
+ mu.Lock();
+ Foo *ret = foo_ptr;
+ mu.Unlock();
+ return *ret; // no warning (false negative)
+ }
};
More information about the cfe-commits
mailing list