Thread safety analysis: Store capability kind in CapabilityExpr

[aaronpuchert]

Summary

This should make us print the right capability kind in many more cases,
especially when attributes name multiple capabilities of different kinds.

Previously we were trying to deduce the capability kind from the
original attribute, but most attributes can name multiple capabilities,
which could be of different kinds. So instead we derive the kind when
translating the attribute expression, and then store it in the returned
CapabilityExpr. Then we can extract the corresponding capability name
when we need it, which saves us lots of plumbing and almost guarantees
that the name is right.

I didn't bother adding any tests for this because it's just a usability
improvement and it's pretty much evident from the code that we don't
fall back to "mutex" anymore (save for a few cases that I'll address in
a separate change).

[aaron.ballman]

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

277

Hrmmmm, I think this may actually be safe, but it does give me pause to store a StringRef as anything other than a local (or param/return type): https://llvm.org/docs/ProgrammersManual.html#the-stringref-class

Can you double-check my thinking? This is fine because it's pulling the StringRef from the CapabilityAttr, and that stores its name internally and isn't released until the AST is destroyed.

[aaronpuchert]

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

277

Exactly, aside from two places where I'm storing a string literal ("mutex" and "wildcard"), the common case is taking the StringRef (as returned by CapabilityAttr::getName) from the attribute. So that would be lifetime-coupled to the AST and should be safe for our analysis.

Of course StringRef is implicitly constructible from other types, and we wouldn't want that. Especially std::string would be an issue. Perhaps we should prevent implicit conversions, and thus force the caller to pass a StringRef glvalue, by overloading with a deleted template?

template<typename T>
CapabilityExpr(const til::SExpr *, T, bool) = delete;

Or maybe you've got a better idea.

As for a justification: we're building lots of CapabilityExprs, essentially every time we see a capability expression in the code (in attributes or as capability type method call arguments). Given that the kind is only used for actual warnings I wouldn't want us to spend of lot of time or memory on storing it.

We could also store the original Expr* and extract on demand, but that seemed to me antithetical to translating into the TIL. Of course it would be slightly faster, but the current code (before this change) also extracts capability names eagerly without waiting for the need to arise.

[aaron.ballman]

LGTM with the extra safety measure added. I'm happy to review again if you'd like, but don't require it.

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

277

Of course StringRef is implicitly constructible from other types, and we wouldn't want that. Especially std::string would be an issue. Perhaps we should prevent implicit conversions, and thus force the caller to pass a StringRef glvalue, by overloading with a deleted template?

I think that's likely a good safety measure, good suggestion!

As for a justification:

Oh, I already thought it was well-motivated, I just wanted to make sure I wasn't wrong about the lack of lifetime issues. :-)

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

 // TODO: move this back into ThreadSafety.cpp
 // This is specific to thread safety.  It is here because
 // translateAttrExpr needs it, but that should be moved too.
 class CapabilityExpr {
 private:
   /// The capability expression and whether it's negated.
   llvm::PointerIntPair<const til::SExpr *, 1, bool> CapExpr;
 
+  /// The kind of capability as specified by @ref CapabilityAttr::getName.
+  StringRef CapKind;
+
 public:
-  CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E, Neg) {}
+  CapabilityExpr() : CapExpr(nullptr, false) {}
+  CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
+      : CapExpr(E, Neg), CapKind(Kind) {}
+
+  // Don't allow implicitly-constructed StringRefs since we'll capture them.
+  template <typename T> CapabilityExpr(const til::SExpr *, T, bool) = delete;
 
   const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
+  StringRef getKind() const { return CapKind; }
   bool negative() const { return CapExpr.getInt(); }
 
   CapabilityExpr operator!() const {
-    return CapabilityExpr(CapExpr.getPointer(), !CapExpr.getInt());
+    return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
   }
 
   bool equals(const CapabilityExpr &other) const {
     return (negative() == other.negative()) &&
            sx::equals(sexpr(), other.sexpr());
   }
 
   bool matches(const CapabilityExpr &other) const {

clang/lib/Analysis/ThreadSafety.cpp

public:
   bool declared() const { return Source == Declared; }
   bool managed() const { return Source == Managed; }
 
   virtual void
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
                                 SourceLocation JoinLoc, LockErrorKind LEK,
                                 ThreadSafetyHandler &Handler) const = 0;
   virtual void handleLock(FactSet &FSet, FactManager &FactMan,
-                          const FactEntry &entry, ThreadSafetyHandler &Handler,
-                          StringRef DiagKind) const = 0;
+                          const FactEntry &entry,
+                          ThreadSafetyHandler &Handler) const = 0;
   virtual void handleUnlock(FactSet &FSet, FactManager &FactMan,
                             const CapabilityExpr &Cp, SourceLocation UnlockLoc,
-                            bool FullyRemove, ThreadSafetyHandler &Handler,
-                            StringRef DiagKind) const = 0;
+                            bool FullyRemove,
+                            ThreadSafetyHandler &Handler) const = 0;
 
   // Return true if LKind >= LK, where exclusive > shared
   bool isAtLeast(LockKind LK) const {
     return  (LKind == LK_Exclusive) || (LK == LK_Shared);
   }
 };
 
 using FactID = unsigned short;
