使用 Visual Studio 锁定 SAL

Locking SAL with Visual Studio

我正在尝试将锁定 SAL 语句添加到 find/prevent 我的应用程序中的错误锁定。比如丢失的锁定调用和不匹配的锁定调用。我收到我不明白的警告。我已将它们作为注释放在下面的示例中。我打开代码分析并设置 Microsoft All Rules。我正在使用 Visual Studio 2019 16.11.9。我也试过 Visual Studio 2022 17.0.5 并得到相同的结果。

我试过用_Success_代替_When_。我试过 returning bool 而不是 BOOL 和 _Success_(return) vs _Success_(return != 0).

我读过的一些研究:

https://docs.microsoft.com/en-us/cpp/code-quality/c26135?view=msvc-170

_When_(return != 0, _Acquires_lock_(p->cs))
int TryEnter(DATA* p)
{
    if (p->state != 0)
    {
        EnterCriticalSection(&p->cs);
        return p->state;
    }
    return 0;
}

前一篇文章发表于 2021 年,因此可能比 Visual Studio 2015 年的后一篇文章更新。VS2015 示例无法按原样工作; _Success_ 注释似乎已更改,因为它不再需要两个参数。

https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2015/code-quality/best-practices-and-examples-sal?view=vs-2015

// Incorrect  
_Success_(return == TRUE, _Acquires_lock_(*lpCriticalSection))  
BOOL WINAPI TryEnterCriticalSection(  
  _Inout_ LPCRITICAL_SECTION lpCriticalSection  
);  
  
// Correct  
_Success_(return != 0, _Acquires_lock_(*lpCriticalSection))  
BOOL WINAPI TryEnterCriticalSection(  
  _Inout_ LPCRITICAL_SECTION lpCriticalSection  
);  

我的测试代码:

#define _WIN32_WINNT _WIN32_WINNT_WIN7

#include <windows.h>
#include <tchar.h>

// https://docs.microsoft.com/en-us/cpp/code-quality/c26135?view=msvc-170

class CLock {
public:

    CLock() noexcept : m_hMutex(CreateMutex(nullptr,FALSE,nullptr)){
    }

    ~CLock(){

        CloseHandle(m_hMutex);

        m_hMutex = nullptr;

    }

    _When_(return != 0, _Acquires_lock_(this->m_hMutex)) BOOL Lock() noexcept {

        // warning C26135: Missing annotation _Acquires_lock_(this->m_hMutex) at function 'CLock::Lock'.
        // I don't understand this warning, I have the annotation?
        return (WaitForSingleObject(m_hMutex,INFINITE) == WAIT_OBJECT_0);

    }

    _When_(return != 0, _Releases_lock_(this->m_hMutex)) BOOL Unlock() noexcept {

        // warning C26135 : Missing annotation _Releases_lock_(this->m_hMutex) at function 'CLock::Unlock'.
        // I don't understand this warning, I have the annotation?
        return ReleaseMutex(m_hMutex);

    }

private:

    // Ensure no copies can be made
    CLock(const CLock &lCopy) = delete;
    CLock(const CLock &&lMove) = delete;
    const CLock &operator=(const CLock &lCopy) = delete;
    const CLock &&operator=(const CLock &&lMove) = delete;

    _Has_lock_kind_(_Lock_kind_mutex_) HANDLE m_hMutex;

};

class ThreadedThingy {
public:

    ThreadedThingy() noexcept : m_iThingy(0){
    }

    VOID TestBadNotLocked() noexcept {

        // warning C26130: Missing annotation _Requires_lock_held_(this->m_lLock) or _No_competing_thread_
        //                 at function 'ThreadedThingy::TestBadNotLocked'. Otherwise it could be a race
        //                 condition. Variable 'this->m_iThingy' should be protected by lock 'this->m_lLock'.
        // This should be a warning since the lock isn't held
        m_iThingy = 10;

    }

    VOID TestBadNotUnlocked() noexcept {

        if (m_lLock.Lock()){

            // warning C26165: Possibly failing to release lock '(&this->m_lLock)->m_hMutex' in function
            //                'ThreadedThingy::TestBadNotUnlocked'.
            // This should be a warning since lock isn't unlocked

            // warning C26130: Missing annotation _Requires_lock_held_(this->m_lLock) or _No_competing_thread_
            //                 at function 'ThreadedThingy::TestBadNotUnlocked'. Otherwise it could be a race
            //                 condition. Variable 'this->m_iThingy' should be protected by lock 'this->m_lLock'.
            // I don't understand this warning, the lock is held
            m_iThingy = 20;

        }

    }

