Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AllocationHeader is not copied from stack to the memory position #8

Open
talhasaruhan opened this issue May 22, 2018 · 2 comments
Open

Comments

@talhasaruhan
Copy link

talhasaruhan commented May 22, 2018

In StackAllocator you're doing this:

AllocationHeader allocationHeader{padding}; AllocationHeader * headerPtr = (AllocationHeader*) headerAddress; headerPtr = &allocationHeader;

Which doesn't copy the AllocationHeader object that's created on the stack into the memory position.

You should either use

memcpy(headerAddress, &allocationHeader, sizeof(AllocationHeader))

or just use placement new operator to create the object at the memory location.

I've tested this and just as I expected, you're actually getting garbage when you're reading the header in the Free function.

@prabhatg
Copy link

prabhatg commented Sep 5, 2018

Is this warning ok?

src/StackAllocator.cpp:42:46: warning: narrowing conversion of 'padding' from 'std::size_t {aka long unsigned int}' to 'char' inside { } [-Wnarrowing]
AllocationHeader allocationHeader{padding};
^

@talhasaruhan
Copy link
Author

talhasaruhan commented Sep 6, 2018

Well, the author of the code defined AllocationHeader struct as follows (in the header file):

struct AllocationHeader {
        char padding;
    };

so when using list initialization, you're converting "padding" variable from size_t to char, which is theoretically not safe. BUT, since the actual padding will almost always fit into a char (meaning that it's integer value has to be less than 256) it'll work just fine. But if you're trying to align to huge blocks of memory, then padding may overflow.

So the best way to handle this depends on your use case, if you can assume that paddings before headers will always be less than or equal to 255, then you can just ignore the warning. But if you're trying to, for some reason align your blocks to huge chunks, say 512, then padding CAN overflow (or it may be undefined behavior, either case it's unsafe) so you should change the "char" in the definition to "size_t"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants