Proposal: After the lifetime of an object data within its layout has undefined value

[christophe...@gmail.com]

Godbolt to compile code.  

#include <experimental/optional>

struct O {

   bool inited = false;

   int  value;

    ~O() {

       if(inited)

           inited = false;

   }

};

void destruct_optional_int(O& o) {

   o.~O(); // should be noop ?!

}

void more_optional_int() {

   O o;    // address does not leak

}

A few years ago I noticed boost::optional<int> deconstructing with a branch.  One could argue such containers should be coded to have trivial construction.  It seems the container std::optional that ships with gcc and clang have fixed the issue.

I always thought there was room in the language spec to help optimization.  I'd like to propose something like this:

After the lifetime of an object has ended data within its layout has undefined value.

Often the compiler can figure out that operations on such an object's memory are undefined anyway, if the object existed on the stack or a delete expression releases it to the heap.  However in cases where the memory is from a pool or container are or the address leaks out of compilation scope, it may not be able to prove this.

Note that the implementation can optimize further.  Once it is known the memory of an object is undefined at a certain point, any previous stores to that memory may be elided as long as they can be shown to have no side effects up until that point.

It looks like gcc is assuming this is in the standard already.  Clang and ICC do not.  Does anyone know if gcc is right?

[Nicolas Lesser]

On Tue, Apr 17, 2018, 1:47 PM <christophe...@gmail.com> wrote:

Godbolt to compile code.  

#include <experimental/optional>

struct O {

   bool inited = false;

   int  value;

    ~O() {

       if(inited)

           inited = false;

   }

};

void destruct_optional_int(O& o) {

   o.~O(); // should be noop ?!

}

void more_optional_int() {

   O o;    // address does not leak

}

A few years ago I noticed boost::optional<int> deconstructing with a branch.  One could argue such containers should be coded to have trivial construction.  It seems the container std::optional that ships with gcc and clang have fixed the issue.

I always thought there was room in the language spec to help optimization.  I'd like to propose something like this:

After the lifetime of an object has ended data within its layout has undefined value.

But this is already the case. See [basic.life]p1.3 that says that the lifetime of an (class) object ends when the destructor is called and [basic.life]p6.2 which says that non-static member access after the lifetime of the object has ended is undefined behavior.

Often the compiler can figure out that operations on such an object's memory are undefined anyway, if the object existed on the stack or a delete expression releases it to the heap.  However in cases where the memory is from a pool or container are or the address leaks out of compilation scope, it may not be able to prove this.

That's why it's UB.

Note that the implementation can optimize further.  Once it is known the memory of an object is undefined at a certain point, any previous stores to that memory may be elided as long as they can be shown to have no side effects up until that point.

True.

It looks like gcc is assuming this is in the standard already.  Clang and ICC do not.  Does anyone know if gcc is right?

Yes. gcc just has another definition of UB in this case, or rather, it can prove that the object was destructed, while clang and icc cannot. This is a QoI issue, not a standard's one.

[Hyman Rosen]

On Tue, Apr 17, 2018 at 8:55 AM, Nicolas Lesser <blitz...@gmail.com> wrote:

The fundamental question is whether the side effect of modifying an object must
also modify the storage of the object.  If the compiler elides the assignment in
destruct_optional_int, the following code will fail:

alignas(O) unsigned char buf1[sizeof(O)], buf2[sizeof(O)];
O *o = new(buf1) O;
o->inited = true;
o->value = 0;
memcpy(buf2, buf1, sizeof(O));
destruct_optional_int(*o);
assert(memcmp(buf1, buf2, sizeof(O)) != 0);

This explains the compiled code.  The  destruct_optional_int function cannot know
the underlying storage of its parameter, so it must do the side effect.

The more_optional_int function does know that the underlying storage is not used
after the destructor, so the side effect can be elided.

[christophe...@gmail.com]

On Tuesday, 17 April 2018 13:55:36 UTC+1, Nicolas Lesser wrote:

On Tue, Apr 17, 2018, 1:47 PM <christophe...@gmail.com> wrote:

I always thought there was room in the language spec to help optimization.  I'd like to propose something like this:

After the lifetime of an object has ended data within its layout has undefined value.

But this is already the case. See [basic.life]p1.3 that says that the lifetime of an (class) object ends when the destructor is called and [basic.life]p6.2 which says that non-static member access after the lifetime of the object has ended is undefined behavior.

Oh, my bad there it is.  It's also more explicit in [class.cdtor]

For an object with a non-trivial destructor, referring to any non-static member or base class of the object after the destructor finishes execution results in undefined behavior.

The non-trivial destructor clause raises an issue though.  Consider these cases:

void vector_int(std::vector<int>& v) {

   v.back() *= 0xDEADBEEF;  // can't be removed

   v.pop_back();

}

void vector_O(std::vector<O>& v) {

   v.back().value *= 0xDEADBEEF;

   v.pop_back();

}

void vector_optional(std::vector<std::optional<int>>& v) {

   *(v.back()) *= 0xDEADBEEF;

   v.pop_back();

}

Only vector_O is optimized away.  I assume the issue is trivial deconstruction.  The vector_optional case is especially annoying since the implementer of optional worked hard to give it trivial deconstruction.  

I'm pretty sure the spec for vector says referencing beyond end is undefined.  Is there a way for the container implementer to tell the compiler data is undefined?  I've wondered if someone has proposed something like that.

