Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

clarification of specifics of P0137

In the following code I have been meticulous in the following of the standard's words (plus in the light of the wording of P0137) on object lifetimes.

Note that all memory allocation is through suitably-aligned storage of type unsigned char, as per P0137.

Note also that Foo is a POD, with a trivial constructor.

Questions:

A. If I have misunderstood the standard, and there is any UB here, please kindly point it out (or alternatively confirm that there is no UB)

B. Are the initialisations at A, B, C, D, E, F strictly necessary in light of the fact that the construction is trivial, and performs no actual initialisation. If so, please indicate which part of the standard contradicts or clarifies [object.lifetime] in this regard.

code:

#include <memory>

// a POD with trivial constructor
struct Foo 
{
  int x;
};

struct destroy1
{
  void operator()(Foo* p)
  {
    // RAII to guarantee correct destruction order
    auto memory = std::unique_ptr<unsigned char[]>(reinterpret_cast<unsigned char*>(p));
    p->~Foo(); // A
  }
};
std::unique_ptr<Foo, destroy1> create1()
{
  // RAII to guarantee correct exception handling
  auto p = std::make_unique<unsigned char[]>(sizeof(Foo));
  auto pCandidate = reinterpret_cast<Foo*>(p.get());
  new (pCandidate) Foo(); // B
  return std::unique_ptr<Foo, destroy1>(reinterpret_cast<Foo*>(p.release()), 
                                        destroy1());
}

struct call_free
{
  void operator()(void *p) const { std::free(p); } 
};
using malloc_ptr = std::unique_ptr<unsigned char, call_free>;

struct destroy2
{
  void operator()(Foo *pfoo) const {
    // RAII to guarantee correct destruction order
    auto memory = malloc_ptr(reinterpret_cast<unsigned char*>(pfoo));
    pfoo->~Foo(); // C
  }
};

std::unique_ptr<Foo, destroy2> create2()
{
    // RAII to guarantee correct exception handling
  auto p = malloc_ptr(reinterpret_cast<unsigned char*>(std::malloc(sizeof(Foo))));
  auto pCandidate = reinterpret_cast<Foo*>(p.get());
  new (pCandidate) Foo(); // D
  return std::unique_ptr<Foo, destroy2>(reinterpret_cast<Foo*>(p.release()), 
                                        destroy2());
}

struct nodelete {
  void operator()(Foo * p) {
    p->~Foo();  // E
  }
};

std::shared_ptr<Foo> provide()
{
  alignas(Foo) static unsigned char  storage[sizeof(Foo)];

  auto make = [] {
    auto p = reinterpret_cast<Foo*>(storage);
    new (p) Foo (); // F
    return std::shared_ptr<Foo>(p, nodelete());
  };

  static std::shared_ptr<Foo> pCandidate = make();

  return pCandidate;
}


int main()
{
  auto foo1 = create1();
  auto foo2 = create2();
  auto foo3 = provide();

  foo1->x = 1;
  foo2->x = 2;
  foo3->x = 3;
}
like image 735
Richard Hodges Avatar asked Dec 02 '16 10:12

Richard Hodges


2 Answers

If you take seriously Core Issue 1776 and the never voted for idea that "that malloc alone is not sufficient to create an object", then you have to take seriously these ideas:

  • unions were not usable without UB in C++ until quite recently, as the lifetime of their members was not defined
  • string literals cannot be examined, as their lifetime is never started

and many other deeper more difficult controversies and contradictions, like what is a lvalue, an object, is lifetime a property of a preexisting object (that exists outside it's life), etc.

Yet I don't see people taking at least the two bullet points seriously. Why would the claim in the DR be taken seriously then?

like image 59
curiousguy Avatar answered Nov 03 '22 10:11

curiousguy


On create1

std::unique_ptr<Foo, destroy1>(reinterpret_cast<Foo*>(p.release()), destroy1());

This doesn't work, because you're using the wrong pointer.

p.release() thinks it points to an unsigned char[]. However, that's not the object you want to point to. What you want to point to is the object that lives inside this array, the Foo you've created.

So you are now subject to [basic.life]/8. The gist of that is that you can only use the previous pointer as a pointer to the new object if they are of the same type. Which they're not in your case.

Now, I could tell you to launder the pointer, but the more reasonable way to handle this is to just store the pointer returned by the placement-new call:

auto p = std::make_unique<unsigned char[]>(sizeof(Foo));
auto ret = std::unique_ptr<Foo, destroy1>(new(p.get()) Foo(), destroy1());
p.release();
return ret;

That pointer will always be correct.

Your use of of placement-new is not optional. [intro.object]/1 tells us:

An object is created by a definition (3.1), by a new-expression (5.3.4), when implicitly changing the active member of a union (9.3), or when a temporary object is created (4.4, 12.2).

When you allocate an unsigned char[], that's the object you have created in that storage. You cannot simply pretend that it is a Foo, just because Foo is an aggregate. [intro.object]/1 doesn't allow that. You must explicitly create that object via one of the mechanisms listed above. Since you can't use a definition, union member activation, or temporary objects with arbitrary memory buffers to create objects from existing storage, the only recourse you have to create objects is a new-expression.

Specifically, placement-new.

As for delete1, you do need a custom deleter, since the default deleter will call delete on the Foo pointer. Your code is as follows:

auto memory = std::unique_ptr<unsigned char[]>(reinterpret_cast<unsigned char*>(p));
p->~Foo();

unsigned char[] has some special logic to it, in terms of how it behaves when objects are allocated in their storage, thanks to [intro.object]/3-4. If the object entirely overlays the storage of the unsigned char[], then it functions as if the object were allocated within the array. That means that the unsigned char[] is still technically there; it has not destroy the byte array.

As such, you can still delete the byte array, which your code here does.

On create2

This is also wrong, due to further violations of [basic.life]/8. A fixed version would be similar to the above:

auto p = malloc_ptr(reinterpret_cast<unsigned char*>(std::malloc(sizeof(Foo))));
auto ret std::unique_ptr<Foo, destroy2>(new(p.get()) Foo(), destroy2());
p.release();
return ret;

Unlike new-expressions, malloc never creates an object via [intro.object]/1; it only acquires storage. As such, placement-new is again required.

Similarly, free just releases memory; it doesn't deal with objects. So your delete2 is essentially fine (though the use of malloc_ptr there makes it needlessly confusing).

On provide

This has the same [basic.life]/8 problems that the rest of your examples have:

alignas(Foo) static unsigned char storage[sizeof(Foo)];
static auto pCandidate = std::shared_ptr<Foo>(new(storage) Foo(), nodelete());
return pCandidate;

But other than that, it's fine (so long as you don't break it elsewhere). Why? That's complex.

[basic.start.term]/1 tells us that static objects are destroyed in the reverse order of their initialization. And [stmt.decl]/4 tells us that block-scoped static objects are initialized in the order they are encountered in a function.

Therefore, we know that pCandidate will be destroyed before storage. So long as you don't keep a copy of that shared_ptr in a static variable, or otherwise fail to destroy/reset all such shared objects before termination, you should be fine.


That all being said, using blocks of unsigned char is really pre-C++11. We have std::aligned_storage and std::aligned_union now. Use them.

like image 34
Nicol Bolas Avatar answered Nov 03 '22 12:11

Nicol Bolas