When C++ meets SAL annotations

SAL annotations introduced by Microsoft is an interesting attempt to improve security and reliability of C/C++ code. They fill the gap between human-readable documentation and documentation that compiler is able to understand. Understanding actually means that compiler can apply static analysis and check code against common mistakes like null pointer dereferences, buffer overflows, etc. To be honest, compiler is also able to analyze code without SAL – but accuracy of such analysis is questionable at best. SAL annotations allows compiler to make analysis more accurate without tons of false positives. I had recently an occasion to check myself how code analysis works in VS2012 for real world C code. It seem that it performs pretty good – for example it was able to detect deadlock in similar (more complicated) code:

BOOL func()
{
	EnterCriticalSection(&cs);
	if(var)
	{
		return FALSE;
	}
	LeaveCriticalSection(&cs);
	return TRUE;
}

Code analysis tool knows that EnterCriticalSection acquires lock so it will give me warning: C26115: Failing to release lock ‘cs’ in function ‘func’.

But this is oldschool C coding style. Now we have 21st century and we are using C++11 in brave new world of information technology. Let’s see how VS2012 code analysis works with new C++ standard.

Deadlock detection
In first test I will modify previous code to use std::mutex instead of CRITICAL_SECTION. Pretty innocent change.

BOOL func()
{
	mtx.lock();
	if(var)
	{
		return false;
	}
	mtx.unlock();
	return true;
}

After running code analysis, VS2012 is not able to detect any flaws. It seems that compiler does not understand C++ locking primitives, even if it uses WinApi under the hood. Fortunately std::mutex implementation performs runtime checks that can detect mistake. But trading compilation error for runtime error is not good deal (especially in production code).

NULL dereference
Another simple example is NULL pointer dereference. VS2012 code analysis is able to check this kind of bug even without SAL annotations (if pointer is local variable):

	char* p = 0;
	if(var) p = (char*)malloc(10);
	strcpy_s(p, 10, "hello");

As expected, compiler shows warning: C6387: ‘p’ could be ‘0’.
This code is of course in poor taste provided that we are coding in C++11. I will use std::unique_ptr instead of raw pointers, which will help to avoid memory leaks:

	std::unique_ptr<char[]>  p = 0;
	if(var) p.reset(new (std::nothrow) char[10]);
	strcpy_s(p.get(), 10, "hello");

After running code analysis, again no flaws detected. std::unique_ptr does not use SAL annotations so compiler cannot determine if p.get() can return NULL. To avoid false positives this situation is just ignored.
But what if get() method would be declared with _Maybenull_ annotation? Well, in this case you will get false positives all the time, even if p will be correct non-null pointer (yeah I’ve checked it). So this is definitely not a solution.

New overload
It seems that VS2012 is at least able to deal correctly with new operator:

  • dereference of memory allocated by new (std::nothrows) will raise warning
  • dereference of memory allocated by default new will pass

But what if we have to use overloaded new that does not throw? I know this is not very good idea to overload new in this way (it will cause problem with STL containers), but unfortunately, sometimes in legacy code it is a must. So let’s check how overloaded new will be treated:

_Maybenull_ void* operator new(size_t size)
{
	return malloc(size);
}

int _tmain(int argc, _TCHAR* argv[])
{
	char* p = (char*)new char;
	*p = 1;
	return 0;
}

Guess what? No warning at all. Visual Studio code analysis assumes that new always returns correct pointer. It totally ignores overloaded new definition.

C++ Paradox
Some wise computer scientist said that all problems in computer science can be solved by another level of indirection. Unfortunately in case of static code analysis this is not true – another level of indirection introduced by C++ abstractions (like smart pointers, portable mutexes, etc.) makes static analysis much harder. There is paradox in here: you use C++ standard library to make code more safe, only to realize that now your code analysis tools are useless. Maybe in some distant future Visual Studio will be able to verify C++ code more or less correctly. But currently all what you can do is to choose lesser evil: resign from automatic code analysis or resign from C++ standard library classes.