I've thought about a set_undefined(T&) function that at least for trivial types overwrites them with uninitialized memory and/or calls VALGRIND_MAKE_MEM_UNDEFINED or ASAN_UNPOISON_MEMORY_REGION.  If I did this in my code though, there's still the risk that some compiler would actually feel obligated to overwrite the data with junk instead of just taking the hint that this is undefined.

[Hyman Rosen]

On Tue, Apr 17, 2018 at 4:55 PM, <christophe...@gmail.com> wrote:

I've thought about a set_undefined(T&) function that at least for trivial types overwrites them with uninitialized memory and/or calls VALGRIND_MAKE_MEM_UNDEFINED or ASAN_UNPOISON_MEMORY_REGION.  If I did this in my code though, there's still the risk that some compiler would actually feel obligated to overwrite the data with junk instead of just taking the hint that this is undefined.

Optimizationism is a curse.  C++ doesn't need more ways for the compiler
to eliminate code that the programmer wrote.  It needs coherent and simple
design so that everyone, human and machine, who looks at a piece of code
knows what that code does. 

[Arthur O'Dwyer]

On Tuesday, April 17, 2018 at 1:55:28 PM UTC-7, christophe...@gmail.com wrote:

Only vector_O is optimized away.  I assume the issue is trivial deconstruction.  The vector_optional case is especially annoying since the implementer of optional worked hard to give it trivial deconstruction.  

Wow, that's a neat case!  Here it is reduced to its essence:

https://godbolt.org/g/uVjdrW

 

I'm pretty sure the spec for vector says referencing beyond end is undefined.  Is there a way for the container implementer to tell the compiler data is undefined?  I've wondered if someone has proposed something like that.

I can't think of any such mechanism currently.

It wouldn't work to create just a std::unbless() mechanism, either, because just as the vector is currently skipping the destructor call, it might also skip a constructor call when it starts using that element's memory again (after a push_back, or let's say if you're concatenating another vector onto this vector and we decide to use memcpy for the concatenation, so we skip calling the constructors of the new elements).  So if the vector calls "unbless" when it skips a destructor, it would also have to call "bless" when it skips a constructor.

There is lots of talk about std::bless (and std::launder, which is related but distinct, I think). I don't know what the active proposals are, if any. I don't know if any of them include support for std::unbless / std::curse... but I'm pretty sure I've heard people bikeshedding those names before! ;)

–Arthur

[Ren Industries]

Optimizations are why we use C++ for games. We absolutely need more ways for the compiler to eliminate code, assuming those mechanisms are well specified and clear.

On Tue, Apr 17, 2018 at 5:04 PM, Hyman Rosen <hyman...@gmail.com> wrote:

--
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Future Proposals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-proposals+unsubscribe@isocpp.org.
To post to this group, send email to std-pr...@isocpp.org.
To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/CAHSYqdbn-N-LCqk_%3DyPx%2B43g_Z_MzB4vQk0oYax7uD7O%3DKv1Xg%40mail.gmail.com.

[Nicol Bolas]

On Tuesday, April 17, 2018 at 9:16:40 PM UTC-4, Ren Industries wrote:

Optimizations are why we use C++ for games. We absolutely need more ways for the compiler to eliminate code, assuming those mechanisms are well specified and clear.

Normally, I disagree with Hyman about thinks like this. But in this particular case, I have to agree. Or at least partially; it's crazy to expect the compiler to recognize cases like this. And it's even worse to start adding functions and other unnecessary bits to help the compiler detect such occurrences.

That's not to say that I think that the compiler shouldn't be able to eliminate it. After all, it's impossible for anyone to tell the difference, so compilers are free to do so. But I don't think anyone should look at that code and assume that the compiler will catch it. And I certainly don't think we should start necessitating special functions like `std::unbless` or whatever just to tell the compiler something.

Reliance on the omniscience of the compiler is not always a good idea. Better to learn to write good code yourself than to assume the compiler will catch you.

[Avi Kivity]

Related gcc bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61982

--
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Future Proposals" group.

To unsubscribe from this group and stop receiving emails from it, send an email to std-proposal...@isocpp.org.

To post to this group, send email to std-pr...@isocpp.org.

To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/1abb061a-5da1-4dec-af26-a2f0184543c3%40isocpp.org.

[T. C.]

Currently, a destructor call does not end the lifetime of a trivially destructible object. See [basic.life]/p1. Core issue 2256 is going to change that.

[christophe...@gmail.com]

On Wednesday, 18 April 2018 23:34:57 UTC+1, T. C. wrote:

Currently, a destructor call does not end the lifetime of a trivially destructible object. See [basic.life]/p1. Core issue 2256 is going to change that. 

Cool!  So does the proposed solution mean that deconstruction of a POD would invalidate it?  I guess the container just needs to keep calling std::destroy_at(std::addressof(thing)).

Theoretically you could do this to a trivially destructible object on the stack to shorten its lifetime.

Have there been objects when sub-objects reference each other?

volatile int sink=0;

struct A {

   int& ir;

   ~A () { sink += ir++; }  // Undefined Behavior?

};

struct B : A{

   int  i = 3;

   B() : A{i} {}

};

void bar() {

   B b;

   b.i = 7;  // May the compiler skip this store?

}

B is being naughty giving A ref to its i, but traditionally this was safe code.  The storage isn't released until the whole object goes.