[clang] [clang][ThreadSafety]: fix discrepancy between capability attributes (PR #139343)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Cory Fields (theuni)

<details>
<summary>Changes</summary>

Fix the case where `release_generic_capability` did not correctly release when 
used as a reverse capability as enabled by commit 6a68efc959.

I noticed this when trying to implement a reverse lock.

My my project still uses the old `UNLOCK_FUNCTION` macro which maps to 
`unlock_function`. With that attribute, the [MutexUnlock test-case seen 
here](https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/warn-thread-safety-analysis.cpp#L3126)
 does not work.

I'm not at all familiar with the clang code so I have no idea if this is the 
correct fix, but it fixes my problem. Hopefully it's helpful.

Here's a minimal reproducer:

```c++
class __attribute__((capability(""))) Mutex {
 public:
  const Mutex&amp; operator!() const { return *this; }
};

class  __attribute__((scoped_lockable)) MutexLock {
public:
  MutexLock(Mutex *mu) __attribute__((acquire_capability(mu))) {}
  ~MutexLock() __attribute__((release_capability())){}
};

  class  __attribute__((scoped_lockable)) MutexLockOld {
  public:
      MutexLockOld(Mutex *mu) __attribute__((exclusive_lock_function(mu))) {}
      ~MutexLockOld() __attribute__((unlock_function())){}
  };

class  __attribute__((scoped_lockable)) MutexLockGeneric {
public:
    MutexLockGeneric(Mutex *mu) __attribute__((acquire_capability(mu))) {}
    ~MutexLockGeneric() __attribute__((release_generic_capability())){}
};

class  __attribute__((scoped_lockable)) MutexUnlock {
public:
  MutexUnlock(Mutex *mu) __attribute__((release_capability(mu))){}
  ~MutexUnlock() __attribute__((release_capability())){}
};

class  __attribute__((scoped_lockable)) MutexUnlockOld {
public:
  MutexUnlockOld(Mutex *mu) __attribute__((unlock_function(mu))){}
  ~MutexUnlockOld() __attribute__((unlock_function())){}
};

class  __attribute__((scoped_lockable)) MutexUnlockGeneric {
public:
  MutexUnlockGeneric(Mutex *mu) 
__attribute__((release_generic_capability(mu))){}
  ~MutexUnlockGeneric() __attribute__((release_generic_capability())){}
};


Mutex mut;
void req() __attribute__((requires_capability(mut))){}
void req2() __attribute__((exclusive_locks_required(mut))){}
void not_req() __attribute__((requires_capability(!mut))){}

void good()
{
    MutexLock lock(&amp;mut);
    req();
    {
        MutexUnlock reverse_lock(&amp;mut);
        not_req();
    }
    req();
}

void bad()
{
    MutexLockGeneric lock(&amp;mut);
    req();
    {
        MutexUnlockGeneric reverse_lock(&amp;mut);
        not_req();
    }
    req();
    req2();
}

void bad2()
{
    MutexLockOld lock(&amp;mut);
    req();
    {
        MutexUnlockOld reverse_lock(&amp;mut);
        not_req();
    }
    req();
    req2();
}
```

Result:
```bash
clangtest.cpp:67:5: warning: calling function 'req' requires holding  'mut' 
exclusively [-Wthread-safety-analysis]
   67 |     req();
      |     ^
clangtest.cpp:68:5: warning: calling function 'req2' requires holding  'mut' 
exclusively [-Wthread-safety-analysis]
   68 |     req2();
      |     ^
clangtest.cpp:79:5: warning: calling function 'req' requires holding  'mut' 
exclusively [-Wthread-safety-analysis]
   79 |     req();
      |     ^
clangtest.cpp:80:5: warning: calling function 'req2' requires holding  'mut' 
exclusively [-Wthread-safety-analysis]
   80 |     req2();
```

---
Full diff: https://github.com/llvm/llvm-project/pull/139343.diff


1 Files Affected:

- (modified) clang/lib/Analysis/ThreadSafety.cpp (+2) 


``````````diff
diff --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 7e86af6b4a317..a963bcda0c2d0 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2026,6 +2026,8 @@ void BuildLockset::handleCall(const Expr *Exp, const 
NamedDecl *D,
       ScopedEntry->addExclusiveUnlock(M);
     for (const auto &M : SharedLocksToRemove)
       ScopedEntry->addSharedUnlock(M);
+    for (const auto &M : GenericLocksToRemove)
+      ScopedEntry->addExclusiveUnlock(M);
     Analyzer->addLock(FSet, std::move(ScopedEntry));
   }
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/139343
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits