I encountered much trouble today by tracking a really evasive corruption bug.
I guess it wouldn't have been so hard to find it if I actually paid attention to the the warnings, but as I couln't find relevant information on why this specific warning popped up I let it slide, which was a mistake.
So here is the incriminated warning Visual Studio 2013 gives me :
warning C4316: object allocated on the heap may not be aligned 16
And it is generated when passing an align(16) temporary to a constructor by const reference, as the following code demonstrate :
class Vector
{};
__declspec(align(16)) class VectorA
{};
class Shape
{
public:
Shape(const Vector& vec) {}
};
class ShapeA
{
public:
ShapeA(const VectorA& vec) : mVec(vec) {}
private:
VectorA mVec;
};
int main(int argc, char *argv[])
{
Shape* shape = new Shape(Vector()); // ok
ShapeA* shapea = new ShapeA(VectorA()); // warning C4316:
// object allocated on the heap may not be aligned 16
}
Which brings a couple of questions :
As I'm not responsible for the alignment of said struct (it comes from a library), I can't really effect on that. What would be the recommended way to work around this problem ?
As this is a warning, I was guessing it wouldn't have dramatic effect. But the effects I discovered were quite dramatic and silent (which only appeared when passing a completely unrelated reference to a float to an sqlite function).(Edit : this is actually wrong, the heap hasn't been corrupted because of this) Is it really as straightforward as : "This code will surely cause heap corruption, don't do it." ? or is there a more complex chain of actions to cause something like this ?
One workaround I thought of is to have a unique_ptr to the aligned object instead of having it as a direct member, but I'm a bit reluctant to add unique_ptr everywhere I had plain structs before (even if it allows me to pimpl my code)
Addendum
As it turns out I was subclassing the class VectorA which did provide new an delete overloads assuring the proper alignment, but my own subclass didn't. But a second occurence of this warning in the next paragraph is more tricky to get rid of :
Extended
The other situation producing the warning is the following :
__declspec(align(16)) class VectorA
{
void* operator new(size_t size)
{
void* p = _aligned_malloc(size, 16);
if(p == 0) throw std::bad_alloc();
return p;
}
void operator delete(void *p)
{
VectorA* pc = static_cast<VectorA*>(p);
_aligned_free(p);
}
};
class ShapeB
{
public:
ShapeB() {}
private:
VectorA mVec;
};
int main()
{
std::unique_ptr<BoxShapeB> shapeb = std::make_unique<BoxShapeB>();
}
It seems any class of mine having an aligned member like in this example will produce a warning too when instantiated. Like I said above a way to remove the warning is to have pointers to these member. This warning didn't occur in the precedent versions of Visual Studio with the same exact code, so my question is : to what extent is the above plainly wrong and should it be avoided at all costs ? i.e. what should I make of the warning ?
Supposing most of my classes all have a common base class Object, and most of my classes have align(16) Vector as members, is it worth it to just put the overloaded new and delete in this base class and forget about turning all my members Vectors to pointers ? Would that approach work ?