Thread safety analysis incorrectly warns when using `std::scoped_lock` with multiple locks

[GodTamIt]

Bugzilla Link	42655
Version	trunk
OS	All
CC	@AaronBallman,@aaronpuchert,@zygoloid,@frobtech

Extended Description

Example: https://godbolt.org/z/d3qZS6

When using the std::scoped_lock constructor to acquire multiple locks, thread safety analysis seems to think none of the locks are taken in the ensuing block.

[AaronBallman]

I think that will need some changes to Clang, because we seem unable to parse a pack expression inside an attribute:

[...]/libcxx/test/libcxx/thread/thread.mutex/thread_safety_lock_guard.pass.cpp:20:
[...]/include/c++/v1/mutex:520:105: error: expected ')'
    explicit scoped_lock(_MArgs&... __margs) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__margs...))
                                                                                                        ^
[...]/include/c++/v1/mutex:527:111: error: expected ')'
    scoped_lock(adopt_lock_t, _MArgs&... __margs) _LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__margs...))
                                                                                                              ^

@AaronBallman, any idea how hard that would be? Perhaps we would also need to teach template instantiation to look into these attributes?

We currently don't support any attribute parameter packs, so I suspect there's some effort to add here. It'd mostly be around the template instantiation logic to ensure we expand the parameter packs, but there may be some bits around parsing as well.

[aaronpuchert]

Strangely it parses without "...", but then template instantiation doesn't work:

CXXConstructorDecl scoped_lock<_MArgs...> 'void (_MArgs &...)'
|-ParmVarDecl 0x168e6a0 referenced __margs '_MArgs &...' pack
`-AcquireCapabilityAttr acquire_capability
  `-DeclRefExpr '_MArgs' lvalue ParmVar 0x168e6a0 '__margs' '_MArgs &...'
       non_odr_use_unevaluated

instantiated as:

CXXConstructorDecl used scoped_lock 'void (std::mutex &, std::mutex &)'
|-ParmVarDecl referenced __margs 'std::mutex &'
|-ParmVarDecl referenced __margs 'std::mutex &'
`-AcquireCapabilityAttr acquire_capability
  `-FunctionParmPackExpr '_MArgs' lvalue

So then we get warning: cannot resolve lock expression [-Wthread-safety-analysis], and the mutexes are not considered to be held.

Though template instantiation is not the actual issue here: since we have no ellipsis, there is nothing to expand.

[aaronpuchert]

I think that will need some changes to Clang, because we seem unable to parse a pack expression inside an attribute:

[...]/libcxx/test/libcxx/thread/thread.mutex/thread_safety_lock_guard.pass.cpp:20:
[...]/include/c++/v1/mutex:520:105: error: expected ')'
    explicit scoped_lock(_MArgs&... __margs) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__margs...))
                                                                                                        ^
[...]/include/c++/v1/mutex:527:111: error: expected ')'
    scoped_lock(adopt_lock_t, _MArgs&... __margs) _LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__margs...))
                                                                                                              ^

@AaronBallman, any idea how hard that would be? Perhaps we would also need to teach template instantiation to look into these attributes?

[frobtech]

Petr has a change in progress for this.

[GodTamIt]

assigned to @petrhosek

[AaronBallman]

This should be a bit easier to now because we added support for this recently: ead1690d31f815c00fdd2bc23db4766191bbeabc