[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert added a comment. It seems @phosek was able to fix the issue in https://github.com/flutter/engine/pull/5944. By the way, a nice way to think about the attributes is that they encode state transitions as shown in the following table. This change fills the remaining two gaps. |

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert added a comment. In https://reviews.llvm.org/D49355#1188603, @phosek wrote: > In https://reviews.llvm.org/D49355#1188520, @aaronpuchert wrote: > > > Could you explain what annotations like `LOCK_UNLOCK` are useful for? What > > do they check? The annotation should certainly not be n

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

phosek added a comment. In https://reviews.llvm.org/D49355#1188520, @aaronpuchert wrote: > Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do > they check? The annotation should certainly not be necessary. > > Shouldn't you just use `REQUIRES(!...)` or `EXCLUDES(...)`?

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert added a comment. Could you explain what annotations like `LOCK_UNLOCK` are useful for? What do they check? The annotation should certainly not be necessary. Shouldn't you just use `REQUIRES(!...)` or `EXCLUDES(...)`? If a function locks and unlocks a mutex, I don't see a reason to

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

phosek added a comment. This change broke the acquire+release case. Concretely, in Flutter we have this code: https://github.com/flutter/engine/blob/master/lib/ui/isolate_name_server/isolate_name_server.h#L26. This worked fine previously, but after this change we're getting an error in https:/

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r338024, thank you for the patch! Repository: rC Clang https://reviews.llvm.org/D49355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert added a comment. No problem. Thanks for the review! Would be nice if you or @aaron.ballman could commit this, as I don't have commit access. Repository: rC Clang https://reviews.llvm.org/D49355 ___ cfe-commits mailing list cfe-commi

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

delesley accepted this revision. delesley added a comment. This revision is now accepted and ready to land. Looks good to me. Thanks for the patch, and my apologies for the slow response. (I'm on a different project now, so I'm afraid thread safety doesn't always get the attention from me that

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaron.ballman added a comment. I'm going to let @delesley give the final sign off on this as he is more familiar with this part of the analysis, but I think this looks reasonable. Repository: rC Clang https://reviews.llvm.org/D49355 ___ cfe-comm

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert added a comment. Ping? The functional changes should be minimal. Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:38-39 #endif +#define EXCLUSIVE_UNLOCK_FUNCTION(...) __attribute__((release_capability(__VA_ARGS__))) +#define SHARED_UNLOCK_FUNCTION(...)

[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. Herald added a subscriber: cfe-commits. We can now have methods that release a locked in shared mode and acquire it in exclusive mode or the other way around. The fix was just to release the locks before acq