Advertisements

Volatile: Dark Corners of C

Volatile keyword is an interesting quirk. Used to fight back compiler optimization in order to access memory, which could be modified concurrently and unexpectedly. This includes:

  • communication with hardware via memory mapped I/O
  • access to system structures
  • access to program variables modified by multiple threads

I have described general purpose of volatile keyword in Adventures in Parallel Universe. Today we will dive more deeply into concept and we will see some confusing and counter-intuitive cases which can arise during volatile usage.

Volatile Pointers
Assume you have lock-free list with pointer to it’s tail. First thread atomically exchanges tail with pointer to new node, second waits until tail is not NULL.

struct Node {
  struct Node* prev;
  int elem;
}*tail;

void thread1() {
  while(!__sync_bool_compare_and_swap(&tail, oldtail, newtail))
    cpu_relax();
}

void thread2() {
  while(!tail) cpu_relax();
}

Because we are spinning at tail in thread2, tail has to be volatile to prevent compiler optimizations. We have 3 variants of volatile single pointer:
1. volatile Node* tail;
2. Node* volatile tail;
3. volatile Node* volatile tail;
Which should we choose?

After translating it to English we have:
1. pointer to volatile memory which means that dereferences of pointer will not be optimized by compiler. Example: while(tail->prev) will stay but while(tail) could be wiped out.
2. volatile pointer to memory which means that accessing pointer value will not be optimized. Example: while(tail) will stay but while(tail->prev) could be optimized.
3. volatile pointer to volatile memory which is combination of two previous. Example: both while(tail) and while(tail->prev) will not be optimized.

So in our case we can choose option 2 and 3. Option 3 is less desirable because in presented code we are not dereferencing memory on which tail points, but additional volatile prevents compiler from optimizations.
Of course there exists also multiple volatile pointers – rules are the same as with single volatile pointers.

Const Volatile
We can declare variable as const and volatile in the same time. This could be confusing because const tells us that variable is constant (so cannot be changed) and volatile tells us that variable could be changed. As an example let’s take code that communicates with external device via memory:

volatile int* p = (volatile int*)0xC0000000;
while(*p) cpu_relax();

In code we are only reading memory at address 0xC0000000 but we do not perform writes. And as we know, variables that are read-only should be marked as const to prevent accidental modification.

const volatile int* p = (volatile int*)0xC0000000;
while(*p) cpu_relax();

const volatile int* p means that memory pointed by p could be changed by “outside world” (like hardware) but cannot be changed by dereference of p (such attempt will result in compilation error).

Volatile Casts 1
In Windows there is a function for atomic increments of variable:

LONG __cdecl InterlockedIncrement(
  _Inout_  LONG volatile *Addend
);
...
long var;
InterlockedIncrement(&var);

Can we pass non-volatile variable to InterlockedIncrement function?
The answer is yes – in this situation we are performing implicit cast from non-volatile memory to volatile memory. It simply means that InterlockedIncrement will make less assumptions about passed argument and will expect that argument can be changed by outside world.

What with opposite situation: can we pass volatile memory to function that does not expect volatile argument (e.g. to memcpy)?

void * memcpy ( void * destination, const void * source, size_t num );
...
volatile char *dst = (volatile char*)0xC0000000;
char src[40];
memcpy((void*)dst, src, sizeof(src));

In this situation what we’ve got is undefined behavior. The C standard says:

If an attempt is made to refer to an object defined with a volatile-qualified type through use of an lvalue with non-volatile-qualified type, the behavior is undefined.

We have to use memcpy implementation that handles volatile memory or write our own implementation.

Volatile Casts 2
Assume we have legacy code which declares variable as non-volatile but we have to use it in multithreaded context:

#include <pthread.h>
#include <unistd.h>

int flag = 0;

void* thread_func(void* arg) {
    while(!(volatile int)flag);
    return 0;
}

