Raymond Chen's singleton implementation uses dodgy casts?

Question

In this link:

http://blogs.msdn.com/b/oldnewthing/archive/2011/04/06/10150261.aspx

It was recently pointed out to me that the following line:

Widget *pwidOld = reinterpret_cast<Widget*>
                 (InterlockedCompareExchangePointerRelease(
                  &reinterpret_cast<PVOID&>(g_pwidCached),
                  pwid, NULL));

has one benign and one serious problem.

The benign one is that a static_cast can be done on the return type.

The serious one seems to be:

&reinterpret_cast<PVOID&>(g_pwidCached)

I was told that that with strict aliasing, when you pass &(void*&)g_pwidCached to the function, the compiler is allowed to assume that the value of g_pwidCached isn't changing, because that change would be occurring through a pointer type that isn't the type of the object and isn't a char* (because g_pwidCached isn't a void*, it's a Widget*). 3.10/10 seems to be relevant.

Is this just a function of a particular compiler implementation and its just that Visual C++ guarantees that line will work correctly?

Answer 1

The code certainly relies on implementation-specific properties. It's not even guaranteed that Widget* is the same size as void*, never mind that some function called InterlockedCompareExchangePointerRelease will work correctly when passed a void** that points to a Widget*.

I may be overlooking something, but I think the practical issue is, "will the reference-escape code in the optimizer see reinterpret_cast<PVOID&>, and assume that no reference to gpwidCached has escaped the function?" If the answer is "yes" then we have a problem, because the compiler will assume no modification when in fact a modification does occur. But the answer is "no" provided that it treats InterlockedCompareExchangePointerRelease as a black box, because for all it knows that function casts the point back to the correct type before accessing it, in which case the compiler does not have a free hand to assume that no modifications occur.

[Edit: actually, the answer is even more "no" than I at first realised. Presumably g_pwidCached is a global, so the compiler can never assume that it isn't modified by any unknown code that it calls, regardless of the parameters. That code might modify it using the name g_pwidCached, which of course would have the correct type for there to be no aliasing.]

The answer is also "no" if InterlockedCompareExchangePointerRelease is inlined and/or implemented as a compiler intrinsic, because the implementation will (if it's correct) do whatever is necessary to ensure nothing goes wrong. Note that the function takes a void *volatile*, so however it is implemented it must do implementation-specific things to ensure no aliasing problems, because passing a type-punning pointer is the expected use-case.

I was told that that with strict aliasing, when you pass &(void*&)g_pwidCached to the function, the compiler is allowed to assume that the value of g_pwidCached isn't changing, because that change would be occurring through a pointer type that isn't the type of the object and isn't a char*

That's not quite correct. Behavior is undefined if the function actually does access the value through an incorrect type. No doubt the Windows implementation in fact does. But without seeing the definition of the function, the compiler doesn't know whether it accesses it through the type it's passed as, or somehow figures out the correct type to cast it back to in order to do the access without any violation of strict aliasing. This is the reason why (as I say above) compilers can't do any strict-aliasing-reliant optimizations around calls to unknown functions.

Answer 2

The type of g_pwidCached is wrong, it should be

void* g_pwidCached;

because the only thing that ever uses it, treats it as void*.

(Arguably delete g_pwidCached; is wrong anyway, and should be done via InterlockedExchangePointer(nullptr, &g_pwidCached) to get the value into a local and delete using the local)

But all of this, including better code, was addressed in the comments in that blog posting.