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 <el...@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 59a5de516fa7f81154d45a228ddda036a1440775 Mon Sep 17 00:00:00 2001 From: Marco Elver <el...@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. 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. 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. 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@-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@-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@-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) + } }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
