[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -1359,17 +1361,18 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet
, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg) {
- // Find out which branch has the lock
- bool branch = false;
- if (const auto *BLE = dyn_cast_or_null(BrE))
-branch = BLE->getValue();
- else if (const auto *ILE = dyn_cast_or_null(BrE))
-branch = ILE->getValue().getBoolValue();
aaronpuchert wrote:
> My instinct is to explicitly disallow enumerator success expressions. [...]
> To my knowledge, enumerators were never actually supported in success
> expressions, so in some sense this will only break code that was already
> broken.
Agreed.
> Is it normally expected to keep incorrect behavior around for compatibility?
It depends. If the incorrect behavior is being relied upon, or the correction
could produce hard-to-find issues like different behavior at runtime, then it
could make sense to keep it. But here I find it unlikely that someone relies on
this, as it does not properly work, and the additional error would at most make
compilation fail and would be trivial to fix.
> How are breaking changes such as this typically handled?
I'd argue that the change isn't breaking anything, it's merely diagnosing a
case that was already not appropriately handled.
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -1359,17 +1361,18 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet
, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg) {
- // Find out which branch has the lock
- bool branch = false;
- if (const auto *BLE = dyn_cast_or_null(BrE))
-branch = BLE->getValue();
- else if (const auto *ILE = dyn_cast_or_null(BrE))
-branch = ILE->getValue().getBoolValue();
dmcardle wrote:
Thanks for the feedback, @aaronpuchert! I've reverted the change and I'll work
through these comments for a reland.
My instinct is to explicitly disallow enumerator success expressions.
Currently, the analysis can produce both false negatives and false positives
when enumerator success expressions are used:
. To my knowledge, enumerators were never
actually supported in success expressions, so in some sense this will only
break code that was already broken. However, I do want to be sensitive to
downstream breakage based on my experience with this PR!
One suggestion was to add a flag that disables any new errors, the goal being
to enabling large code bases to incrementally adapt to the new behavior. I'm
just not sure how this flag would actually work.
* If we suppress any new type errors, but otherwise apply the attribute, the
analysis is now off the rails, operating on unexpected inputs.
* If we simply ignore any attributes that would cause an error to be emitted,
now the analysis can produce different kinds of incorrect results, e.g. it
might complain that you're unlocking a lock that was never acquired.
* I suppose we could mimic the current/incorrect behavior when the flag is
present. Is it normally expected to keep incorrect behavior around for
compatibility?
@asmok-g, do you have any thoughts? How are breaking changes such as this
typically handled?
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -1359,17 +1361,18 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet
, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg) {
- // Find out which branch has the lock
- bool branch = false;
- if (const auto *BLE = dyn_cast_or_null(BrE))
-branch = BLE->getValue();
- else if (const auto *ILE = dyn_cast_or_null(BrE))
-branch = ILE->getValue().getBoolValue();
aaronpuchert wrote:
I kind of prefer the old code here. Regardless of what we accept as return
type, we only care about the return type converted to `bool`, so other values
don't really make sense as of now. (We need the `IntegerLiteral` for C.)
What we should document is that this isn't an equality comparison, but a
comparison after conversion to `bool`. (Or whatever the equivalent for C is.
Basically what is allowed in an `if` condition.)
Should we support integer/enumeration values properly, that would of course
change the picture. In that case we can allow enumerators in the attribute, and
the return type must be explicitly (?) convertible to type of the attribute.
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -1954,6 +1954,125 @@ struct TestTryLock {
} // end namespace TrylockTest
+// Regression test for trylock attributes with an enumerator success argument.
+// Prior versions of the analysis silently failed to evaluate success arguments
+// that were neither `CXXBoolLiteralExpr` nor `IntegerLiteral` and assumed the
+// value was false.
+namespace TrylockSuccessEnumFalseNegative {
+
+enum TrylockResult { Failure = 0, Success = 1 };
+
+class LOCKABLE Mutex {
+public:
+ TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
+ void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+class TrylockTest {
+ Mutex mu_;
+ int a_ GUARDED_BY(mu_) = 0;
+
+ void test_bool_expression() {
+if (!mu_.TryLock()) { // expected-note {{mutex acquired here}}
+ a_++; // expected-warning {{writing variable 'a_' requires holding
mutex 'mu_' exclusively}}
+ mu_.Unlock(); // expected-warning {{releasing mutex 'mu_' that was not
held}}
+}
+ } // expected-warning {{mutex 'mu_' is not held on every path through here}}
+};
+} // namespace TrylockSuccessEnumFalseNegative
+
+// This test demonstrates that the analysis does not distinguish between
+// different non-zero enumerators.
+namespace TrylockFalseNegativeWhenEnumHasMultipleFailures {
+
+enum TrylockResult { Failure1 = 0, Failure2, Success };
+
+class LOCKABLE Mutex {
+public:
+ TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
+ void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
+};
+
+class TrylockTest {
+ Mutex mu_;
+ int a_ GUARDED_BY(mu_) = 0;
+
+ void test_eq_success() {
+if (mu_.TryLock() == Success) {
+ a_++;
+ mu_.Unlock();
+}
+ }
+
+ void test_bool_expression() {
+// This branch checks whether the trylock function returned a non-zero
+// value. This satisfies the analysis, but fails to account for `Failure2`.
+// From the analysis's perspective, `Failure2` and `Success` are
equivalent!
+if (mu_.TryLock()) {
+ a_++;
+ mu_.Unlock();
+}
+ }
+};
+} // namespace TrylockSuccessEnumMultipleFailureModesFalseNegative
+
+
+// This test demonstrates that the analysis does not detect when all
enumerators
+// are positive, and thus incapable of representing a failure.
aaronpuchert wrote:
positive → non-zero or nonzero.
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -608,15 +606,31 @@ static bool checkTryLockFunAttrCommon(Sema , Decl *D,
const ParsedAttr ,
if (!AL.checkAtLeastNumArgs(S, 1))
return false;
- if (!isIntOrBool(AL.getArgAsExpr(0))) {
+ // The attribute's first argument defines the success value.
+ const Expr *SuccessArg = AL.getArgAsExpr(0);
+ if (!isa(SuccessArg) &&
aaronpuchert wrote:
I can see the appeal of `nullptr` in the attribute, but it suggests you can put
arbitrary pointers in there, and I don't think we want that. But I can live
with it.
Though it seems strange that one would return a pointer, and `nullptr` is
success. Maybe an error message? More typical is probably to have `nullptr` if
the lock couldn't be acquired.
In any event, since we want to able to express both the null and non-null case,
conversion to `bool` seems the better conceptual framework. How much we need
bother users with that framework is another question, but it's probably not a
bad idea to bring home the idea that we're not trying to track pointers.
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/aaronpuchert commented: Don't want to give the wrong impression, I like the idea of this change a lot. But I think we need to be clearer about the attribute, the return value, and the relation between them. The attribute needs to be a constant expression for sure, and it's probably Ok to limit it to literals. I don't see a reason to do any kind of computation in there, or have something dependent on other values in the code. This would seem to make for an awful interface. (But I'm happy to be proven wrong with compelling use cases.) The actual return value of the function need not be of the same type. Pointers have been mentioned, smart pointers as well. Handles could also be interesting. Anything that converts to `bool`, or more generally, the type in the attribute, should be fine. The idea being, as long as I can write ```c++ auto ret = mu.tryLock(); if (ret) /*...*/; ``` we should support it. This doesn't fit the `nullptr` case, but we could mildly generalize that. Or we ask users to write `false` instead, see also my comment below. https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
@@ -756,7 +771,68 @@ void etf_fun_params(int lvar
EXCLUSIVE_TRYLOCK_FUNCTION(1)); // \
// Check argument parsing.
-// legal attribute arguments
+int global_int = 0;
+int* etf_fun_with_global_ptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(_int,
); //\
+ // expected-error {{'exclusive_trylock_function' attribute requires
parameter 1 to be nullptr or a bool, int, or enum literal}}
+
+#ifdef CPP11
+constexpr int kTrylockSuccess = 1;
+int etf_succ_constexpr() EXCLUSIVE_TRYLOCK_FUNCTION(kTrylockSuccess, mu2); // \
+ // expected-error {{'exclusive_trylock_function' attribute requires
parameter 1 to be nullptr or a bool, int, or enum literal}}
+
+int success_value_non_constant = 1;
+
+bool etf_succ_variable()
EXCLUSIVE_TRYLOCK_FUNCTION(success_value_non_constant, mu2); //\
+ // expected-error {{attribute requires parameter 1 to be nullptr or a bool,
int, or enum literal}}
+#endif
+
+
+// Legal permutations of the first argument and function return type.
+struct TrylockResult;
+
+#ifdef CPP11
+int* etf_fun_with_nullptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(nullptr, );
+#endif
+
+#ifndef __cplusplus
+int* etf_fun_with_nullptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(NULL, );
+#endif
+
+int etf_succ_1_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
+bool etf_succ_1_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
+TrylockResult *etf_succ_1_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
+
+int etf_succ_true_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2);
+bool etf_succ_true_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2);
+TrylockResult *etf_succ_true_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2);
+
+int etf_succ_false_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2);
+bool etf_succ_false_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2);
+TrylockResult *etf_succ_false_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2);
+
+enum TrylockSuccessEnum { TrylockNotAcquired = 0, TrylockAcquired };
+int etf_succ_enum_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2);
+bool etf_succ_enum_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2);
+TrylockResult *etf_succ_enum_ret_p()
+EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2);
+
+#ifdef CPP11
+enum class TrylockSuccessEnumClass { NotAcquired = 0, Acquired};
+int etf_succ_enum_class_ret_i()
+EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+bool etf_succ_enum_class_ret_b()
+EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+TrylockResult *etf_succ_enum_class_ret_p()
+EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+#endif
aaronpuchert wrote:
I think `enum class` should not be allowed for now, because we can't convert it
to `bool`.
Should we add proper support for enumerations, we should probably not convert
them to `bool` but rather check for equality.
https://github.com/llvm/llvm-project/pull/95290
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
aaronpuchert wrote: Thanks for looping me in and sorry for the late reply! > However, I'm not certain we implement the analysis in an exception-aware way. If I remember correctly, the CFG is still without exception-handling edges by default, and support for those edges is still rudimentary and breaks other warnings that operate on the same CFG. It would definitely be nice, but it might be a bigger project. > I was really surprised to see a trylock method that returns a pointer! > Confusingly, the "success" parameter to the attribute is `1`. I think the > semantics work out so that returning a non-`nullptr` value indicates > successful mutex acquisition. We use something like this in our code base, though I believe we've written it as `true`. Basically, the function returns some kind of handle/ticket that needs to be passed into the unlock method. > Maybe we should be lenient and only enforce that the return type is boolean, > integer, or pointer? We can probably generalize this a bit. I think we should be checking for [contextually convertibility to `bool`](https://eel.is/c++draft/conv.general#def:conversion,contextual_to_bool). This would cover `bool`, pointers, smart pointers, and other custom types with an `explicit operator bool()`. Support for arbitrary integers/enumerations would also be nice, but it's a bit harder to track the data flow for them. We can probably only support them unchanged, whereas for booleans we currently allow negations and some comparisons. Didn't check how well this is supported. https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
asmok-g wrote: https://godbolt.org/z/s6nTh6xxd does this serve ? sorry if it's a bad code.. but it serves the purpose I think. https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
asmok-g wrote: Sorry for late reply. Sending in minutes! https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
dmcardle wrote: Hi @asmok-g, it was definitely not my intention to break any use cases where the analysis was behavior properly! I wanted to verify that the prior release, which predates this change, understands smart pointer returns on trylock functions, but I'm having trouble finding a test case where the analysis behaves that way. In this [example on godbolt.org] with Clang 18.1.0, I mocked up (1) a plain `unique_ptr` return value and (2) a nullable `unique_ptr` type alias. It looks like the analysis incorrectly emits an error for unguarded access in both scenarios. Could you help me construct an example of something that used to work and is now broken? Regardless, it still might make sense to add support for any return type that can be implicitly converted to bool. [example on godbolt.org]: https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIMxqkrgAyeAyYAHI%2BAEaYxCAArAmkAA6oCoRODB7evv6BaRmOAqHhUSyx8Um2mPbFDEIETMQEOT5%2BATV1WY3NBKWRMXGJyQpNLW15nWN9A%2BWVIwCUtqhexMjsHAD0WwDUALJMyMTpu0S7jEzR9GeYY2HAu6hUu9GoBAi7AG7NeIYECieLw%2BmDOCGImCY6F2CiYVEwBAAnrtDGJERkFAA6EwaACCOPxZgAzHgXnIhNgAPpuXHKXEWACSwQZABUAJoE8xErA0cK7YIAeTcAGl6cFsLtJVLpTLZVLKZSmAQCMQ8NEvARMAqoKIUlc6IRERBzGYWBrMKoTYtrZziTywqDcUIKQAlFmU7AADTcwXJDIAalTBSKIJiw4tZQqlSq1ebtRAmAolC1Kbr9bRDRAFf7cZTcS6AOJCBXWxa27mYXmO53YN2UoQACXz2AAIpTg8LQ%2BG5ZKo8rVerNfHE8mCJSFAhmph0KmmHrogakVnKTm84Xi5TS%2BX7XyvT6/YH20LhZSAGJyCJuFkMgURLuYiPyxX92NDylQI4ARy8eAhs/ni5Gtmub5kWJY2niXI7qCjbNm2HZnheV43neYYPnKfYxoOWrvgmyDfr%2BWoTlOM5pguGZLsBa5gZuEGEhWVa7BeCHnpe163vej49r2z5YXGuEQvQiZasAjBxHgyD/ummZUaBG5bpBdqVg6ux7r6QgBlSLIumyLFIexqHdphA78VAKqIoq%2BE/n%2BZGAcuq5yeBZaKQxKmwS6raUtpunHohbEoZx0rGa%2BOFmcQFlftZRGThCpFztJlEriB65Oduym7t66maUeIrFh5ACKcgMh5LaBTx0YmW%2BUAQgREIKFJ5EyUl1HyXRUHpTBTYlTlwp5dghXFa2ZVBbxlWhRANVRfVxGxQ1dmySltHOfRtRKGljEdmKErcTtI0VSF8a0KgyAANZXPQbVKYxTquu6akHkGx7DU%2B%2B3YcOSZxGOFrIN4GRfFqR2nfZyU0Qp9HQbsN21u67meR2z0ysFb24SOn3jjF06UoDJ3Ay1qUuRD90aYeen%2BRxaFcbsSOmRA32/Xg/1Y8dJ2UlQXgMA4WS445S3rW5XVw75rHIeT3aRqNB24TNmPY6z7OcwI3OLWD7WMcxQv6QFFO7VTEvI1A7Oy2zHP1EroOXa5GX7sTWk6aTIuGeh1NVbTqg/V4f1auZRvy6bC3m8tqv83BXl2xrZOO5TzvjdLM7e8zcsm1z/utYHV0qUT2Udn1A0lcN0fxnTHsMwDzP1ZNhHoGbqd83ysPwceOdFXn2uI3rNOx0zp3l5gtXTtX%2BMrQw%2BBUJyeI7PyzMwpgLD/BJgICLQiK16CBZyPmLaeTmLo69xBe4cAXjNFgM4/MQFsQ8o7prxvW/5rvbevTTKRjofx%2BY2fbXj3sbjxY1jh3CeAwJeK9VKZQepSdWIo/IOzKvvaqtRIRKDmhRICzUeYq3TnXAWbYoEnmFgZBG8CJqIOEujEiKCmoOWVhfDquwb4uk3m2CwbIICqEpg/XWT8XZv2ICfSk0QjTsNoYxK%2BlIGFMMpCwthHDdrEJfpSXh/DBEyK/riCeHgWAsAEKAoQbgBTKEFiKLanDyovn1hABQaAUgy2ZudTAIiVK4jcLnTyuJTwslrAjHsxDIqV0VFQTUxAB68wJnQ5xrjmHYFPAKDy3jxbcPGn42asR%2BAQhCZgy2oIEIeRZHIF0EQhrsNMcQ2WEICBrHCFXYRacsn8kbh6cBTD4kJPMTTbG9Ui4nwyY4vkEQBReQbB5XEbYhDuOwOyPMERcTBDZBpIQGF24uwYKgSkHwIRQnHHCBEEVmBLwxGorkYR3ZYF2CYIkbg2DaPCuc7AY9cTMDYAoPUGxdgpAMGESk7M8Dfi1C/YgZyADsVhIJ4h%2BiOepxiLDigOOaVQQKQW4hSF4a4EkQAEklGMdAIAQDfN%2BZSf55y3BhAILcyULJwrBGZhACMmdDzeXtoQlUXgHHnMRZKL4qA8DQjkMA6ltLwE20gRERlAUyxEkRSYQFLY2X3PBUmXYFLETYxZHcAgCKMWws1PCs0lJZV4klCSlElJ6Hr0YZ5aRuqIznJbLsDQ%2Br8QGu%2BFy6EmoxgCNQJ4SkFoUh1QyIra1wLNWGpeBAXVmIlVUqBtaDVTrpRMD1ZYawDrZTht5djGlKapRSplU6nNnJpUOpzbsCe7ymCfLxSyglKp7mPLuC80EDAvC0AMNcL2iIbGKgzImWNuJi5GF2FEFt9iGSCGUCqM5CQrBTvBUYHFhgVlNE1MaMwZgh2tvoFaEwCQZU7rOUSW1WKcWVr%2BTWi5JLblFrBQYBVm1oUSn2HC3tyLUXIHRU69dI6x0TsjfysB1tsoMvDrA5lrKJWas5dypifLo3/qyoePBMDCHislYW8DoLcTysBEqlVare2Skfdq3Yuqs1GoTaa2%2BzDWFWv3ba%2B16HHW4g5S6247q3hep9X6rmgb2VSlJLsMNXhKQRspfynjwapQJuTUmhjqahOYnTfyrNkp815sLZBNDqHbUTybcOttayO1ajEH8BQBIODLFoJwBIvA/DcF4KgTgbhpMWBhKsdYoIuQ8FIAQTQ5nlgnRAJIQFmJiSSEkESVdRIwsRbMNISzHBJA2d86QBzHBeAKBAIEHzHAtDLDgLAJAaAWApDoHEcglAislfoPEWdwAuAAA4uCBBoLQIJGWIDRGSwuZg4VOBee680REApojaEwA4PrvAitsEEAKYBiJktYHVMAX%2BLaMt2dIFgWeRhxA5d4PgCEnN/pra0EEVQY3zQTfIIIWoyWMzRGIINjwWBksxhYJd/6xA3hKBbDPQwwAMxzt28sKgBhgAKH9HgTAAB3AUNjbNef4IIEQYh2BSBkIIRQKh1C7dILoLg%2Bg/soGc/oNUGXIDLFQC/LIa2AC0WKbWmCTZYLgXBdg04AOpxBOMQG16zIToDp9spEbOWzkipDSOkjJmTshtV51AH3VRYDJzSroY36guGHpMPw%2BOQgOnmMMfHhRMgCC13oI39Q5hDHiPjuwauejjFaJ4doehbcKwaA7y3FQDe2Ad6bm3Hu9dW4kMsBQbmNjB/0FZpLOPUu7FUPVgAbDThPkhdi1d2A1zEXBMQaAE7gQgJAznEi4IsXg2XcvLAQPz4YKuAtcAAJyYnC5F6LLfwuR4S9Hk7qX0uZe875vLMBEAgFWAQZFBBysQEq6V4gERWCbHj0nlPaeDADogMyhgJ1S9BHwEQRXehEfCFEOINHh/MdqGS3j0gUOHspAmxZqPpBbPd84AKDU4%2BgRx8T8n1P6fM/Z9zwgE0SqziCLyJBLzLwH38xACJCJExFgIQMQKQI70Syf2Sx71sD73Lz8w7zMC73s04EgKB2WA%2B39T8EkCAA%3D%3D%3D https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
asmok-g wrote: Hi! Is it expected for this to fail with unique_ptrs ? We have a case in the code where the return type is an alias to unique_ptr and it's failing. Also, cases where the return type is something like ``` absl::Nullable> ``` that is failing. Is that expected ? https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
github-actions[bot] wrote: @dmcardle Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/AaronBallman approved this pull request. LGTM still https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/dmcardle edited https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/dmcardle updated https://github.com/llvm/llvm-project/pull/95290 >From 807953fc74920e3d5ed727a00c0e1177870c56fe Mon Sep 17 00:00:00 2001 From: Dan McArdle Date: Thu, 20 Jun 2024 17:43:16 -0400 Subject: [PATCH] [clang][ThreadSafety] Check trylock function success and return types With this change, Clang will generate errors when trylock functions have improper return types. Today, it silently fails to apply the trylock attribute to these functions which may incorrectly lead users to believe they have correctly acquired locks before accessing guarded data. As a side effect of explicitly checking the success argument type, I seem to have fixed a false negative in the analysis that could occur when a trylock's success argument is an enumerator. I've added a regression test to warn-thread-safety-analysis.cpp named `TrylockSuccessEnumFalseNegative`. This change also improves the documentation with descriptions of of the subtle gotchas that arise from the analysis interpreting the success arg as a boolean. Issue #92408 --- clang/docs/ReleaseNotes.rst | 10 ++ clang/docs/ThreadSafetyAnalysis.rst | 52 +++- .../clang/Basic/DiagnosticSemaKinds.td| 23 +++- clang/include/clang/Sema/ParsedAttr.h | 2 + clang/lib/Analysis/ThreadSafety.cpp | 82 +++- clang/lib/Sema/SemaDeclAttr.cpp | 34 +++-- clang/test/Sema/attr-capabilities.c | 12 +- .../SemaCXX/warn-thread-safety-analysis.cpp | 123 +- .../SemaCXX/warn-thread-safety-parsing.cpp| 107 --- clang/unittests/AST/ASTImporterTest.cpp | 6 +- 10 files changed, 369 insertions(+), 82 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7ac0fa0141b47..28ec3d54a5a77 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -71,6 +71,11 @@ C++ Specific Potentially Breaking Changes To fix this, update libstdc++ to version 14.1.1 or greater. +- Clang now emits errors when Thread Safety Analysis trylock attributes are + applied to functions or methods with incompatible return values, such as + constructors, destructors, and void-returning functions. This only affects the + ``TRY_ACQUIRE`` and ``TRY_ACQUIRE_SHARED`` attributes (and any synonyms). + ABI Changes in This Version --- - Fixed Microsoft name mangling of implicitly defined variables used for thread @@ -720,6 +725,11 @@ Bug Fixes in This Version - Fixed `static_cast` to array of unknown bound. Fixes (#GH62863). +- Clang's Thread Safety Analysis now evaluates trylock success arguments of enum + types rather than silently defaulting to false. This fixes a class of false + negatives where the analysis failed to detect unchecked access to guarded + data. + Bug Fixes to Compiler Builtins ^^ diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..0ecbebe7a692f 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -420,10 +420,17 @@ TRY_ACQUIRE(, ...), TRY_ACQUIRE_SHARED(, ...) *Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION`` These are attributes on a function or method that tries to acquire the given -capability, and returns a boolean value indicating success or failure. -The first argument must be ``true`` or ``false``, to specify which return value -indicates success, and the remaining arguments are interpreted in the same way -as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses. +capability, and returns a boolean, integer, or pointer value indicating success +or failure. + +The attribute's first argument defines whether a zero or non-zero return value +indicates success. Syntactically, it accepts ``NULL`` or ``nullptr``, ``bool`` +and ``int`` literals, as well as enumerator values. *The analysis only cares +whether this success value is zero or non-zero.* This leads to some subtle +consequences, discussed in the next section. + +The remaining arguments are interpreted in the same way as ``ACQUIRE``. See +:ref:`mutexheader`, below, for example uses. Because the analysis doesn't support conditional locking, a capability is treated as acquired after the first branch on the return value of a try-acquire @@ -445,6 +452,43 @@ function. } } +Subtle Consequences of Non-Boolean Success Values +^ + +The trylock attributes accept non-boolean expressions for the success value, but +the analysis only cares whether the value is zero or non-zero. + +Suppose you define an enum with two non-zero enumerators: ``LockAcquired = 1`` +and ``LockNotAcquired = 2``. If your trylock function returns ``LockAcquired`` +on success and ``LockNotAcquired`` on failure, the analysis may fail to detect +access to guarded data without holding the mutex because
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/dmcardle updated https://github.com/llvm/llvm-project/pull/95290 >From 66715ea5566946f0608d77dc4c86b23e70179760 Mon Sep 17 00:00:00 2001 From: Dan McArdle Date: Thu, 20 Jun 2024 17:43:16 -0400 Subject: [PATCH] [clang][ThreadSafety] Check trylock function success and return types With this change, Clang will generate errors when trylock functions have improper return types. Today, it silently fails to apply the trylock attribute to these functions which may incorrectly lead users to believe they have correctly acquired locks before accessing guarded data. As a side effect of explicitly checking the success argument type, I seem to have fixed a false negative in the analysis that occurred when a trylock's success argument is negative. I've added a regression test to warn-thread-safety-analysis.cpp named `TrylockSuccessEnumFalseNegative`. This change also improves the documentation with descriptions of of the subtle gotchas that arise from the analysis interpreting the success arg as a boolean. Issue #92408 --- clang/docs/ReleaseNotes.rst | 10 ++ clang/docs/ThreadSafetyAnalysis.rst | 52 +++- .../clang/Basic/DiagnosticSemaKinds.td| 23 +++- clang/include/clang/Sema/ParsedAttr.h | 2 + clang/lib/Analysis/ThreadSafety.cpp | 82 +++- clang/lib/Sema/SemaDeclAttr.cpp | 34 +++-- clang/test/Sema/attr-capabilities.c | 12 +- .../SemaCXX/warn-thread-safety-analysis.cpp | 123 +- .../SemaCXX/warn-thread-safety-parsing.cpp| 107 --- clang/unittests/AST/ASTImporterTest.cpp | 6 +- 10 files changed, 369 insertions(+), 82 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7ac0fa0141b47..28ec3d54a5a77 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -71,6 +71,11 @@ C++ Specific Potentially Breaking Changes To fix this, update libstdc++ to version 14.1.1 or greater. +- Clang now emits errors when Thread Safety Analysis trylock attributes are + applied to functions or methods with incompatible return values, such as + constructors, destructors, and void-returning functions. This only affects the + ``TRY_ACQUIRE`` and ``TRY_ACQUIRE_SHARED`` attributes (and any synonyms). + ABI Changes in This Version --- - Fixed Microsoft name mangling of implicitly defined variables used for thread @@ -720,6 +725,11 @@ Bug Fixes in This Version - Fixed `static_cast` to array of unknown bound. Fixes (#GH62863). +- Clang's Thread Safety Analysis now evaluates trylock success arguments of enum + types rather than silently defaulting to false. This fixes a class of false + negatives where the analysis failed to detect unchecked access to guarded + data. + Bug Fixes to Compiler Builtins ^^ diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..0ecbebe7a692f 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -420,10 +420,17 @@ TRY_ACQUIRE(, ...), TRY_ACQUIRE_SHARED(, ...) *Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION`` These are attributes on a function or method that tries to acquire the given -capability, and returns a boolean value indicating success or failure. -The first argument must be ``true`` or ``false``, to specify which return value -indicates success, and the remaining arguments are interpreted in the same way -as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses. +capability, and returns a boolean, integer, or pointer value indicating success +or failure. + +The attribute's first argument defines whether a zero or non-zero return value +indicates success. Syntactically, it accepts ``NULL`` or ``nullptr``, ``bool`` +and ``int`` literals, as well as enumerator values. *The analysis only cares +whether this success value is zero or non-zero.* This leads to some subtle +consequences, discussed in the next section. + +The remaining arguments are interpreted in the same way as ``ACQUIRE``. See +:ref:`mutexheader`, below, for example uses. Because the analysis doesn't support conditional locking, a capability is treated as acquired after the first branch on the return value of a try-acquire @@ -445,6 +452,43 @@ function. } } +Subtle Consequences of Non-Boolean Success Values +^ + +The trylock attributes accept non-boolean expressions for the success value, but +the analysis only cares whether the value is zero or non-zero. + +Suppose you define an enum with two non-zero enumerators: ``LockAcquired = 1`` +and ``LockNotAcquired = 2``. If your trylock function returns ``LockAcquired`` +on success and ``LockNotAcquired`` on failure, the analysis may fail to detect +access to guarded data without holding the mutex because they are
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
dmcardle wrote: So... I appear to have accidentally fixed a bug. Looks like enum success values were not being evaluated, and defaulted to false. As a result, the analysis could fail to detect unguarded access [[demo on godbolt.org](https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIAMwA7KSuADJ4DJgAcj4ARpjEIABsAKykAA6oCoRODB7evgHBmdmOAuGRMSzxiam2mPZlDEIETMQE%2BT5%2BQfWNuS1tBBXRcQnJaQqt7Z2FPZODw1U14wCUtqhexMjsHAD0uwDUALJMyMRZB0QHjEyx9JeYkxHAB6hUB7GoBAgHAG5teEMBAUr3e30wlwQxEwTHQBwUTComAIAE8DoYxCjsgoAHQmDQAQXxRLM/jw7zkQmwAH03ATlASLABJUJMgAqAE1ieZ/FgaJEDqEAPJuADSjNC2AO0plsrl8pl1OpTAIBGIeFiXgImCVUFE6VudEIKIg5jMLC1mFUZpWtu5pL5EQhBKEVIASmzqdgABpuUKUpkANRpwrFEBxEZW8qVKrVGstuogTAUSna1P1htoxogSsDBOpBLdAHEhErbSt7bzMPzna7sB7qUIABKF7AAEWpodF4cjCulMdV6s12sTydTBGpCgQbUw6HTTANsSNqJz1LzBeLpep5crjoFPr9AeDnZFoupADE5FE3GymUKoj2cVHFcrB/GR9SoKcAI5ePDQ%2BdF2XE1c3zQsSzLO1CR5PcIWbVsOy7C8rxvO8HwjJ8FQHONhx1T8k2QX9/x1KcZznDMlyzFdQI3CDtygkkqxrA4ryQy9r1ve9H2fPt%2B1fHCE3w6F6GTHVgEYBI8GQQDM2zGjwK3HdoIdasnQOA9/SEIMaTZN0OTYlDOPQ3tsKHQSoDVFFlUIv8AIo4DV3XBTIIrZSmLU%2BC3XbaldP009kI4tDuNlUz3zwiziCsn9bJI6doXIhdZOotcwM3Fzd1U/dfU07STzFUsvIARTkJkvLbYK%2BNjMyPygaEiOhBQZMouSUtoxSGJgzK4JbMq8tFArsGK0r2wqkL%2BOq8KIDqmLGtI%2BKmoc%2BS0vo1zGIaJQMuYrsJSlXi9rGqqwsTWhUGQABrW56A6lTmJdd1PQ0o8Q1PUaX0O3DRxTBIJytZBvGyX4dRO87HNSuilMY2CDju%2BtPU87yu1euVQo%2B/Cx2%2Byc4tnalgbO0G2vStyocerTjwMwKuIwniDhR8yIF%2B/68EBnHTrO6kqC8BgHFyfHnJWzaPJ6hH/PY1DKd7aNxqO/C5ux3H2c57mBF55aIc65jWJFwygqp/aaal1GoE5%2BWOa5poVfB673Kyw9SZ0vTybF4zMNpmr6dUP6vABnVLJNxXzaWy3VvVwWEJ8h2tYp53qddybZbnX3WYVs2ecD9rg5utSSdyrsBqGsrRtjxMGa9pmgdZxrpuI9ALfTgWBXhxDTzzkqC915GDbp%2BOWfOyvMHq2da8JtaGHwKhuUJfZBVZ%2BFMBYIEpJBARaBReuISLORCzbby8zdPXeKL/DgC8NosDnf5iCtqHlE9Det53wt947966fSCdj9P7GL46yfDjcRLmqOEeK8BgK817qWyk9akmsxQBSdhVQ%2BtUGgwiUAtKiIFWp8zVpnBuQsOwwLPKLIySNEFTWQaJTGZE0EtScqrK%2BXUDh3zdNvDsFgOQQFUNTJ%2B%2BsX5uw/sQM%2B1JYgmk4fQ5iN9qRMJYdSNhHCuH7VIW/ak/DBHCLkT/AkU8PAsBYAIcBQg3BCmUMLMUO1uGVTfIbCACg0DpDlqzS6mAxFqQJG4fO3kCTnjZPWJGfZSHRWrsqKg2piBD35kTBhrj3GsOwOeIUXlfGS14ZNAJ814j8GhGE7B1sIRIS8myOQboogjU4eY0h8toQEE2JEGuoiM45MFM3L0kCWGJKSZYumuNGolzPlk5xAoohCh8k2LyBIOxCE8dgTkBYogElCByLSQgsKdzdgwVA1JvjQlhJOREyIorMBXtiDRU83SYGAA1bIAgHiTAOBky4kVcbog6dqEEAB3QgPxDDXAYD4BIKoSDwi8MgbYKZ0TEGPmwQQeJf4HGUOqAFgNiCXIYEvMECAIQYkOXgEE2R6CCBXrcpgdBZyXFQNcf43gVQQgUECkFII2gQsYMCYkU9vgqgOK8hIEJIgfISAcEwSQNBuG9N6CwqBPDhBCWIbAqh0jEAFRoA4aziD8sFUyQQZyEiSr%2BbQBV6JR7ohTL8uE4IWWHApV4CErzkyEtoEoaFBJmBsAUAabYBw2QPNZkIWljwFDYB%2BSwc8YglBRDOSqMu/LAhWGgoSRgPh3WevOqcmltACCRosAcINdBNgQhMP4NsBwNCkAON64Fvr%2BX5oOFwSNbY83RqJISP6Y5GmmIsJKY4lpVDpuJOkLwdwpIgGJNKD1KJcbJq8KmhNKJQiswgFGbOx5fKO2IaWulFZ/D1ulL8VAeA4RyFAbO%2BdkC7bQKiMuoK6760mECLWjdE8CRNtBSO3GbJHhpuvZujt2ou0WmpHWodBwIhpqYNSRhm9mHeVkb%2BqMeaC0aH/TGgkW6d0mrfUI8VtBqRWjlb6nmMGo0AelOSA4EAwBgF/TiEdM6Qa2nTdKKeUQvgQnBIBhgiLtRwliMQQwyAfhoFHjkAQDr5QgesGJjd5ip5sgQNi%2BECANi0E4xCKgBhgDiU42iFj0mtlwiELs1E0MDlYmxWRwjMoKP7txnOhDSGZTXtrYSaU9nuQ3ps/Zg4U8nWPFdRCZ9XqfUpn9T4INdrohhscIDDgaxaCcBSLwPw3BeCoE4G4MTlh4QbC2Lm0kPBSAEE0FFtYZ0Aj%2BBxP4crFXKuVaSPoTgkh4sFdIMljgvAFAgCLfljgWg1hwFgEgNALB0jErIBQCAA2hv0ESE2owGziCczOnwOgIS2sQFiI1pczBIqcFyxttoKIhSxG0JgBw23eADchQQIUoCUSNawJqYA/9aB2tO6QLA88jDiC67wfA0JuaAza19kIqhjuWhe0BhojWsxcb2x4LAjW4wsBe4iz4Sg2xz0MMALMRgCtrBU0wYAChAx4EwK8oUdiEu5f4IIEQYh2BSBkIIRQKh1CA90FwfQGOUBpZsFDtrkA1ioDfrkAHABaSY6BYOmEsNYLg1aRcAHUEjnHlfmzZMJ0Bi/02iEXbZKQ0jpAyZkrJOSwdy6gRF6osB87nb0Y7TQXCjxmH4dnYQnRLDGOzkogm8ieC6HoL3TRFijESOzuwdv%2BhTA6L7woofkFK2aJHoP1QPe2Ej07vQ8x2hJ%2BWFwNYChMvbAkNF2LDXAfNYOKoAAHEkEXSRJAHGmy8CAap5tRggLgQgAKeS594J17rax0WwjGDb4r5WytVYn%2BVmrMWOD1dIAlrQTXOCtfa3lnHpBeuIBABsAgvaCDkEoON4bURWA7CrzXuvDeDBGBIy3hgZ0Vi8FnJ3y3egqfCFEOIen7%2BmdqEa2z0gV5bjdIU7YvDgOLefRrZrIULUPfUECvavWvevRvW/Obe/NvbRCbPlbvR/NfL7W0UgUfUrSfSfWrWfUvRfZrFfDrHHMAswCgpLZfPA/vUgRFZFEASQIAA%3D%3D%3D)]. I think that the old `isIntOrBool()` function accidentally let enumerator values through, but the analysis assumed it would only ever see `CXXBoolLiteralExpr ` or `IntegerLiteral `. I'm adding a regression test that closely resembles the demo, but verifies that the analysis actually detects the unguarded access. I also realized I was supposed to update the release notes. New patch with these changes coming momentarily. https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
AaronBallman wrote: > > LGTM -- that new documentation is fantastic, thank you for that! > > Thank you! I hope it saves future readers some time. > > I don't have write access, so feel free to merge unless we're waiting for > @aaronpuchert to review :) I'll give Aaron a chance to weigh in and land the changes on your behalf early next week if we don't hear back from him. https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
dmcardle wrote: > LGTM -- that new documentation is fantastic, thank you for that! Thank you! I hope it saves future readers some time. I don't have write access, so feel free to merge unless we're waiting for @aaronpuchert to review :) https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/AaronBallman approved this pull request. LGTM -- that new documentation is fantastic, thank you for that! https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)
https://github.com/dmcardle edited https://github.com/llvm/llvm-project/pull/95290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