LockableFactEntry(const CapabilityExpr &CE, LockKind LK, SourceLocation Loc,
                     SourceKind Src = Acquired)
       : FactEntry(CE, LK, Loc, Src) {}
 
   void
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
                                 SourceLocation JoinLoc, LockErrorKind LEK,
                                 ThreadSafetyHandler &Handler) const override {
     if (!asserted() && !negative() && !isUniversal()) {
-      Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
+      Handler.handleMutexHeldEndOfScope(getKind(), toString(), loc(), JoinLoc,
                                         LEK);
     }
   }
 
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
-                  ThreadSafetyHandler &Handler,
-                  StringRef DiagKind) const override {
-    Handler.handleDoubleLock(DiagKind, entry.toString(), loc(), entry.loc());
+                  ThreadSafetyHandler &Handler) const override {
+    Handler.handleDoubleLock(entry.getKind(), entry.toString(), loc(),
+                             entry.loc());
   }
 
   void handleUnlock(FactSet &FSet, FactManager &FactMan,
                     const CapabilityExpr &Cp, SourceLocation UnlockLoc,
-                    bool FullyRemove, ThreadSafetyHandler &Handler,
-                    StringRef DiagKind) const override {
+                    bool FullyRemove,
+                    ThreadSafetyHandler &Handler) const override {
     FSet.removeLock(FactMan, Cp);
     if (!Cp.negative()) {
       FSet.addLock(FactMan, std::make_unique<LockableFactEntry>(
                                 !Cp, LK_Exclusive, UnlockLoc));
     }
   }
 };
 
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
                                 SourceLocation JoinLoc, LockErrorKind LEK,
                                 ThreadSafetyHandler &Handler) const override {
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
       const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
       if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
           (UnderlyingMutex.Kind != UCK_Acquired && !Entry)) {
         // If this scoped lock manages another mutex, and if the underlying
         // mutex is still/not held, then warn about the underlying mutex.
-        Handler.handleMutexHeldEndOfScope(
-            "mutex", UnderlyingMutex.Cap.toString(), loc(), JoinLoc, LEK);
+        Handler.handleMutexHeldEndOfScope(UnderlyingMutex.Cap.getKind(),
+                                          UnderlyingMutex.Cap.toString(), loc(),
+                                          JoinLoc, LEK);
       }
     }
   }
 
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
-                  ThreadSafetyHandler &Handler,
-                  StringRef DiagKind) const override {
+                  ThreadSafetyHandler &Handler) const override {
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
       if (UnderlyingMutex.Kind == UCK_Acquired)
         lock(FSet, FactMan, UnderlyingMutex.Cap, entry.kind(), entry.loc(),
-             &Handler, DiagKind);
+             &Handler);
       else
-        unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), &Handler,
-               DiagKind);
+        unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), &Handler);
     }
   }
 
   void handleUnlock(FactSet &FSet, FactManager &FactMan,
                     const CapabilityExpr &Cp, SourceLocation UnlockLoc,
-                    bool FullyRemove, ThreadSafetyHandler &Handler,
-                    StringRef DiagKind) const override {
+                    bool FullyRemove,
+                    ThreadSafetyHandler &Handler) const override {
     assert(!Cp.negative() && "Managing object cannot be negative.");
     for (const auto &UnderlyingMutex : UnderlyingMutexes) {
       // Remove/lock the underlying mutex if it exists/is still unlocked; warn
       // on double unlocking/locking if we're not destroying the scoped object.
       ThreadSafetyHandler *TSHandler = FullyRemove ? nullptr : &Handler;
       if (UnderlyingMutex.Kind == UCK_Acquired) {
-        unlock(FSet, FactMan, UnderlyingMutex.Cap, UnlockLoc, TSHandler,
-               DiagKind);
+        unlock(FSet, FactMan, UnderlyingMutex.Cap, UnlockLoc, TSHandler);
       } else {
         LockKind kind = UnderlyingMutex.Kind == UCK_ReleasedShared
                             ? LK_Shared
                             : LK_Exclusive;
-        lock(FSet, FactMan, UnderlyingMutex.Cap, kind, UnlockLoc, TSHandler,
-             DiagKind);
+        lock(FSet, FactMan, UnderlyingMutex.Cap, kind, UnlockLoc, TSHandler);
       }
     }
     if (FullyRemove)
       FSet.removeLock(FactMan, Cp);
   }
 
 private:
   void lock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp,
-            LockKind kind, SourceLocation loc, ThreadSafetyHandler *Handler,
-            StringRef DiagKind) const {
+            LockKind kind, SourceLocation loc,
+            ThreadSafetyHandler *Handler) const {
     if (const FactEntry *Fact = FSet.findLock(FactMan, Cp)) {
       if (Handler)
-        Handler->handleDoubleLock(DiagKind, Cp.toString(), Fact->loc(), loc);
+        Handler->handleDoubleLock(Cp.getKind(), Cp.toString(), Fact->loc(),
+                                  loc);
     } else {
       FSet.removeLock(FactMan, !Cp);
       FSet.addLock(FactMan,
                    std::make_unique<LockableFactEntry>(Cp, kind, loc, Managed));
     }
   }
 
   void unlock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp,
