Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is reusing a memory location safe?

Tags:

c++

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"?

like image 631
jcoder Avatar asked Feb 11 '16 08:02

jcoder


2 Answers

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.

like image 120
Niall Avatar answered Sep 19 '22 03:09

Niall


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.

like image 35
DevSolar Avatar answered Sep 22 '22 03:09

DevSolar