Edit

Share via

Warning C26837

  • Article

Value for the comparand comp for function func has been loaded from the destination location dest through non-volatile read.

This rule was added in Visual Studio 2022 17.8.

Remarks

The InterlockedCompareExchange function, and its derivatives such as InterlockedCompareExchangePointer, perform an atomic compare-and-exchange operation on the specified values. If the Destination value is equal to the Comparand value, the exchange value is stored in the address specified by Destination. Otherwise, no operation is performed. The interlocked functions provide a simple mechanism for synchronizing access to a variable that is shared by multiple threads. This function is atomic with respect to calls to other interlocked functions. Misuse of these functions can generate object code that behaves differently than you expect because optimization can change the behavior of the code in unexpected ways.

Consider the following code:

#include <Windows.h> 
 
bool TryLock(__int64* plock) 
{ 
    __int64 lock = *plock; 
    return (lock & 1) && 
        _InterlockedCompareExchange64(plock, lock & ~1, lock) == lock; 
}

The intent of this code is:

  1. Read the current value from the plock pointer.
  2. Check if this current value has the least significant bit set.
  3. If it does have least significant bit set, clear the bit while preserving the other bits of the current value.

To accomplish this, a copy of the current value is read from theplock pointer and saved to a stack variable lock. lock is used three times:

  1. First, to check if the least-significant bit is set.
  2. Second, as the Comparand value to InterlockedCompareExchange64.
  3. Finally, in the comparison of the return value from InterlockedCompareExchange64

This assumes that the current value saved to the stack variable is read once at the start of the function and doesn't change. This is necessary because the current value is first checked before attempting the operation, then explicitly used as the Comparand in InterlockedCompareExchange64, and finally used to compare the return value from InterlockedCompareExchange64.

Unfortunately, the previous code can be compiled into assembly that behaves differently than from what you expect from the source code. Compile the previous code with the Microsoft Visual C++ (MSVC) compiler and the /O1 option and inspect the resultant assembly code to see how the value of the lock for each of the references to lock is obtained. The MSVC compiler version v19.37 produces assembly code that looks like:

plock$ = 8 
bool TryLock(__int64 *) PROC                          ; TryLock, COMDAT 
        mov     r8b, 1 
        test    BYTE PTR [rcx], r8b 
        je      SHORT $LN3@TryLock 
        mov     rdx, QWORD PTR [rcx] 
        mov     rax, QWORD PTR [rcx] 
        and     rdx, -2 
        lock cmpxchg QWORD PTR [rcx], rdx 
        je      SHORT $LN4@TryLock 
$LN3@TryLock: 
        xor     r8b, r8b 
$LN4@TryLock: 
        mov     al, r8b 
        ret     0 
bool TryLock(__int64 *) ENDP                          ; TryLock

rcx holds the value of the parameter plock. Rather than make a copy of the current value on the stack, the assembly code is re-reading the value from plock every time. This means the value could be different each time it's read. This invalidates the sanitization that the developer is performing. The value is re-read from plock after it's verified that it has its least-significant bit set. Because it's re-read after this validation is performed, the new value might no longer have the least-significant bit set. Under a race condition, this code might behave as if it successfully obtained the specified lock when it was already locked by another thread.

The compiler is allowed to remove or add memory reads or writes as long as the behavior of the code isn't altered. To prevent the compiler from making such changes, force reads to be volatile when you read the value from memory and cache it in a variable. Objects that are declared as volatile aren't used in certain optimizations because their values can change at any time. The generated code always reads the current value of a volatile object when it's requested, even if a previous instruction asked for a value from the same object. The reverse also applies for the same reason. The value of the volatile object isn't read again unless requested. For more information about volatile, see volatile. For example:

#include <Windows.h> 
 
bool TryLock(__int64* plock) 
{ 
    __int64 lock = *static_cast<volatile __int64*>(plock); 
    return (lock & 1) && 
        _InterlockedCompareExchange64(plock, lock & ~1, lock) == lock; 
}

Compile this code with same /O1 option as before. The generated assembly no longer reads plock for use of the cached value in lock.

For more examples of how the code can be fixed, see Example.

Code analysis name: INTERLOCKED_COMPARE_EXCHANGE_MISUSE

Example

The compiler might optimize the following code to read plock multiple times instead of using the cached value in lock:

#include <Windows.h> 
 
bool TryLock(__int64* plock) 
{ 
    __int64 lock = *plock; 
    return (lock & 1) && 
        _InterlockedCompareExchange64(plock, lock & ~1, lock) == lock; 
}

To fix the problem, force reads to be volatile so that the compiler doesn't optimize code to read successively from the same memory unless explicitly instructed. This prevents the optimizer from introducing unexpected behavior.

The first method to treat memory as volatile is to take the destination address as volatile pointer:

#include <Windows.h> 
 
bool TryLock(volatile __int64* plock) 
{ 
    __int64 lock = *plock; 
    return (lock & 1) && 
        _InterlockedCompareExchange64(plock, lock & ~1, lock) == lock; 
}

The second method is using volatile read from the destination address. There are a few different ways to do this:

  • Casting the pointer to volatile pointer before dereferencing the pointer
  • Creating a volatile pointer from the provided pointer
  • Using volatile read helper functions.

For example:

#include <Windows.h> 
 
bool TryLock(__int64* plock) 
{ 
    __int64 lock = ReadNoFence64(plock); 
    return (lock & 1) && 
        _InterlockedCompareExchange64(plock, lock & ~1, lock) == lock; 
}

Heuristics

This rule is enforced by detecting if the value in the Destination of the InterlockedCompareExchange function, or any of its derivatives, is loaded through a non-volatile read, and then used as the Comparand value. However, it doesn't explicitly check if the loaded value is used to determine the exchange value. It assumes the exchange value is related to the Comparand value.

See also

InterlockedCompareExchange function (winnt.h)
_InterlockedCompareExchange intrinsic functions