-              SourceLocation loc, ThreadSafetyHandler *Handler,
-              StringRef DiagKind) const {
+              SourceLocation loc, ThreadSafetyHandler *Handler) const {
     if (FSet.findLock(FactMan, Cp)) {
       FSet.removeLock(FactMan, Cp);
       FSet.addLock(FactMan, std::make_unique<LockableFactEntry>(
                                 !Cp, LK_Exclusive, loc));
     } else if (Handler) {
       SourceLocation PrevLoc;
       if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
         PrevLoc = Neg->loc();
-      Handler->handleUnmatchedUnlock(DiagKind, Cp.toString(), loc, PrevLoc);
+      Handler->handleUnmatchedUnlock(Cp.getKind(), Cp.toString(), loc, PrevLoc);
     }
   }
 };
 
 /// Class which implements the core thread safety analysis routines.
 class ThreadSafetyAnalyzer {
   friend class BuildLockset;
   friend class threadSafety::BeforeSet;

 
 public:
   ThreadSafetyAnalyzer(ThreadSafetyHandler &H, BeforeSet* Bset)
       : Arena(&Bpa), SxBuilder(Arena), Handler(H), GlobalBeforeSet(Bset) {}
 
   bool inCurrentScope(const CapabilityExpr &CapE);
 
   void addLock(FactSet &FSet, std::unique_ptr<FactEntry> Entry,
-               StringRef DiagKind, bool ReqAttr = false);
+               bool ReqAttr = false);
   void removeLock(FactSet &FSet, const CapabilityExpr &CapE,
-                  SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind,
-                  StringRef DiagKind);
+                  SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind);
 
   template <typename AttrType>
   void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
                    const NamedDecl *D, VarDecl *SelfDecl = nullptr);
 
   template <class AttrType>
   void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
                    const NamedDecl *D,
class has_arg_iterator_range {
   static no& test(...);
 
 public:
   static const bool value = sizeof(test<Ty>(nullptr)) == sizeof(yes);
 };
 
 } // namespace
 
-static StringRef ClassifyDiagnostic(const CapabilityAttr *A) {
-  return A->getName();
-}
-
-static StringRef ClassifyDiagnostic(QualType VDT) {
-  // We need to look at the declaration of the type of the value to determine
-  // which it is. The type should either be a record or a typedef, or a pointer
-  // or reference thereof.
-  if (const auto *RT = VDT->getAs<RecordType>()) {
-    if (const auto *RD = RT->getDecl())
-      if (const auto *CA = RD->getAttr<CapabilityAttr>())
-        return ClassifyDiagnostic(CA);
-  } else if (const auto *TT = VDT->getAs<TypedefType>()) {
-    if (const auto *TD = TT->getDecl())
-      if (const auto *CA = TD->getAttr<CapabilityAttr>())
-        return ClassifyDiagnostic(CA);
-  } else if (VDT->isPointerType() || VDT->isReferenceType())
-    return ClassifyDiagnostic(VDT->getPointeeType());
-
-  return "mutex";
-}
-
-static StringRef ClassifyDiagnostic(const ValueDecl *VD) {
-  assert(VD && "No ValueDecl passed");
-
-  // The ValueDecl is the declaration of a mutex or role (hopefully).
-  return ClassifyDiagnostic(VD->getType());
-}
-
-template <typename AttrTy>
-static std::enable_if_t<!has_arg_iterator_range<AttrTy>::value, StringRef>
-ClassifyDiagnostic(const AttrTy *A) {
-  if (const ValueDecl *VD = getValueDecl(A->getArg()))
-    return ClassifyDiagnostic(VD);
-  return "mutex";
-}
-
-template <typename AttrTy>
-static std::enable_if_t<has_arg_iterator_range<AttrTy>::value, StringRef>
-ClassifyDiagnostic(const AttrTy *A) {
-  for (const auto *Arg : A->args()) {
-    if (const ValueDecl *VD = getValueDecl(Arg))
-      return ClassifyDiagnostic(VD);
-  }
-  return "mutex";
-}
-
 bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
   const threadSafety::til::SExpr *SExp = CapE.sexpr();
   assert(SExp && "Null expressions should be ignored");
 
   if (const auto *LP = dyn_cast<til::LiteralPtr>(SExp)) {
     const ValueDecl *VD = LP->clangDecl();
     // Variables defined in a function are always inaccessible.
     if (!VD->isDefinedOutsideFunctionOrMethod())
bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
 
   return false;
 }
 
 /// Add a new lock to the lockset, warning if the lock is already there.
 /// \param ReqAttr -- true if this is part of an initial Requires attribute.
 void ThreadSafetyAnalyzer::addLock(FactSet &FSet,
                                    std::unique_ptr<FactEntry> Entry,
-                                   StringRef DiagKind, bool ReqAttr) {
+                                   bool ReqAttr) {
   if (Entry->shouldIgnore())
     return;
 
   if (!ReqAttr && !Entry->negative()) {
     // look for the negative capability, and remove it from the fact set.
     CapabilityExpr NegC = !*Entry;
     const FactEntry *Nen = FSet.findLock(FactMan, NegC);
     if (Nen) {
       FSet.removeLock(FactMan, NegC);
     }
     else {
       if (inCurrentScope(*Entry) && !Entry->asserted())
-        Handler.handleNegativeNotHeld(DiagKind, Entry->toString(),
+        Handler.handleNegativeNotHeld(Entry->getKind(), Entry->toString(),
                                       NegC.toString(), Entry->loc());
     }
   }
 
   // Check before/after constraints
   if (Handler.issueBetaWarnings() &&
       !Entry->asserted() && !Entry->declared()) {
     GlobalBeforeSet->checkBeforeAfter(Entry->valueDecl(), FSet, *this,
-                                      Entry->loc(), DiagKind);
+                                      Entry->loc(), Entry->getKind());
   }
 
   // FIXME: Don't always warn when we have support for reentrant locks.
   if (const FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
     if (!Entry->asserted())
-      Cp->handleLock(FSet, FactMan, *Entry, Handler, DiagKind);
+      Cp->handleLock(FSet, FactMan, *Entry, Handler);
   } else {
     FSet.addLock(FactMan, std::move(Entry));
   }
 }
 
 /// Remove a lock from the lockset, warning if the lock is not there.
 /// \param UnlockLoc The source location of the unlock (only used in error msg)
 void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const CapabilityExpr &Cp,
                                       SourceLocation UnlockLoc,
