SCOPED_CAPABILITIES think that capabilities REQUIRED during construction are released by methods marked RELEASE

[johngro]

Repro Steps

Reproduced so far using the following versions of Clang.

1. The version of clang currently distributed with the Fuchsia project at Google.
2. x86-64 clang (trunk) on godbolt.org
3. x86-64 clang 16.0.0 on godbolt.org
4. x86-64 clang 15.0.0 on godbolt.org
5. armv8-a clang (trunk) on godbolt.org
6. armv8-a clang 16.0.0 on godbolt.org
7. armv8-a clang 15.0.0 on godbolt.org

arguments are --std=c++17 -Wall -Wthread-safety-analysis -Werror

To reproduce, compile the following file scoped_capability_releases_required_capability.cc.txt, but be sure to change the extension back to just .cc and build using clang and the arguments above.

This example file contains a description of the setup which show various uses of the Clang Thread Safety Analysis feature. Some of the examples here succeed/fail as they should. The example starting on line 231, however, fails when it should not. Please make sure that the #if 0 gating this failure case has been changed to an #if 1 in order to successfully reproduce the issue.

For the most simple repro case, please use scoped_capability_releases_required_capability_extra_simple.cc.txt
instead. This version contains only the most simple repro and none of the extra comments or examples.

Background

Clang Thread Safety Analysis allows users to annotate their code statically in order to enforce invariant related to concurrency and thread safety. These are modeled as "capabilities" which must be explicitly held (or explicitly not held) in various scenarios defined by the static annotations. For example, a structure might have a lock_ member which is an instance of a class which has been properly annotated to be a capability, and a member variable which is annotated as GUARDED_BY(lock_). Attempts to access the variable when the analyzer cannot statically demonstrate that the lock must have been acquired will result in a warning.

In addition to annotating variables as GUARDED_BY, methods may be annotated to indicate that they REQUIRE or EXCLUDE sets of capabilities as well, allowing capabilities to be acquired and held several stack levels above where a GUARDED_BY variable may be accessed.

In addition to this, the analyzer allows users to introduce SCOPED_CAPABILITIES, which are RAII style "guards" who typically acquire one or more capabilities during construction, and then release whatever capabilities were acquired either on destruction, or early if a properly annotated method of the guard is used to release the capabilities instead. See here for the documentation for SCOPED_CAPABILITY

Documentation for SCOPED_CAPABILITY says

Scoped capabilities are treated as capabilities that are implicitly acquired on construction and released on destruction. They are associated with the set of (regular) capabilities named in thread safety attributes on the constructor or function returning them by value (using C++17 guaranteed copy elision). Acquire-type attributes on other member functions are treated as applying to that set of associated capabilities, while RELEASE implies that a function releases all associated capabilities in whatever mode they’re held.

An example is also provided later on in the docuementation, but (assuming that Lock is properly annotated) a user might say something like

struct SCOPED_CAPABILITY MyGuard {
  MyGuard(Lock& lock) ACQUIRE(lock) lock_(lock) { lock_.Acquire(); }
  ~MyGuard() RELEASE() { lock_.Release(); }
  Lock& lock_;
};

// An then later on in code

uint32_t AccessGuardedVariable() {
  MyGuard guard{lock_we_need};
  return guarded_variable;
}

The Issue

The issue here is that capabilities which are REQUIRED (but not ACQUIRED) during construction are considered to be among the set of capabilities which are also released during destruction (or any method annotated with a parameter-less RELEASE() annotation).

The example attached in the Repro Steps section shows a very simple example of this, along with a practical use case of where this matters. Basically, it shows a situation where we have a spinlock modeled as a capability which requires interrupts to be disabled in order to be used correctly (think of a kernel programming environment in this situation, but there are others where it might apply). Interrupts being disabled are also modeled as a capability, but attempting to create a guard which acquires and releases spinlocks, but demands that interrupts be disabled during construciton/destruction, fails because the guard thinks that it is re-enabling interrupts when it destructs.

Expected Behavior.

Only capabilities which are ACQUIRED during construction (or other methods of the scoped capability class, according to documentation) should be assumed to have been RELEASED during destruction (or any other method of the scoped capability class annotated with RELEASE()). Capabilities which are REQUIRED during construction of a scoped capability should still be required, but not assumed to be released later on.