[clang] Thread Safety Analysis: Warn when using negative reentrant capability (PR #141599)

[Aaron Puchert via cfe-commits]

================
@@ -286,51 +291,76 @@ static bool checkRecordTypeForScopedCapability(Sema &S, QualType Ty) {
   return checkRecordDeclForAttr<ScopedLockableAttr>(RT->getDecl());
 }
 
-static bool checkTypedefTypeForCapability(QualType Ty) {
+static std::optional<TypeDecl *> checkTypedefTypeForCapability(QualType Ty) {
   const auto *TD = Ty->getAs<TypedefType>();
   if (!TD)
-    return false;
+    return std::nullopt;
 
   TypedefNameDecl *TN = TD->getDecl();
   if (!TN)
-    return false;
+    return std::nullopt;
 
-  return TN->hasAttr<CapabilityAttr>();
-}
-
-static bool typeHasCapability(Sema &S, QualType Ty) {
-  if (checkTypedefTypeForCapability(Ty))
-    return true;
+  if (TN->hasAttr<CapabilityAttr>())
+    return {TN};
 
-  if (checkRecordTypeForCapability(S, Ty))
-    return true;
+  return std::nullopt;
+}
 
-  return false;
+/// Returns capability TypeDecl if defined, nullptr if not yet defined (maybe
+/// capability), and nullopt if it definitely is not a capability.
+static std::optional<TypeDecl *> checkTypeForCapability(Sema &S, QualType Ty) {
+  if (auto TD = checkTypedefTypeForCapability(Ty))
+    return TD;
+  if (auto TD = checkRecordTypeForCapability(S, Ty))
+    return TD;
+  return std::nullopt;
 }
 
-static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
+static bool validateCapabilityExpr(Sema &S, const ParsedAttr &AL,
+                                   const Expr *Ex, bool Neg = false) {
   // Capability expressions are simple expressions involving the boolean logic
   // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once
   // a DeclRefExpr is found, its type should be checked to determine whether it
   // is a capability or not.
 
   if (const auto *E = dyn_cast<CastExpr>(Ex))
-    return isCapabilityExpr(S, E->getSubExpr());
+    return validateCapabilityExpr(S, AL, E->getSubExpr(), Neg);
   else if (const auto *E = dyn_cast<ParenExpr>(Ex))
-    return isCapabilityExpr(S, E->getSubExpr());
+    return validateCapabilityExpr(S, AL, E->getSubExpr(), Neg);
   else if (const auto *E = dyn_cast<UnaryOperator>(Ex)) {
-    if (E->getOpcode() == UO_LNot || E->getOpcode() == UO_AddrOf ||
-        E->getOpcode() == UO_Deref)
-      return isCapabilityExpr(S, E->getSubExpr());
-    return false;
+    switch (E->getOpcode()) {
+    case UO_LNot:
+      Neg = !Neg;
+      [[fallthrough]];
+    case UO_AddrOf:
+    case UO_Deref:
+      return validateCapabilityExpr(S, AL, E->getSubExpr(), Neg);
+    default:
+      return false;
+    }
----------------
aaronpuchert wrote:

Ok, I'll take a look myself. My intuition says that it should be easy in `ThreadSafety.cpp`, but I might be overlooking something.

> This is declaration-time validation, not flow analysis

That's correct, but in Sema we mainly want to check the expression from a C++ point of view, which means we want to check types. And the types are fine in this case. There is another layer in `ThreadSafety.cpp`, which for example emits "cannot resolve lock expression".

https://github.com/llvm/llvm-project/pull/141599