-                                      bool FullyRemove, LockKind ReceivedKind,
-                                      StringRef DiagKind) {
+                                      bool FullyRemove, LockKind ReceivedKind) {
   if (Cp.shouldIgnore())
     return;
 
   const FactEntry *LDat = FSet.findLock(FactMan, Cp);
   if (!LDat) {
     SourceLocation PrevLoc;
     if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
       PrevLoc = Neg->loc();
-    Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc, PrevLoc);
+    Handler.handleUnmatchedUnlock(Cp.getKind(), Cp.toString(), UnlockLoc,
+                                  PrevLoc);
     return;
   }
 
   // Generic lock removal doesn't care about lock kind mismatches, but
   // otherwise diagnose when the lock kinds are mismatched.
   if (ReceivedKind != LK_Generic && LDat->kind() != ReceivedKind) {
-    Handler.handleIncorrectUnlockKind(DiagKind, Cp.toString(), LDat->kind(),
+    Handler.handleIncorrectUnlockKind(Cp.getKind(), Cp.toString(), LDat->kind(),
                                       ReceivedKind, LDat->loc(), UnlockLoc);
   }
 
-  LDat->handleUnlock(FSet, FactMan, Cp, UnlockLoc, FullyRemove, Handler,
-                     DiagKind);
+  LDat->handleUnlock(FSet, FactMan, Cp, UnlockLoc, FullyRemove, Handler);
 }
 
 /// Extract the list of mutexIDs from the attribute on an expression,
 /// and push them onto Mtxs, discarding any duplicates.
 template <typename AttrType>
 void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
                                        const Expr *Exp, const NamedDecl *D,
                                        VarDecl *SelfDecl) {
   if (Attr->args_size() == 0) {
     // The mutex held is the "this" object.
     CapabilityExpr Cp = SxBuilder.translateAttrExpr(nullptr, D, Exp, SelfDecl);
     if (Cp.isInvalid()) {
-       warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr));
-       return;
+      warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind());
     }
     //else
     if (!Cp.shouldIgnore())
       Mtxs.push_back_nodup(Cp);
     return;
   }
 
   for (const auto *Arg : Attr->args()) {
     CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl);
     if (Cp.isInvalid()) {
-       warnInvalidLock(Handler, nullptr, D, Exp, ClassifyDiagnostic(Attr));
-       continue;
+      warnInvalidLock(Handler, nullptr, D, Exp, Cp.getKind());
     }
     //else
     if (!Cp.shouldIgnore())
       Mtxs.push_back_nodup(Cp);
   }
 }
 
 /// Extract the list of mutexIDs from a trylock attribute.  If the
void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
   const Stmt *Cond = PredBlock->getTerminatorCondition();
   // We don't acquire try-locks on ?: branches, only when its result is used.
   if (!Cond || isa<ConditionalOperator>(PredBlock->getTerminatorStmt()))
     return;
 
   bool Negate = false;
   const CFGBlockInfo *PredBlockInfo = &BlockInfo[PredBlock->getBlockID()];
   const LocalVarContext &LVarCtx = PredBlockInfo->ExitContext;
