melver wrote:
Committed!
> Looks good to me, and thanks for the contribution!
Thanks for your review!
Fingers crossed the Linux kernel changes will also land soon.
> > Note, I think for now it might be safer to not enable by default yet, but
> > I've made a note (and hinted at in changelog)
https://github.com/melver closed
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/aaronpuchert approved this pull request.
Looks good to me, and thanks for the contribution!
> Note, I think for now it might be safer to not enable by default yet, but
> I've made a note (and hinted at in changelog) that we're planning to default
> enable in future, so that
https://github.com/melver edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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
Date: Sun, 16 Feb 2025 14:53:41 +0100
Subject: [PATCH 1/2] Thread Safety Analysis: Handle address-of followed by
derefere
melver wrote:
> I think this looks very good! I just have some minor remarks.
>
> Thanks to @aoates for trying this out, this is always appreciated!
>
> And sorry for the delay.
Thanks for the review! I addressed the comments, PTAL.
Note, I think for now it might be safer to not enable by def
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
Date: Sun, 16 Feb 2025 14:53:41 +0100
Subject: [PATCH 1/2] Thread Safety Analysis: Handle address-of followed by
derefere
https://github.com/melver edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -4955,13 +4968,18 @@ 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_'}}
+
+(v
@@ -528,6 +529,9 @@ for a period of time, after which they are migrated into
the standard analysis.
* ``-Wthread-safety-beta``: New features. Off by default.
+ + ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+guarded variables, or pointers
@@ -528,6 +529,9 @@ for a period of time, after which they are migrated into
the standard analysis.
* ``-Wthread-safety-beta``: New features. Off by default.
+ + ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+guarded variables, or pointers
https://github.com/aaronpuchert commented:
I think this looks very good! I just have some minor remarks.
Thanks to @aoates for trying this out, this is always appreciated!
And sorry for the delay.
https://github.com/llvm/llvm-project/pull/127396
___
@@ -515,7 +515,8 @@ 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``:
@@ -1780,6 +1782,14 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet
&FSet, const Expr *Exp,
Exp = CE->getSubExpr();
continue;
}
+if (const auto *UO = dyn_cast(Exp)) {
+ if (UO->getOpcode() == UO_AddrOf) {
+// Pointer access via pointe
@@ -1795,9 +1795,22 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet
&FSet, const Expr *Exp,
// Pass by reference warnings are under a different flag.
aaronpuchert wrote:
"reference/pointer"
https://github.com/llvm/llvm-project/pull/127396
___
@@ -528,6 +529,9 @@ for a period of time, after which they are migrated into
the standard analysis.
* ``-Wthread-safety-beta``: New features. Off by default.
+ + ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+guarded variables, or pointers
@@ -2294,6 +2309,11 @@ void BuildLockset::VisitReturnStmt(const ReturnStmt *S) {
FunctionExitFSet, RetVal,
ReturnType->getPointeeType().isConstQualified() ? AK_Read : AK_Written,
POK_ReturnByRef);
+ } else if (ReturnType->isPointerType()) {
@@ -4944,6 +4949,14 @@ 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
@@ -4955,13 +4968,18 @@ 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_'}}
+
+(v
@@ -142,9 +145,19 @@ 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 '
https://github.com/aaronpuchert edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -6087,9 +6215,37 @@ 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
melver wrote:
> I only have a few comments about documenting the caveats (no alias analysis).
>
> The actual code changes look very simple and this looks like a clear
> improvement that would catch many useful cases.
>
> I don't have much experience with this code and would still advise to wai
@@ -143,6 +143,11 @@ Improvements to Clang's diagnostics
- A statement attribute applied to a ``case`` label no longer suppresses
'bypassing variable initialization' diagnostics (#84072).
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``
---
https://github.com/melver updated
https://github.com/llvm/llvm-project/pull/127396
>From a70f021becb2888d6c2a63b2d1e619393a996058 Mon Sep 17 00:00:00 2001
From: Marco Elver
Date: Sun, 16 Feb 2025 14:53:41 +0100
Subject: [PATCH 1/2] Thread Safety Analysis: Handle address-of followed by
derefere
@@ -6087,9 +6215,37 @@ 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
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ilya-biryukov commented:
I only have a few comments about documenting the caveats (no alias analysis).
The actual code changes look very simple and this looks like a clear
improvement that would catch many useful cases.
I don't have much experience with this code and would s
@@ -143,6 +143,11 @@ Improvements to Clang's diagnostics
- A statement attribute applied to a ``case`` label no longer suppresses
'bypassing variable initialization' diagnostics (#84072).
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``
---
melver wrote:
> > I need to commit this by end of day Wednesday - if I should wait a little
> > longer, let me know so I can plan around it. Thanks.
>
> Sorry, but I'm at C standards meetings this week, so I don't think I'll be
> able to review it by then. CC @aaronpuchert who maybe can help?
AaronBallman wrote:
> I need to commit this by end of day Wednesday - if I should wait a little
> longer, let me know so I can plan around it. Thanks.
Sorry, but I'm at C standards meetings this week, so I don't think I'll be able
to review it by then. CC @aaronpuchert who maybe can help?
htt
melver wrote:
Gentle ping.
Thanks!
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aoates wrote:
As an additional data point --- I patched in these commits and have been
testing with them on my C codebase and
a) so far I have no found any false positives
b) it has made a _huge_ difference in the effectiveness of TSA
I'm a strong supporter of this independent of the Linux kern
melver wrote:
FWIW, I'm ready for sending a v2 series to the Linux kernel mailing list:
https://git.kernel.org/pub/scm/linux/kernel/git/melver/linux.git/log/?h=cap-analysis/dev
But I'd like to sort out this PR first before sending the v2 series.
https://github.com/llvm/llvm-project/pull/127396
melver wrote:
(Side-note: I cannot figure out what the canonical way to do "stacked commits"
is for LLVM these days, so I'm just doing this the "old" way i.e. plain and
simple normal git commits which I will commit separately after PR review.)
https://github.com/llvm/llvm-project/pull/127396
_
https://github.com/melver edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/melver edited
https://github.com/llvm/llvm-project/pull/127396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
37 matches
Mail list logo
