Thread Safety Analysis: Support reentrant capabilities

Introduce the `reentrant_capability` attribute, which may be specified
alongside the `capability(..)` attribute to denote that the defined
capability type is reentrant. Marking a capability as reentrant means
that acquiring the same capability multiple times is safe, and does not
produce warnings on attempted re-acquisition.

The most significant changes required are plumbing to propagate if the
attribute is present to a CapabilityExpr, and then introducing a
ReentrancyCount to FactEntry that can be incremented while a fact
remains in the FactSet.

Author: melver

clang/docs/ReleaseNotes.rst

@@ -427,6 +427,7 @@ Improvements to Clang's diagnostics
   as function arguments or return value respectively. Note that
   :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
   feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- The :doc:`ThreadSafetyAnalysis` now supports reentrant capabilities.
 - Clang will now do a better job producing common nested names, when producing
   common types for ternary operator, template argument deduction and multiple return auto deduction.
 - The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers

clang/docs/ThreadSafetyAnalysis.rst

@@ -434,6 +434,21 @@ class can be used as a capability.  The string argument specifies the kind of
 capability in error messages, e.g. ``"mutex"``.  See the ``Container`` example
 given above, or the ``Mutex`` class in :ref:`mutexheader`.
 
+REENTRANT_CAPABILITY
+--------------------
+
+``REENTRANT_CAPABILITY`` is an attribute on capability classes, denoting that
+they are reentrant. Marking a capability as reentrant means that acquiring the
+same capability multiple times is safe. Acquiring the same capability with
+different access privileges (exclusive vs. shared) again is not considered
+reentrant by the analysis.
+
+Note: In many cases this attribute is only required where a capability is
+acquired reentrant within the same function, such as via macros or other
+helpers. Otherwise, best practice is to avoid explicitly acquiring a capability
+multiple times within the same function, and letting the analysis produce
+warnings on double-acquisition attempts.
+
 .. _scoped_capability:
 
 SCOPED_CAPABILITY
@@ -846,6 +861,9 @@ implementation.
   #define CAPABILITY(x) \
     THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
 
+  #define REENTRANT_CAPABILITY \
+    THREAD_ANNOTATION_ATTRIBUTE__(reentrant_capability)
+
   #define SCOPED_CAPABILITY \
     THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
 

clang/include/clang/Analysis/Analyses/ThreadSafety.h

@@ -94,16 +94,17 @@ enum AccessKind {
 
 /// This enum distinguishes between different situations where we warn due to
 /// inconsistent locking.
-/// \enum SK_LockedSomeLoopIterations -- a mutex is locked for some but not all
-/// loop iterations.
-/// \enum SK_LockedSomePredecessors -- a mutex is locked in some but not all
-/// predecessors of a CFGBlock.
-/// \enum SK_LockedAtEndOfFunction -- a mutex is still locked at the end of a
-/// function.
 enum LockErrorKind {
+  /// A capability is locked for some but not all loop iterations.
   LEK_LockedSomeLoopIterations,
+  /// A capability is locked in some but not all predecessors of a CFGBlock.
   LEK_LockedSomePredecessors,
+  /// A capability is still locked at the end of a function.
   LEK_LockedAtEndOfFunction,
+  /// A capability is locked with mismatching reentrancy depth in predecessors
+  /// of a CFGBlock.
+  LEK_LockedReentrancyMismatch,
+  /// Expecting a capability to be held at the end of function.
   LEK_NotLockedAtEndOfFunction
 };
 

clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

@@ -22,6 +22,7 @@
 #define LLVM_CLANG_ANALYSIS_ANALYSES_THREADSAFETYCOMMON_H
 
 #include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/Analyses/ThreadSafetyTIL.h"
 #include "clang/Analysis/Analyses/ThreadSafetyTraverse.h"
@@ -272,27 +273,33 @@ class CFGWalker {
 class CapabilityExpr {
 private:
   static constexpr unsigned FlagNegative = 1u << 0;
+  static constexpr unsigned FlagReentrant = 1u << 1;
 
   /// The capability expression and flags.
-  llvm::PointerIntPair<const til::SExpr *, 1, unsigned> CapExpr;
+  llvm::PointerIntPair<const til::SExpr *, 2, unsigned> CapExpr;
 
   /// The kind of capability as specified by @ref CapabilityAttr::getName.
   StringRef CapKind;
 
 public:
   CapabilityExpr() : CapExpr(nullptr, 0) {}
-  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
-      : CapExpr(E, Neg ? FlagNegative : 0), CapKind(Kind) {}
+  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg, bool Reentrant)
+      : CapExpr(E, (Neg ? FlagNegative : 0) | (Reentrant ? FlagReentrant : 0)),
+        CapKind(Kind) {}
+  CapabilityExpr(const til::SExpr *E, QualType QT, bool Neg);
 
   // Don't allow implicitly-constructed StringRefs since we'll capture them.
-  template <typename T> CapabilityExpr(const til::SExpr *, T, bool) = delete;
+  template <typename T>
+  CapabilityExpr(const til::SExpr *, T, bool, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
   StringRef getKind() const { return CapKind; }
   bool negative() const { return CapExpr.getInt() & FlagNegative; }
+  bool reentrant() const { return CapExpr.getInt() & FlagReentrant; }
 
   CapabilityExpr operator!() const {
-    return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative());
+    return CapabilityExpr(CapExpr.getPointer(), CapKind, !negative(),
+                          reentrant());
   }
 
   bool equals(const CapabilityExpr &other) const {
@@ -389,10 +396,6 @@ class SExprBuilder {
   // Translate a variable reference.
   til::LiteralPtr *createVariable(const VarDecl *VD);
 
-  // Create placeholder for this: we don't know the VarDecl on construction yet.
-  std::pair<til::LiteralPtr *, StringRef>
-  createThisPlaceholder(const Expr *Exp);
-
   // Translate a clang statement or expression to a TIL expression.
   // Also performs substitution of variables; Ctx provides the context.
   // Dispatches on the type of S.

clang/include/clang/Basic/Attr.td

@@ -4011,6 +4011,13 @@ def LocksExcluded : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
+def ReentrantCapability : InheritableAttr {
+  let Spellings = [Clang<"reentrant_capability">];
+  let Subjects = SubjectList<[Record, TypedefName]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;
+}
+
 // C/C++ consumed attributes.
 
 def Consumable : InheritableAttr {

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4048,6 +4048,9 @@ def warn_thread_attribute_not_on_scoped_lockable_param : Warning<
   "%0 attribute applies to function parameters only if their type is a "
   "reference to a 'scoped_lockable'-annotated type">,
   InGroup<ThreadSafetyAttributes>, DefaultIgnore;
+def warn_reentrant_capability_without_capability : Warning<
+  "ignoring %0 attribute on %1 without 'capability' attribute">,
+  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
 def err_attribute_argument_out_of_bounds_extra_info : Error<
   "%0 attribute parameter %1 is out of bounds: "
   "%plural{0:no parameters to index into|"
@@ -4076,6 +4079,9 @@ def warn_lock_some_predecessors : Warning<
 def warn_expecting_lock_held_on_loop : Warning<
   "expecting %0 '%1' to be held at start of each loop">,
   InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_lock_reentrancy_mismatch : Warning<
+  "expecting %0 '%1' to be held with matching reentrancy depth on every path through here">,
+  InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
 def note_locked_here : Note<"%0 acquired here">;
 def note_unlocked_here : Note<"%0 released here">;
 def warn_lock_exclusive_and_shared : Warning<