-  StringRef CapDiagKind = "mutex";
 
   const auto *Exp = getTrylockCallExpr(Cond, LVarCtx, Negate);
   if (!Exp)
     return;
 
   auto *FunDecl = dyn_cast_or_null<NamedDecl>(Exp->getCalleeDecl());
   if(!FunDecl || !FunDecl->hasAttrs())
     return;
 
   CapExprSet ExclusiveLocksToAdd;
   CapExprSet SharedLocksToAdd;
 
   // If the condition is a call to a Trylock function, then grab the attributes
   for (const auto *Attr : FunDecl->attrs()) {
     switch (Attr->getKind()) {
       case attr::TryAcquireCapability: {
         auto *A = cast<TryAcquireCapabilityAttr>(Attr);
         getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
                     Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(),
                     Negate);
-        CapDiagKind = ClassifyDiagnostic(A);
         break;
       };
       case attr::ExclusiveTrylockFunction: {
         const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
-        getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl,
-                    PredBlock, CurrBlock, A->getSuccessValue(), Negate);
-        CapDiagKind = ClassifyDiagnostic(A);
+        getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+                    A->getSuccessValue(), Negate);
         break;
       }
       case attr::SharedTrylockFunction: {
         const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
-        getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl,
-                    PredBlock, CurrBlock, A->getSuccessValue(), Negate);
-        CapDiagKind = ClassifyDiagnostic(A);
+        getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+                    A->getSuccessValue(), Negate);
         break;
       }
       default:
         break;
     }
   }
 
   // Add and remove locks.
   SourceLocation Loc = Exp->getExprLoc();
   for (const auto &ExclusiveLockToAdd : ExclusiveLocksToAdd)
     addLock(Result, std::make_unique<LockableFactEntry>(ExclusiveLockToAdd,
-                                                         LK_Exclusive, Loc),
-            CapDiagKind);
+                                                        LK_Exclusive, Loc));
   for (const auto &SharedLockToAdd : SharedLocksToAdd)
     addLock(Result, std::make_unique<LockableFactEntry>(SharedLockToAdd,
-                                                         LK_Shared, Loc),
-            CapDiagKind);
+                                                        LK_Shared, Loc));
 }
 
 namespace {
 
 /// We use this class to visit different types of expressions in
 /// CFGBlocks, and build up the lockset.
 /// An expression may cause us to add or remove locks from the lockset, or else
 /// output error messages related to missing locks.
 /// FIXME: In future, we may be able to not inherit from a visitor.
 class BuildLockset : public ConstStmtVisitor<BuildLockset> {
   friend class ThreadSafetyAnalyzer;
 
   ThreadSafetyAnalyzer *Analyzer;
   FactSet FSet;
   LocalVariableMap::Context LVarCtx;
   unsigned CtxIndex;
 
   // helper functions
   void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK,
                           Expr *MutexExp, ProtectedOperationKind POK,
-                          StringRef DiagKind, SourceLocation Loc);
-  void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp,
-                       StringRef DiagKind);
+                          SourceLocation Loc);
+  void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp);
 
   void checkAccess(const Expr *Exp, AccessKind AK,
                    ProtectedOperationKind POK = POK_VarAccess);
   void checkPtAccess(const Expr *Exp, AccessKind AK,
                      ProtectedOperationKind POK = POK_VarAccess);
 
   void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
   void examineArguments(const FunctionDecl *FD,

 
 } // namespace
 
 /// Warn if the LSet does not contain a lock sufficient to protect access
 /// of at least the passed in AccessKind.
 void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
                                       AccessKind AK, Expr *MutexExp,
                                       ProtectedOperationKind POK,
-                                      StringRef DiagKind, SourceLocation Loc) {
+                                      SourceLocation Loc) {
   LockKind LK = getLockKindFromAccessKind(AK);
 
   CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
   if (Cp.isInvalid()) {
-    warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind);
+    warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
     return;
   } else if (Cp.shouldIgnore()) {
     return;
   }
 
   if (Cp.negative()) {
     // Negative capabilities act like locks excluded
     const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
     if (LDat) {
       Analyzer->Handler.handleFunExcludesLock(
-          DiagKind, D->getNameAsString(), (!Cp).toString(), Loc);
+          Cp.getKind(), D->getNameAsString(), (!Cp).toString(), Loc);
       return;
     }
 
     // If this does not refer to a negative capability in the same class,
     // then stop here.
     if (!Analyzer->inCurrentScope(Cp))
       return;
 
void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
   bool NoError = true;
   if (!LDat) {
     // No exact match found.  Look for a partial match.
     LDat = FSet.findPartialMatch(Analyzer->FactMan, Cp);
     if (LDat) {
       // Warn that there's no precise match.
       std::string PartMatchStr = LDat->toString();
       StringRef   PartMatchName(PartMatchStr);
-      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+      Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
                                            LK, Loc, &PartMatchName);
     } else {
       // Warn that there's no match at all.
-      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+      Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
                                            LK, Loc);
     }
     NoError = false;
   }
   // Make sure the mutex we found is the right kind.
   if (NoError && LDat && !LDat->isAtLeast(LK)) {
-    Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+    Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
                                          LK, Loc);
   }
 }
 
 /// Warn if the LSet contains the given lock.
 void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
-                                   Expr *MutexExp, StringRef DiagKind) {
+                                   Expr *MutexExp) {
   CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
   if (Cp.isInvalid()) {
-    warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind);
+    warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
     return;
   } else if (Cp.shouldIgnore()) {
     return;
   }
 
   const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
   if (LDat) {
-    Analyzer->Handler.handleFunExcludesLock(
-        DiagKind, D->getNameAsString(), Cp.toString(), Exp->getExprLoc());
+    Analyzer->Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
+                                            Cp.toString(), Exp->getExprLoc());
   }
 }
 
 /// Checks guarded_by and pt_guarded_by attributes.
 /// Whenever we identify an access (read or write) to a DeclRefExpr that is
 /// marked with guarded_by, we must ensure the appropriate mutexes are held.
 /// Similarly, we check if the access is to an expression that dereferences
 /// a pointer marked with pt_guarded_by.
void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK,
   if (!D || !D->hasAttrs())
     return;
 
   if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan)) {
     Analyzer->Handler.handleNoMutexHeld("mutex", D, POK, AK, Loc);
   }
 
   for (const auto *I : D->specific_attrs<GuardedByAttr>())
