[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)

Aaron Puchert via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 30 15:46:08 PDT 2024


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


More information about the cfe-commits mailing list