    VOID TestGood() noexcept {

        if (m_lLock.Lock()){

            // warning C26130: Missing annotation _Requires_lock_held_(this->m_lLock) or _No_competing_thread_ at
            //                 function 'ThreadedThingy::TestGood'. Otherwise it could be a race condition.
            //                 Variable 'this->m_iThingy' should be protected by lock 'this->m_lLock'.
            // I don't understand this warning, the lock is held
            m_iThingy = 20;

            // warning C26165: Possibly failing to release lock '(&this->m_lLock)->m_hMutex' in function
            //                 'ThreadedThingy::TestGood'.
            // I kind of understand this warning because Unlock can't gaurentee the lock is released
            m_lLock.Unlock();

        }

    }

private:

    _Has_lock_kind_(_Lock_kind_mutex_) CLock m_lLock;

    _Guarded_by_(this->m_lLock) INT m_iThingy;

};

INT _tmain() noexcept {

    ThreadedThingy ttTest;

    ttTest.TestBadNotLocked();

    ttTest.TestBadNotUnlocked();

    ttTest.TestGood();

    return 0;

}

谢谢!

-丹尼尔

我向 Microsoft 开了一张工单以得到这个问题的答案。我学到了三件事。首先,当 WaitForSingleObject returns WAIT_ABANDONED 时,这意味着锁现在已被拥有,类似于当它 returns WAIT_OBJECT_0 但未发出信号时。所以,你必须检查两个 return 值,否则它会说你没有正确的注释。其次,静态代码分析工具假设 ReleaseMutex 总是释放锁。所以,我在 Unlock() 上的_When_ 注释是 unnecessary/wrong。最后,link CLock 对象的互斥锁需要另一个注解。这个注解是_Post_same_lock_。 Google 这个注解没有太多内容,只有头文件。工作代码:

#define _WIN32_WINNT _WIN32_WINNT_WIN7

#include <windows.h>
#include <tchar.h>

class CLock {
public:

    CLock() noexcept : m_hMutex(CreateMutex(nullptr,FALSE,nullptr)){
    }

    ~CLock(){

        CloseHandle(m_hMutex);

        m_hMutex = nullptr;

    }

    _When_(return != 0, _Acquires_lock_(this->m_hMutex) _Post_same_lock_(*this, this->m_hMutex)) BOOL Lock() noexcept {

        const DWORD dwWait = WaitForSingleObject(m_hMutex, INFINITE);

        // You have to do more than this to handle WAIT_ABANDONED_0
        // This example is only about SAL, not how to corretly handle WAIT_ABANDONED_0

        return ((dwWait == WAIT_OBJECT_0) || (dwWait == WAIT_ABANDONED_0));

    }

    _Releases_lock_(this->m_hMutex) VOID Unlock() noexcept {

        ReleaseMutex(m_hMutex);

    }

private:

    // Ensure no copies can be made
    CLock(const CLock &lCopy) = delete;
    CLock(const CLock &&lMove) = delete;
    const CLock &operator=(const CLock &lCopy) = delete;
    const CLock &&operator=(const CLock &&lMove) = delete;

    _Has_lock_kind_(_Lock_kind_mutex_) HANDLE m_hMutex;

};

class ThreadedThingy {
public:

    ThreadedThingy() noexcept : m_iThingy(0){
    }

    VOID TestBadNotLocked() noexcept {

        // warning C26130: Missing annotation _Requires_lock_held_(this->m_lLock) or _No_competing_thread_
        //                 at function 'ThreadedThingy::TestBadNotLocked'. Otherwise it could be a race
        //                 condition. Variable 'this->m_iThingy' should be protected by lock 'this->m_lLock'.
        // This should be a warning since the lock isn't held
        m_iThingy = 10;

    }

    VOID TestBadNotUnlocked() noexcept {

        if (m_lLock.Lock()){

            // warning C26165: Possibly failing to release lock '(&this->m_lLock)->m_hMutex' in function
            //                'ThreadedThingy::TestBadNotUnlocked'.
            // This should be a warning since lock isn't unlocked

            m_iThingy = 20;

        }

    }

    VOID TestGood() noexcept {

        if (m_lLock.Lock()){

            m_iThingy = 20;

            m_lLock.Unlock();

        }

    }

private:

    CLock m_lLock;

    _Guarded_by_(this->m_lLock) INT m_iThingy;

};

INT _tmain() noexcept {

    ThreadedThingy ttTest;

    ttTest.TestBadNotLocked();

    ttTest.TestBadNotUnlocked();

    ttTest.TestGood();

    return 0;

}