-    warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK,
-                       ClassifyDiagnostic(I), Loc);
+    warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, Loc);
 }
 
 /// Checks pt_guarded_by and pt_guarded_var attributes.
 /// POK is the same  operationKind that was passed to checkAccess.
 void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
                                  ProtectedOperationKind POK) {
   while (true) {
     if (const auto *PE = dyn_cast<ParenExpr>(Exp)) {
void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
   if (!D || !D->hasAttrs())
     return;
 
   if (D->hasAttr<PtGuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan))
     Analyzer->Handler.handleNoMutexHeld("mutex", D, PtPOK, AK,
                                         Exp->getExprLoc());
 
   for (auto const *I : D->specific_attrs<PtGuardedByAttr>())
-    warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK,
-                       ClassifyDiagnostic(I), Exp->getExprLoc());
+    warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, Exp->getExprLoc());
 }
 
 /// Process a function call, method call, constructor call,
 /// or destructor call.  This involves looking at the attributes on the
 /// corresponding function/method/constructor/destructor, issuing warnings,
 /// and updating the locksets accordingly.
 ///
 /// FIXME: For classes annotated with one of the guarded annotations, we need
 /// to treat const method calls as reads and non-const method calls as writes,
 /// and check that the appropriate locks are held. Non-const method calls with
 /// the same signature as const method calls can be also treated as reads.
 ///
 void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
                               VarDecl *VD) {
   SourceLocation Loc = Exp->getExprLoc();
   CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd;
   CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
   CapExprSet ScopedReqsAndExcludes;
-  StringRef CapDiagKind = "mutex";
 
   // Figure out if we're constructing an object of scoped lockable class
   bool isScopedVar = false;
   if (VD) {
     if (const auto *CD = dyn_cast<const CXXConstructorDecl>(D)) {
       const CXXRecordDecl* PD = CD->getParent();
       if (PD && PD->hasAttr<ScopedLockableAttr>())
         isScopedVar = true;
     }
   }
 
   for(const Attr *At : D->attrs()) {
     switch (At->getKind()) {
       // When we encounter a lock function, we need to add the lock to our
       // lockset.
       case attr::AcquireCapability: {
         const auto *A = cast<AcquireCapabilityAttr>(At);
         Analyzer->getMutexIDs(A->isShared() ? SharedLocksToAdd
                                             : ExclusiveLocksToAdd,
                               A, Exp, D, VD);
-
-        CapDiagKind = ClassifyDiagnostic(A);
         break;
       }
 
       // An assert will add a lock to the lockset, but will not generate
       // a warning if it is already there, and will not generate a warning
       // if it is not removed.
       case attr::AssertExclusiveLock: {
         const auto *A = cast<AssertExclusiveLockAttr>(At);
 
         CapExprSet AssertLocks;
         Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
         for (const auto &AssertLock : AssertLocks)
           Analyzer->addLock(
-              FSet,
-              std::make_unique<LockableFactEntry>(AssertLock, LK_Exclusive, Loc,
-                                                  FactEntry::Asserted),
-              ClassifyDiagnostic(A));
+              FSet, std::make_unique<LockableFactEntry>(
+                        AssertLock, LK_Exclusive, Loc, FactEntry::Asserted));
         break;
       }
       case attr::AssertSharedLock: {
         const auto *A = cast<AssertSharedLockAttr>(At);
 
         CapExprSet AssertLocks;
         Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
         for (const auto &AssertLock : AssertLocks)
           Analyzer->addLock(
-              FSet,
-              std::make_unique<LockableFactEntry>(AssertLock, LK_Shared, Loc,
-                                                  FactEntry::Asserted),
-              ClassifyDiagnostic(A));
+              FSet, std::make_unique<LockableFactEntry>(
+                        AssertLock, LK_Shared, Loc, FactEntry::Asserted));
         break;
       }
 
       case attr::AssertCapability: {
         const auto *A = cast<AssertCapabilityAttr>(At);
         CapExprSet AssertLocks;
         Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
         for (const auto &AssertLock : AssertLocks)
-          Analyzer->addLock(FSet,
-                            std::make_unique<LockableFactEntry>(
+          Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(
                                       AssertLock,
-                                A->isShared() ? LK_Shared : LK_Exclusive, Loc,
-                                FactEntry::Asserted),
-                            ClassifyDiagnostic(A));
+                                      A->isShared() ? LK_Shared : LK_Exclusive,
+                                      Loc, FactEntry::Asserted));
         break;
       }
 
       // When we encounter an unlock function, we need to remove unlocked
       // mutexes from the lockset, and flag a warning if they are not there.
       case attr::ReleaseCapability: {
         const auto *A = cast<ReleaseCapabilityAttr>(At);
         if (A->isGeneric())
           Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD);
         else if (A->isShared())
           Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD);
         else
           Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, VD);
-
-        CapDiagKind = ClassifyDiagnostic(A);
         break;
       }
 
       case attr::RequiresCapability: {
         const auto *A = cast<RequiresCapabilityAttr>(At);
         for (auto *Arg : A->args()) {
           warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
-                             POK_FunctionCall, ClassifyDiagnostic(A),
-                             Exp->getExprLoc());
+                             POK_FunctionCall, Exp->getExprLoc());
           // use for adopting a lock
           if (isScopedVar)
             Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
         }
         break;
       }
 
       case attr::LocksExcluded: {
         const auto *A = cast<LocksExcludedAttr>(At);
         for (auto *Arg : A->args()) {
-          warnIfMutexHeld(D, Exp, Arg, ClassifyDiagnostic(A));
+          warnIfMutexHeld(D, Exp, Arg);
           // use for deferring a lock
           if (isScopedVar)
             Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
         }
         break;
       }
 
       // Ignore attributes unrelated to thread-safety
       default:
         break;
     }
   }
 
   // Remove locks first to allow lock upgrading/downgrading.
   // FIXME -- should only fully remove if the attribute refers to 'this'.
   bool Dtor = isa<CXXDestructorDecl>(D);
   for (const auto &M : ExclusiveLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive, CapDiagKind);
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive);
   for (const auto &M : SharedLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared, CapDiagKind);
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared);
   for (const auto &M : GenericLocksToRemove)
-    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
+    Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic);
 
   // Add locks.
   FactEntry::SourceKind Source =
       isScopedVar ? FactEntry::Managed : FactEntry::Acquired;
   for (const auto &M : ExclusiveLocksToAdd)
-    Analyzer->addLock(
-        FSet, std::make_unique<LockableFactEntry>(M, LK_Exclusive, Loc, Source),
-        CapDiagKind);
+    Analyzer->addLock(FSet, std::make_unique<LockableFactEntry>(M, LK_Exclusive,
+                                                                Loc, Source));
   for (const auto &M : SharedLocksToAdd)
     Analyzer->addLock(
-        FSet, std::make_unique<LockableFactEntry>(M, LK_Shared, Loc, Source),
-        CapDiagKind);
+        FSet, std::make_unique<LockableFactEntry>(M, LK_Shared, Loc, Source));
 
   if (isScopedVar) {
     // Add the managing object as a dummy mutex, mapped to the underlying mutex.
     SourceLocation MLoc = VD->getLocation();
     DeclRefExpr DRE(VD->getASTContext(), VD, false, VD->getType(), VK_LValue,
                     VD->getLocation());
     // FIXME: does this store a pointer to DRE?
     CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr);
 
     auto ScopedEntry = std::make_unique<ScopedLockableFactEntry>(Scp, MLoc);
     for (const auto &M : ExclusiveLocksToAdd)
       ScopedEntry->addLock(M);
     for (const auto &M : SharedLocksToAdd)
       ScopedEntry->addLock(M);
     for (const auto &M : ScopedReqsAndExcludes)
       ScopedEntry->addLock(M);
     for (const auto &M : ExclusiveLocksToRemove)
       ScopedEntry->addExclusiveUnlock(M);
     for (const auto &M : SharedLocksToRemove)
       ScopedEntry->addSharedUnlock(M);
-    Analyzer->addLock(FSet, std::move(ScopedEntry), CapDiagKind);
+    Analyzer->addLock(FSet, std::move(ScopedEntry));
   }
 }
 
 /// For unary operations which read and write a variable, we need to
 /// check whether we hold any required mutexes. Reads are checked in
 /// VisitCastExpr.
 void BuildLockset::VisitUnaryOperator(const UnaryOperator *UO) {
   switch (UO->getOpcode()) {
if (A.kind() != B.kind()) {
     // For managed capabilities, the destructor should unlock in the right mode
     // anyway. For asserted capabilities no unlocking is needed.
     if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) {
       // The shared capability subsumes the exclusive capability, if possible.
       bool ShouldTakeB = B.kind() == LK_Shared;
       if (CanModify || !ShouldTakeB)
         return ShouldTakeB;
     }
-    Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc());
+    Handler.handleExclusiveAndShared(B.getKind(), B.toString(), B.loc(),
+                                     A.loc());
     // Take the exclusive capability to reduce further warnings.
     return CanModify && B.kind() == LK_Exclusive;
   } else {
     // The non-asserted capability is the one we want to track.
     return CanModify && A.asserted() && !B.asserted();
   }
 }
 
void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   // to initial lockset. Also turn off checking for lock and unlock functions.
   // FIXME: is there a more intelligent way to check lock/unlock functions?
   if (!SortedGraph->empty() && D->hasAttrs()) {
     const CFGBlock *FirstBlock = *SortedGraph->begin();
     FactSet &InitialLockset = BlockInfo[FirstBlock->getBlockID()].EntrySet;
 
     CapExprSet ExclusiveLocksToAdd;
     CapExprSet SharedLocksToAdd;
-    StringRef CapDiagKind = "mutex";
 
     SourceLocation Loc = D->getLocation();
     for (const auto *Attr : D->attrs()) {
       Loc = Attr->getLocation();
       if (const auto *A = dyn_cast<RequiresCapabilityAttr>(Attr)) {
         getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
                     nullptr, D);
-        CapDiagKind = ClassifyDiagnostic(A);
       } else if (const auto *A = dyn_cast<ReleaseCapabilityAttr>(Attr)) {
         // UNLOCK_FUNCTION() is used to hide the underlying lock implementation.
         // We must ignore such methods.
         if (A->args_size() == 0)
           return;
         getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
                     nullptr, D);
         getMutexIDs(LocksReleased, A, nullptr, D);
-        CapDiagKind = ClassifyDiagnostic(A);
       } else if (const auto *A = dyn_cast<AcquireCapabilityAttr>(Attr)) {
         if (A->args_size() == 0)
           return;
         getMutexIDs(A->isShared() ? SharedLocksAcquired
                                   : ExclusiveLocksAcquired,
                     A, nullptr, D);
-        CapDiagKind = ClassifyDiagnostic(A);
       } else if (isa<ExclusiveTrylockFunctionAttr>(Attr)) {
         // Don't try to check trylock functions for now.
         return;
       } else if (isa<SharedTrylockFunctionAttr>(Attr)) {
         // Don't try to check trylock functions for now.
         return;
       } else if (isa<TryAcquireCapabilityAttr>(Attr)) {
         // Don't try to check trylock functions for now.
         return;
       }
     }
 
     // FIXME -- Loc can be wrong here.
     for (const auto &Mu : ExclusiveLocksToAdd) {
       auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Exclusive, Loc,
                                                        FactEntry::Declared);
