64

This question is based on some existing C code ported to C++. I am just interested in whether it is "safe". I already know I wouldn't have written it like this. I am aware that the code here is basically C rather than C++ but it's compiled with a C++ compiler and I know that the standards are slightly different sometimes.

I have a function that allocates some memory. I cast the returned void* to an int* and start using it.

Later on I cast the returned void* to a Data* and start using that.

Is this safe in C++?

Example :-

void* data = malloc(10000);

int* data_i = (int*)data;
*data_i = 123;
printf("%d\n", *data_i);

Data* data_d = (Data*)data;
data_d->value = 456;
printf("%d\n", data_d->value);

I never read variables used via a different type than they were stored but worry that the compiler might see that data_i and data_d are different types and so cannot legally alias each other and decide to reorder my code, for example putting the store to data_d before the first printf. Which would break everything.

However this is a pattern that is used all the time. If you insert a free and malloc in between the two accesses I don't believe it alters anything as it doesn't touch the affected memory itself and can reuse the same data.

Is my code broken or is it "correct"?

3
  • 9
    If Data is not a POD, you should probably use placement new (isocpp.org/wiki/faq/dtors#placement-new) and make sure its destructor is called. Feb 11, 2016 at 8:24
  • 1
    I guess we can assume a Data fits in 10000 bytes? Feb 11, 2016 at 13:28
  • 1
    By the way, I realise you are just porting and not creating the code, but this sort of thing is a great opportunity to use a union and indeed what they were created for. It might be worth using one here. It will also add some small level of type safety.
    – Vality
    Feb 11, 2016 at 22:57

8 Answers 8

52

It's "OK", it works as you have written it (assuming primitives and plain-old-datatypes (PODs)). It is safe. It is effectively a custom memory manager.

Some notes:

  • If objects with non-trivial destructors are created in the location of the allocated memory, make sure it is called

    obj->~obj();
    
  • If creating objects, consider the placement new syntax over a plain cast (works with PODs as well)

    Object* obj = new (data) Object();
    
  • Check for a nullptr (or NULL), if malloc fails, NULL is returned

  • Alignment shouldn't a problem, but always be aware of it when creating a memory manager and make sure that the alignment is appropriate

Given you are using a C++ compiler, unless you want to keep the "C" nature to the code you can also look to the global operator new().

And as always, once done don't forget the free() (or delete if using new)


You mention that you are not going to convert any of the code just yet; but if or when you do consider it, there are a few idiomatic features in C++ you may wish to use over the malloc or even the global ::operator new.

You should look to the smart pointer std::unique_ptr<> or std::shared_ptr<> and allow them to take care of the memory management issues.

3
  • 2
    Thanks! Yes, I would certainly do all of that in new C++ code, this is existing code from a old C project that had been changed to use C++ in a few places. I was looking out for strict aliasing issues when it occurred to me that this might not be safe either. I don't intend to change the code at this time, just wanted to make sure it wasn't broken!
    – jcoder
    Feb 11, 2016 at 8:36
  • 1
    Am I missing something on strict aliasing? Why is this okay ... can't the compiler rearrange the instructions data_d->value = 456 to be before the printf of *data_i because those are different pointer types and the compiler can assume they access different memory regions (even in instances where it is obvious the point to the same region!). I would have thought a memory fence was required.
    – dave
    Feb 12, 2016 at 1:38
  • 3
    @dave: According to the C++ standard, POD objects are destroyed when their memory is reused in the fashion OP is asking about. This is not true of any remotely interesting objects, however.
    – Kevin
    Feb 12, 2016 at 5:20
25

Depending on the definition of Data, your code might be broken. It's bad code, either way.

If Data is a plain old data type (POD, i.e. a typedef for a basic type, a struct of POD types etc.), and the allocated memory is properly aligned for the type (*), then your code is well-defined, which means it will "work" (as long as you initialize each member of *data_d before using it), but it is not good practice. (See below.)

If Data is a non-POD type, you are heading for trouble: The pointer assignment would not have invoked any constructors, for example. data_d, which is of type "pointer to Data", would effectively be lying because it points at something, but that something is not of type Data because no such type has been created / constructed / initialized. Undefined behaviour will be not far off at that point.

The solution for properly constructing an object at a given memory location is called placement new:

Data * data_d = new (data) Data();

This instructs the compiler to construct a Data object at the location data. This will work for POD and non-POD types alike. You will also need to call the destructor (data_d->~Data()) to make sure it is run before deleteing the memory.

Take good care to never mix the allocation / release functions. Whatever you malloc() needs to be free()d, what is allocated with new needs delete, and if you new [] you have to delete []. Any other combination is UB.


In any case, using "naked" pointers for memory ownership is discouraged in C++. You should either

  1. put new in a constructor and the corresponding delete in the destructor of a class, making the object the owner of the memory (including proper deallocation when the object goes out of scope, e.g. in the case of an exception); or

  2. use a smart pointer which effectively does the above for you.


(*): Implementations are known to define "extended" types, the alignment requirements of which are not taken into account by malloc(). I'm not sure if language lawyers would still call them "POD", actually. MSVC, for example, does 8-byte alignment on malloc() but defines the SSE extended type __m128 as having a 16-byte alignment requirement.

7
  • 3
    When would memory allocated with malloc not be properly aligned for POD type, assuming that user won't use pointer arithmetric on returned pointer? As far as I know, it's not possible on C; is C++ any different?
    – user694733
    Feb 11, 2016 at 9:20
  • 4
    @user694733: Implementations are known to define "extended" types, the alignment requirements of which are not taken into account by malloc(). I'm not sure if language lawyers would still call them "POD", actually. MSVC, for example, does 8-byte alignment on malloc() but defines the SSE extended type __m128 as having a 16-byte alignment requirement.
    – DevSolar
    Feb 11, 2016 at 9:30
  • 2
    Shouldn't that be Data * data_d = new (data) Data() ?
    – ymett
    Feb 11, 2016 at 15:03
  • 2
    Clarification: it might be bad C++, but it's perfectly good C. Feb 12, 2016 at 0:48
  • Your answer could be a little more explicit that it is grossly illegal to mix new and free() or malloc() and delete.
    – Kevin
    Feb 12, 2016 at 5:26
16

The rules surrounding strict aliasing can be quite tricky.

An example of strict aliasing is:

int a = 0;
float* f = reinterpret_cast<float*>(&a);
f = 0.3;
printf("%d", a);

This is a strict aliasing violation because:

  • the lifetime of the variables (and their use) overlap
  • they are interpreting the same piece of memory through two different "lenses"

If you are not doing both at the same time, then your code does not violate strict aliasing.


In C++, the lifetime of an object starts when the constructor ends and stops when the destructor starts.

In the case of built-in types (no destructor) or PODs (trivial destructor), the rule is instead that their lifetime ends whenever the memory is either overwritten or freed.

Note: this is specifically to support writing memory managers; after all malloc is written in C and operator new is written in C++ and they are explicitly allowed to pool memory.


I specifically used lenses instead of types because the rule is a bit more difficult.

C++ generally use nominal typing: if two types have a different name, they are different. If you access a value of dynamic type T as if it were a U, then you are violating aliasing.

There are a number of exceptions to this rule:

  • access by base class
  • in PODs, access as a pointer to the first attribute

And the most complicated rule is related to union where C++ shifts to structural typing: you can access a piece of memory through two different types, if you only access parts at the beginning of this piece of memory in which the two types share a common initial sequence.

§9.2/18 If a standard-layout union contains two or more standard-layout structs that share a common initial sequence, and if the standard-layout union object currently contains one of these standard-layout structs, it is permitted to inspect the common initial part of any of them. Two standard-layout structs share a common initial sequence if corresponding members have layout-compatible types and either neither member is a bit-field or both are bit-fields with the same width for a sequence of one or more initial members.

Given:

  • struct A { int a; };
  • struct B: A { char c; double d; };
  • struct C { int a; char c; char* z; };

Within a union X { B b; C c; }; you can access x.b.a, x.b.c and x.c.a, x.c.c at the same time; however accessing x.b.d (respectively x.c.z) is a violation of aliasing if the currently stored type is not B (respectively not C).

Note: informally, structural typing is like mapping down the type to a tuple of its fields (flattening them).

Note: char* is specifically exempt from this rule, you can view any piece of memory through char*.


In your case, without the definition of Data I cannot say whether the "lenses" rule could be violated, however since you are:

  • overwriting memory with Data before accessing it through Data*
  • not accessing it through int* afterwards

then you are compliant with the lifetime rule, and thus there is no aliasing taking place as far as the language is concerned.

9
  • I thought that given your declarations above plus A a; B b; C c;, the following is ok, as you say: int i = ((A *)&B)->a; (i.e. access through a base class expression), while the following is violating the aliasing rules: int i = ((A *)&C)->a; (access through an expression of unrelated type). Am I mistaken? Did you mean something different? Feb 11, 2016 at 11:42
  • @PeterA.Schneider: int i = ((A *)&c)->a; is specifically allowed for trivial layout types (no virtual anywhere). Viewing types as tuples is helpful here: a B is ultimately (int, char, double) while a C is (int, char, char*) so the language allows accessing the first two elements. This only applies to trivial layout because the layout compatibility is not guaranteed if you mix virtual pointers, virtual base classes, multiple bases, ... Attempting to access the third one ((B*)&c)->d however would be an aliasing violation: not part of the common prefix subsequence. Feb 11, 2016 at 12:24
  • Can you cite the standard supporting that? I thought that the Python PyObject bug was exactly this scenario (casting between two structs which start with the same sequence of field types). I see 3.10/10, "an aggregate or union type that includes one of the aforementioned types among its elements or nonstatic data members" -- which is unintelligible to me: That condition is so wide (there is no mention of element order, or that it must be at the start of the struct!) that it would permit far too many aliasings. Feb 11, 2016 at 12:38
  • @PeterA.Schneider: I am not promising anything for C, however in the C++14 standard (though I think it's unchanged from at least C++11) you will be interested in §9.2/18: If a standard-layout union contains two or more standard-layout structs that share a common initial sequence, and if the standard-layout union object currently contains one of these standard-layout structs, it is permitted to inspect the common initial part of any of them. => it seems I was a bit enthusiastic and it is limited to types in union and not arbitrary pointer casts; I'll edit. Feb 11, 2016 at 13:10
  • 2
    @jcoder: Actually, I think this code is fine, because the temporal use of the memory for each variable does not overlap. Feb 11, 2016 at 15:25
8

As long as the memory is used for only one thing at a time it's safe. You're basically use the allocated data as a union.

If you want to use the memory for instances of classes and not only simple C-style structures or data-types, you have to remember to do placement new to "allocate" the objects, as this will actually call the constructor of the object. The destructor you have to call explicitly when you're done with the object, you can't delete it.

6

As long as you only handle "C"-types, this would be ok. But as soon as you use C++ classes you will get into trouble with proper initialization. If we assume that Data would be std::string for example, the code would be very wrong.

The compiler cannot really move the store across the call to printf, because that is a visible side effect. The result has to be as if the side effects are produced in the order the program prescribes.

5

Effectively, you've implemented your own allocator on top of malloc/free that reuses a block in this case. That's perfectly safe. Allocator wrappers can certainly reuse blocks so long as the block is big enough and comes from a source that guarantees sufficient alignment (and malloc does).

5

As long a Data remains a POD this should be fine. Otherwise you would have to switch to placement new.

I would however put a static assert in place so that this doesn't change during later refactoring

4

I don't find any mistake in reusing the memory space. Only what I care for is the dangling reference. Reusing memory space as you have said I think it doesn't have any effect on the program.
You can go on with your programming. But it is always preferable to free() the space and then allocate to another variable.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.