[PATCH] D50110: Handle shared release attributes correctly
aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D50110#1187828, @aaronpuchert wrote: > For now I think I'm done fixing things in the thread safety analysis, but if > I see myself contributing more in the longer term, I will definitely try to > obtain commit access. Sounds good to me. I've commit in r338912. Thank you for the thread safety analysis patches! Repository: rC Clang https://reviews.llvm.org/D50110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaronpuchert added a comment. For now I think I'm done fixing things in the thread safety analysis, but if I see myself contributing more in the longer term, I will definitely try to obtain commit access. Repository: rC Clang https://reviews.llvm.org/D50110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaron.ballman added a comment. In https://reviews.llvm.org/D50110#1187771, @aaronpuchert wrote: > Thanks for the review! Could you commit for me again? I can, but it might also be a good idea for you to obtain your own commit privileges at this point (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access), if that's something you're interested in. Let me know which route you'd like to go. Repository: rC Clang https://reviews.llvm.org/D50110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaronpuchert added a comment. Thanks for the review! Could you commit for me again? Repository: rC Clang https://reviews.llvm.org/D50110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085
+ void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+ }
+
+ void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {
aaronpuchert wrote:
> aaron.ballman wrote:
> > Nothing calls either of these functions within the test; is that intended?
> Yes, I just wanted to check that there are no warnings within the functions.
> Before the changes in `lib/Analysis/ThreadSafety.cpp`, we would get a warning
> "releasing mutex 'mu' using shared access, expected exclusive access" on line
> 4086.
>
> My changes address the attributes on the function being analyzed, not on a
> function that is called from the function being analyzed.
Ah, okay, thank you for the explanation. I wasn't aware that was previously
failing, so I didn't see the behavioral change.
Repository:
rC Clang
https://reviews.llvm.org/D50110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaronpuchert added inline comments.
Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085
+ void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+ }
+
+ void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {
aaron.ballman wrote:
> Nothing calls either of these functions within the test; is that intended?
Yes, I just wanted to check that there are no warnings within the functions.
Before the changes in `lib/Analysis/ThreadSafety.cpp`, we would get a warning
"releasing mutex 'mu' using shared access, expected exclusive access" on line
4086.
My changes address the attributes on the function being analyzed, not on a
function that is called from the function being analyzed.
Repository:
rC Clang
https://reviews.llvm.org/D50110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaron.ballman added inline comments.
Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085
+ void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+ }
+
+ void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {
Nothing calls either of these functions within the test; is that intended?
Repository:
rC Clang
https://reviews.llvm.org/D50110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50110: Handle shared release attributes correctly
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D50110
Files:
lib/Analysis/ThreadSafety.cpp
test/SemaCXX/warn-thread-safety-analysis.cpp
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4078,6 +4078,14 @@
mu_.Unlock();
}
+ void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+ }
+
+ void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {
+mu_.ReaderUnlock();
+ }
+
// Check failure to lock.
void lockBad() EXCLUSIVE_LOCK_FUNCTION(mu_) {// expected-note {{mutex
acquired here}}
mu2_.Lock();
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -2242,8 +2242,8 @@
// We must ignore such methods.
if (A->args_size() == 0)
return;
-// FIXME -- deal with exclusive vs. shared unlock functions?
-getMutexIDs(ExclusiveLocksToAdd, A, nullptr, D);
+getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+nullptr, D);
getMutexIDs(LocksReleased, A, nullptr, D);
CapDiagKind = ClassifyDiagnostic(A);
} else if (const auto *A = dyn_cast(Attr)) {
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4078,6 +4078,14 @@
mu_.Unlock();
}
+ void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) {
+mu_.Unlock();
+ }
+
+ void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) {
+mu_.ReaderUnlock();
+ }
+
// Check failure to lock.
void lockBad() EXCLUSIVE_LOCK_FUNCTION(mu_) {// expected-note {{mutex acquired here}}
mu2_.Lock();
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -2242,8 +2242,8 @@
// We must ignore such methods.
if (A->args_size() == 0)
return;
-// FIXME -- deal with exclusive vs. shared unlock functions?
-getMutexIDs(ExclusiveLocksToAdd, A, nullptr, D);
+getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+nullptr, D);
getMutexIDs(LocksReleased, A, nullptr, D);
CapDiagKind = ClassifyDiagnostic(A);
} else if (const auto *A = dyn_cast(Attr)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