-      addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
+      addLock(InitialLockset, std::move(Entry), true);
     }
     for (const auto &Mu : SharedLocksToAdd) {
       auto Entry = std::make_unique<LockableFactEntry>(Mu, LK_Shared, Loc,
                                                        FactEntry::Declared);
-      addLock(InitialLockset, std::move(Entry), CapDiagKind, true);
+      addLock(InitialLockset, std::move(Entry), true);
     }
   }
 
   for (const auto *CurrBlock : *SortedGraph) {
     unsigned CurrBlockID = CurrBlock->getBlockID();
     CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlockID];
 
     // Use the default initial lockset in case there are no predecessors.

clang/lib/Analysis/ThreadSafetyCommon.cpp

til::SCFG *SExprBuilder::buildCFG(CFGWalker &Walker) {
   return Scfg;
 }
 
 static bool isCalleeArrow(const Expr *E) {
   const auto *ME = dyn_cast<MemberExpr>(E->IgnoreParenCasts());
   return ME ? ME->isArrow() : false;
 }
 
+static StringRef ClassifyDiagnostic(const CapabilityAttr *A) {
+  return A->getName();
+}
+
+static StringRef ClassifyDiagnostic(QualType VDT) {
+  // We need to look at the declaration of the type of the value to determine
+  // which it is. The type should either be a record or a typedef, or a pointer
+  // or reference thereof.
+  if (const auto *RT = VDT->getAs<RecordType>()) {
+    if (const auto *RD = RT->getDecl())
+      if (const auto *CA = RD->getAttr<CapabilityAttr>())
+        return ClassifyDiagnostic(CA);
+  } else if (const auto *TT = VDT->getAs<TypedefType>()) {
+    if (const auto *TD = TT->getDecl())
+      if (const auto *CA = TD->getAttr<CapabilityAttr>())
+        return ClassifyDiagnostic(CA);
+  } else if (VDT->isPointerType() || VDT->isReferenceType())
+    return ClassifyDiagnostic(VDT->getPointeeType());
+
+  return "mutex";
+}
+
 /// Translate a clang expression in an attribute to a til::SExpr.
 /// Constructs the context from D, DeclExp, and SelfDecl.
 ///
 /// \param AttrExp The expression to translate.
 /// \param D       The declaration to which the attribute is attached.
 /// \param DeclExp An expression involving the Decl to which the attribute
 ///                is attached.  E.g. the call to a function.
 CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
else  // For most attributes.
     return translateAttrExpr(AttrExp, &Ctx);
 }
 
 /// Translate a clang expression in an attribute to a til::SExpr.
 // This assumes a CallingContext has already been created.
 CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp,
                                                CallingContext *Ctx) {
   if (!AttrExp)
-    return CapabilityExpr(nullptr, false);
+    return CapabilityExpr();
 
   if (const auto* SLit = dyn_cast<StringLiteral>(AttrExp)) {
     if (SLit->getString() == StringRef("*"))
       // The "*" expr is a universal lock, which essentially turns off
       // checks until it is removed from the lockset.
-      return CapabilityExpr(new (Arena) til::Wildcard(), false);
+      return CapabilityExpr(new (Arena) til::Wildcard(), StringRef("wildcard"),
+                            false);
     else
       // Ignore other string literals for now.
-      return CapabilityExpr(nullptr, false);
+      return CapabilityExpr();
   }
 
   bool Neg = false;
   if (const auto *OE = dyn_cast<CXXOperatorCallExpr>(AttrExp)) {
     if (OE->getOperator() == OO_Exclaim) {
       Neg = true;
       AttrExp = OE->getArg(0);
     }
   }
   else if (const auto *UO = dyn_cast<UnaryOperator>(AttrExp)) {
     if (UO->getOpcode() == UO_LNot) {
       Neg = true;
       AttrExp = UO->getSubExpr();
     }
   }
 
   til::SExpr *E = translate(AttrExp, Ctx);
 
   // Trap mutex expressions like nullptr, or 0.
   // Any literal value is nonsense.
   if (!E || isa<til::Literal>(E))
-    return CapabilityExpr(nullptr, false);
+    return CapabilityExpr();
+
+  StringRef Kind = ClassifyDiagnostic(AttrExp->getType());
 
   // Hack to deal with smart pointers -- strip off top-level pointer casts.
   if (const auto *CE = dyn_cast<til::Cast>(E)) {
     if (CE->castOpcode() == til::CAST_objToPtr)
-      return CapabilityExpr(CE->expr(), Neg);
+      return CapabilityExpr(CE->expr(), Kind, Neg);
   }
-  return CapabilityExpr(E, Neg);
+  return CapabilityExpr(E, Kind, Neg);
 }
 
 // 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.
 til::SExpr *SExprBuilder::translate(const Stmt *S, CallingContext *Ctx) {
   if (!S)
     return nullptr;

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

class Foo {
 
   // don't warn on joins with universal lock
   void test2() {
     if (c) {
       beginNoWarnOnWrites();
     }
     a = 0; // \
       // expected-warning {{writing variable 'a' requires holding mutex 'mu_' exclusively}}
-    endNoWarnOnWrites();  // \
-      // expected-warning {{releasing mutex '*' that was not held}}
+    endNoWarnOnWrites(); // \
+      // expected-warning {{releasing wildcard '*' that was not held}}
   }
 
 
   // make sure the universal lock joins properly
   void test3() {
     if (c) {
       mu_.Lock();
       beginNoWarnOnWrites();