[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D102026#2785243 , @delesley wrote: > Assert_capability is not a back door. It is supposed to be used only on a > function which does a run-time check: if (!mu_.is_locked()) fail(). Right, although assertions can turn

Re: [PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-27 Thread Delesley Hutchins via cfe-commits
> - The `assert_capability` attribute is also a bit of a backdoor. Instead > of statically propagating through the code that a mutex is held, we can > just get that fact "out of thin air". > Assert_capability is not a back door. It is supposed to be used only on a function which does a run-time

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. aaronpuchert marked 2 inline comments as done. Closed by commit rGcf0b337c1b1f: Thread safety analysis: Allow exlusive/shared joins for managed and asserted… (authored

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-27 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D102026#2780384 , @delesley wrote: > Thanks for taking the time to discuss things with me. :-) Thank you as well! > Wrt. to the TEST_LOCKED_FUNCTION, I agree that you can simulate the behavior > using Assert and Lock.

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-25 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley accepted this revision. delesley added a comment. This revision is now accepted and ready to land. Thanks for taking the time to discuss things with me. :-) Wrt. to the TEST_LOCKED_FUNCTION, I agree that you can simulate the behavior using Assert and Lock. But that pattern is too

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D102026#2779983 , @aaronpuchert wrote: > So this change will allow shared/exclusive - asserted/unmanaged joins until > my follow-up will disallow them again, though because of the > asserted/unmanaged part and not the

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-25 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. In D102026#2777438 , @delesley wrote: > The warn/not-warn is consistent with more relaxed handling of managed locks, > but exclusive/shared difference could lead to confusion. Both mechanisms are > sound; they're just not

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-24 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley added inline comments. Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4570 mu_.AssertHeld(); -mu_.Unlock(); - } // should this be a warning? +mu_.Unlock(); // should this be a warning? + } This function should have a

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-24 Thread Delesley Hutchins via Phabricator via cfe-commits
delesley added a comment. I have a few concerns. First, this patch introduces an inconsistency between managed and unmanaged locks. For unmanaged locks, we warn, and //then assume the lock is held exclusively//. For managed locks, we don't warn, but //assume it is held shared//. The

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In general, I think this makes sense, but I'd like to hear from @delesley as well. Comment at: clang/lib/Analysis/ThreadSafety.cpp:2198 +// For managed capabilities, the destructor should unlock in the right mode +// anyway. For asserted

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment. Ping @aaron.ballman, also for the parent change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102026/new/ https://reviews.llvm.org/D102026 ___ cfe-commits mailing list

[PATCH] D102026: Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities

2021-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert created this revision. aaronpuchert added reviewers: aaron.ballman, delesley. aaronpuchert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Similar to how we allow managed and asserted locks to be held and not held in joining