Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Alignment : warning C4316 in all classes that have aligned members

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 :

  1. 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 ?

  2. 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 ?

like image 333
MONK Avatar asked Feb 13 '14 19:02

MONK


2 Answers

This problem is not connected with passing aligned structure by const reference or not as you suggest. See this documentation of C4316. Problem is that you declare aligned structure but don't provide apropriate new/delete operators which handle this alignment.

If you get such situation with 3rd-party library which is pre-built then IMHO you should report a request to its authors to fix this problem. If you can build this library by yourself then you can add mentioned operators yourself.

like image 112
Bogdan Avatar answered Nov 11 '22 09:11

Bogdan


So after a bit of fiddling with Dr. Memory and a session of overall bug solving, it turns out :

  1. The heap wasn't being corrupt because of alignment issues. That means I still don't know to which extent this warning should be alarming, but I opted for a solution to solve it anyway :

  2. As to avoiding this warning, I've used the following solution which gives me satisfaction for now : make a class Align16 which overloads all the new and delete operators to use _aligned_malloc and _aligned_free. Then each class that includes an aligned member must inherit from Align16 to make the warning go away.
    I understand it's not portable because of _aligned_malloc, so in case I want my code to be portable Bullet Physics has a custom implementation that works to in its memory allocators which I can fall back on if needed.

like image 30
MONK Avatar answered Nov 11 '22 09:11

MONK