int main(void) {
    pthread_t t;
    pthread_create(&t, 0, thread_func, 0);
    sleep(1);
    *(volatile int*)&flag = 1;
    pthread_join(t, 0);
    return 0;
}

We have casted flag to volatile so everything should be fine and program should exit. Instead of this, program goes into infinite loop. On disasembly listing you can see that while loop was transformed into infinite loop:

thread_func()
thread_func+38: jmp    0x400776 <thread_func+38>

C standard says:

The properties associated with qualified types [in our case volatile] are meaningful only for expressions that are lvalues

And expression (volatile int)flag is not lvalue (if you will try assign value to such expression you will get compilation error). This means expressions
while(flag)
and
while((volatile int)flag)
are exactly the same and your volatile cast was flushed down to a toilet. Nobody expects the Spanish Inquisition.
To fix it we have transform expression to lvalue:

while(!*(volatile int*)&flag);

Or even better – use volatile alias on non-volatile variable.

Volatile Ordering
Consider following scenario: one thread copies data to buffer and sets flag when it’s done, second thread spins on flag and then reads data from buffer:

char buffer[50];
volatile int done = 0;
void thread1() {
  for(int i = 0; i < _countof(buffer); ++i) 
    buffer[i] = next();
  done = 1;
}
void thread2() {
  while(!done) cpu_relax();
  for(int i = 0; i < _countof(buffer); ++i)
    process(buffer[i]);
}

Because flag is volatile, compiler will not cache it in registers so while loop will exit. But code is still incorrect: done=1 can be executed by compiler before for-loop. This is because buffer is not volatile. And two consecutive accesses: first to volatile variable and second to non-volatile variable (or vice versa) could be reordered by compiler. If such optimization is performed, it will result in accessing uninitialized buffer. This can be fixed by declaring buffer as volatile or by placing Compiler Memory Barrier:

void thread1() {
  for(int i = 0; i < _countof(buffer); ++i) 
    buffer[i] = next();
  KeMemoryBarrierWithoutFence();
  done = 1;
}

void thread2() {
  while(!done) cpu_relax();
  KeMemoryBarrierWithoutFence();
  for(int i = 0; i < _countof(buffer); ++i)
    process(buffer[i]);
}

KeMemoryBarrierWithoutFence will prevent reordering performed by compiler but access to memory could be still reordered by processor. Processor’s reordering, in turn, can be suppressed by Hardware Memory Barrier (but this is topic for another article… or several articles).

Volatile and Atomicity
Accessing volatile variable does not guarantee that access will be atomic. Let’s see what requirements must be met for InterlockedIncrement function to be atomic:

The variable pointed to by the Addend parameter must be aligned on a 32-bit boundary; otherwise, this function will behave unpredictably on multiprocessor x86 systems and any non-x86 systems

So we will prove that volatile does not guarantee alignment and therefore does not guarantee atomicity. We will place volatile variable into not aligned memory:

    #pragma pack(1)
    struct {
        char c;
        volatile long int v;
    } a ;
    #pragma pack()

    printf("%p", &a.v);

Output:

0x7fff2ba07971

It means that volatile variable is not aligned to 32-bit boundary which will result in non-atomic accesses on x86 processors. Even without requesting special alignment we can cause that volatile variable will be placed in not aligned memory – e.g. by putting it in memory returned from malloc (malloc does not guarantee that allocated memory will be aligned).

Volatile C++
When classes was added to C (i.e. when C++ was created) some new rules about volatile had to be established. Volatile objects, volatile methods, function overloads with volatile parameters, volatile copy constructors, volatile partial template specialization… It’s really funny to observe how C++ amplifies complexity of C features. But in case of volatile, which is hard to use correctly even in plane C, level of complication in C++ becomes insane. It is tempting to just ignore topic and do not use volatile in conjunction with C++ stuff. Unfortunately new C++ standard comes with bunch of generic classes that uses volatile for multithreading. This means if you want to understand and use correctly C++11 threading library you will have dig into gory details of C++’s volatile. Stay tuned…