30

Does the following program have undefined behavior in C++17 and later?

struct A {
    void f(int) { /* Assume there is no access to *this here */ }
};

int main() {
    auto a = new A;
    a->f((a->~A(), 0));
}

C++17 guarantees that a->f is evaluated to the member function of the A object before the call's argument is evaluated. Therefore the indirection from -> is well-defined. But before the function call is entered, the argument is evaluated and ends the lifetime of the A object (see however the edits below). Does the call still have undefined behavior? Is it possible to call a member function of an object outside its lifetime in this way?

The value category of a->f is prvalue by [expr.ref]/6.3.2 and [basic.life]/7 does only disallow non-static member function calls on glvalues referring to the after-lifetime object. Does this imply the call is valid? (Edit: As discussed in the comments I am likely misunderstanding [basic.life]/7 and it probably does apply here.)

Does the answer change if I replace the destructor call a->~A() with delete a or new(a) A (with #include<new>)?


Some elaborating edits and clarifications on my question:


If I were to separate the member function call and the destructor/delete/placement-new into two statements, I think the answers are clear:

  1. a->A(); a->f(0): UB, because of non-static member call on a outside its lifetime. (see edit below, though)
  2. delete a; a->f(0): same as above
  3. new(a) A; a->f(0): well-defined, call on the new object

However in all these cases a->f is sequenced after the first respective statement, while this order is reversed in my initial example. My question is whether this reversal does allow for the answers to change?


For standards before C++17, I initially thought that all three cases cause undefined behavior, already because the evaluation of a->f depends on the value of a, but is unsequenced relative to the evaluation of the argument which causes a side-effect on a. However, this is undefined behavior only if there is an actual side-effect on a scalar value, e.g. writing to a scalar object. However, no scalar object is written to because A is trivial and therefore I would also be interested in what constraint exactly is violated in the case of standards before C++17, as well. In particular, the case with placement-new seems unclear to me now.


I just realized that the wording about the lifetime of objects changed between C++17 and the current draft. In n4659 (C++17 draft) [basic.life]/1 says:

The lifetime of an object o of type T ends when:

  • if T is a class type with a non-trivial destructor (15.4), the destructor call starts

[...]

while the current draft says:

The lifetime of an object o of type T ends when:

[...]

  • if T is a class type, the destructor call starts, or

[...]

Therefore, I suppose my example does have well-defined behavior in C++17, but not he current (C++20) draft, because the destructor call is trivial and the lifetime of the A object isn't ended. I would appreciate clarification on that as well. My original question does still stands even for C++17 for the case of replacing the destructor call with delete or placement-new expression.


If f accesses *this in its body, then there may be undefined behavior for the cases of destructor call and delete expression, however in this question I want to focus on whether the call in itself is valid or not. Note however that the variation of my question with placement-new would potentially not have an issue with member access in f, depending on whether the call itself is undefined behavior or not. But in that case there might be a follow-up question especially for the case of placement-new because it is unclear to me, whether this in the function would then always automatically refer to the new object or whether it might need to potentially be std::laundered (depending on what members A has).


While A does have a trivial destructor, the more interesting case is probably where it has some side effect about which the compiler may want to make assumptions for optimization purposes. (I don't know whether any compiler uses something like this.) Therefore, I welcome answers for the case where A has a non-trivial destructor as well, especially if the answer differs between the two cases.

Also, from a practical perspective, a trivial destructor call probably does not affect the generated code and (unlikely?) optimizations based on undefined behavior assumptions aside, all code examples will most likely generate code that runs as expected on most compilers. I am more interested in the theoretical, rather than this practical perspective.


This question intends to get a better understanding of the details of the language. I do not encourage anyone to write code like that.

walnut
  • 21,629
  • 4
  • 23
  • 59
  • 13
    This is the "if the wording permitted this, it would be a defect in the wording" kind of question. – T.C. Sep 23 '19 at 16:02
  • 3
    *"The value category of `a->f` is prvalue ... only disallow non-static member function calls on glvalues"* I'm sure it talks about value category of the left operand of `->`. – HolyBlackCat Sep 23 '19 at 16:04
  • @HolyBlackCat I am not sure which standard reference you are referring to. It very likely that I misunderstood something in the standard language. If you could go into more detail, that would be much appreciated. – walnut Sep 23 '19 at 16:10
  • 4
    I expect UB. Imagine that `f` accesses some members. – Jarod42 Sep 23 '19 at 16:13
  • 3
    @uneven_mark http://www.eel.is/c++draft/basic.life#7.2 *"The program has undefined behavior if: ... the glvalue is used to call a non-static member function of the object"* - `ptr->member_func` is *always* a prvalue, so surely *"glvalue is used to call ... member function"* refers to `*ptr`, not `ptr->member_func`. – HolyBlackCat Sep 23 '19 at 16:17
  • 1
    @HolyBlackCat Ah yes, I guess that makes sense. Then I guess it applies here and does cause undefined behavior in all variants I mentioned. – walnut Sep 23 '19 at 16:28
  • @Jarod42 For that, [6.2](http://www.eel.is/c++draft/basic.life#6.2) applies. `UB if: the pointer is used to access a non-static data member or call a non-static member function of the object`. As the function would use `this` pointer. The call could still be valid. – Adam Sep 23 '19 at 16:30
  • That's like asking what f(x) = (x^2-1)/(x-1) will give for f(1) . Everyone knows the answer is really 2. For the same reason we can see that the compiler doesn't have to write any instructions for the destructor or A::f() in this case. The 'this' pointer will never be dereferenced anyway. – Arne J Sep 23 '19 at 16:48
  • Note that regardless, you have a memory leak since you do not call `delete` :-) – AndyG Sep 23 '19 at 16:52
  • @AndyG I am aware of the leak, I didn't think fixing it would add anything to the actual question. – walnut Sep 23 '19 at 16:53
  • @uneven_mark: you don't need to use `new` at all. `A a{}; a.f((a.~A(), 0));` is equivalent – AndyG Sep 23 '19 at 16:55
  • @AndyG True, but then I would not have been able to ask about replacing the destructor call with a delete expression that easily. – walnut Sep 23 '19 at 17:02
  • "_`a->f` is prvalue_" actually `a->f` isn't even a *thing*; it's a syntax artefact. **It doesn't exist at any semantic step.** – curiousguy Sep 24 '19 at 03:44
  • @Jarod42 Note that the *body* of `A::f(int)` doesn't access any member, and destruction inside a member function is and has always been legal under that condition that you don't try to access members later. (And even calling `delete this;`) – curiousguy Sep 24 '19 at 03:50
  • @AndyG "_is equivalent_" it's not strictly equivalent; what you wrote has a double destruction issue in addition to the member function call issue – curiousguy Sep 24 '19 at 04:10
  • As an analogy from what I'm seeing and understanding from your code snippet above would be as this: Let's add no gas to a gas tank that the cap was already put back on. It seems to be a valid statement as no gas would represent the 0 after the call to the destructor and nothing bad would seem to happen. However, let's say we change that 0 to a 1, where the 1 would represent calling or accessing a member and now we are pouring gas and spilling it all over the ground which leads to UB. I'm no language lawyer but in layman's terms this is what I'm seeing from the code snippet if this helps... – Francis Cugler Sep 24 '19 at 09:10
  • ... as per lifetime of the object, I can not say... It could depend on the specific compiler if, optimizations etc. – Francis Cugler Sep 24 '19 at 09:14
  • @FrancisCugler In C++ you can do ptr arithmetic on null ptr (op+ and both op-) but only when the integer is 0. In C you can't. Does that relate to cars? – curiousguy Sep 25 '19 at 00:37
  • @curiousguy I wasn't being exact with the analogy just using it to illustrate a point. Calling a destructor on an object then trying to access or calling its member is like trying to walk through a glass door; the door may not be visible at first but can lead to disasters later on. Now if there was some kind of internal memory in the class and the destructor was non trivial and had to do some work and one access a member after the destrcutor was invoked, you will definitely be breaking glass and getting cut... – Francis Cugler Sep 25 '19 at 03:07

5 Answers5

7

The postfix expression a->f is sequenced before the evaluation of any arguments (which are indeterminately sequenced relative to one another). (See [expr.call])

The evaluation of the arguments is sequenced before the body of the function (even inline functions, see [intro.execution])

The implication, then is that calling the function itself is not undefined behavior. However, accessing any member variables or calling other member functions within would be UB per [basic.life].

So the conclusion is that this specific instance is safe per the wording, but a dangerous technique in general.

AndyG
  • 39,700
  • 8
  • 109
  • 143
  • How do you think [basic.life]/7 plays into this? See my edit and the comments. – walnut Sep 23 '19 at 16:50
  • `basic.life` plays into this if you try to access a member within the function. – AndyG Sep 23 '19 at 16:50
  • Are you sure? Even thought `f` is `void A::f(int /*dummy*/) {/* no-op*/}` isn't `f` still UB (not at the callsite in this convoluted scenario, but within the member function) when called on a destructed `a`? – Eljay Sep 23 '19 at 17:27
  • Okay, thanks for checking. I wasn't sure that the code (despite being no-op) was UB or not, as per the standard. – Eljay Sep 23 '19 at 22:05
  • Well `delete this;` has always been well defined (under exactly the same assumptions as any `delete p;`), is often considered "dangerous", but has real world uses and is expected to be supported by all compilers. This silly game OTOH has no real use that I can see and probably isn't expected to be supported by any ppl. – curiousguy Sep 24 '19 at 03:54
7

It’s true that trivial destructors do nothing at all, not even end the lifetime of the object, prior to (the plans for) C++20. So the question is, er, trivial unless we suppose a non-trivial destructor or something stronger like delete.

In that case, C++17’s ordering doesn’t help: the call (not the class member access) uses a pointer to the object (to initialize this), in violation of the rules for out-of-lifetime pointers.

Side note: if just one order were undefined, so would be the “unspecified order” prior to C++17: if any of the possibilities for unspecified behavior are undefined behavior, the behavior is undefined. (How would you tell the well-defined option was chosen? The undefined one could emulate it and then release the nasal demons.)

Davis Herring
  • 36,443
  • 4
  • 48
  • 76
  • If the problem lies in the use of the pointer during initialization, then I assume `a->f((new(a) A, 0))` would be valid as long as the requirements of [\[basic.life\]/8](http://eel.is/c++draft/basic#life-8) are satisfied? – walnut Sep 27 '19 at 20:16
  • I thought that [basic.life]/6.2 does not apply, because I interpret the previous sentence *"Indirection through such a pointer is permitted but the resulting lvalue may only be used in limited ways, as described below"* to imply that the UB applies only if there is indirection. There is however no indirection for initialization of `this` and the indirection on `a` for `a->f` is executed separately sequenced before the call. – walnut Sep 27 '19 at 20:17
  • It does also seem that [basic.life]/6 would only apply before deallocation (i.e. not for `a->f((delete a, 0))`). Initialization of `this` would then presumably be implementation-defined as use of invalid pointer or is there some other reason for UB in that case? – walnut Sep 27 '19 at 20:17
  • @uneven_mark: Yes, I think the placement-new case works so long as `a` points (also) to the new object. The statement about indirection is referring to paragraph 7 (about lvalues). With `delete` the initialization would indeed have implementation-defined behavior, but that doesn’t matter because it’s undefined to **call** with it. – Davis Herring Sep 27 '19 at 20:53
2

You seem to assume that a->f(0) has these steps (in that order for most recent C++ standard, in some logical order for previous versions):

  • evaluating *a
  • evaluating a->f (a so called bound member function)
  • evaluating 0
  • calling the bound member function a->f on the argument list (0)

But a->f doesn't have either a value or type. It's essentially a non-thing, a meaningless syntax element needed only because the grammar decomposes member access and function call, even on a member function call which by define combines member access and function call.

So asking when a->f is "evaluated" is a meaningless question: there is no such thing as a distinct evaluation step for the a->f value-less, type-less expression.

So any reasoning based on such discussions of order of evaluation of non entity is also void and null.

EDIT:

Actually this is worse than what I wrote, the expression a->f has a phony "type":

E1.E2 is “function of parameter-type-list cv returning T”.

"function of parameter-type-list cv" isn't even something that would be a valid declarator outside a class: one cannot have f() const as a declarator as in a global declaration:

int ::f() const; // meaningless

And inside a class f() const doesn't mean "function of parameter-type-list=() with cv=const”, it means member-function (of parameter-type-list=() with cv=const). There is no proper declarator for proper "function of parameter-type-list cv". It can only exist inside a class; there is no type "function of parameter-type-list cv returning T" that can be declared or that real computable expressions can have.

curiousguy
  • 8,038
  • 2
  • 40
  • 58
  • Is that really true? [*The expression E1->E2 is converted to the equivalent form (*(E1)).E2*](http://www.eel.is/c++draft/expr#ref-2) and [*if E1.E2 refers to a non-static member function \[...\] then E1.E2 is a prvalue. \[...\] The type of E1.E2 is “function of parameter-type-list cv returning T”.*](http://www.eel.is/c++draft/expr#ref-6.3.2) The same paragraph forbids use of the expression except as the left-hand side of a call, but it does seem to assign it value category and type. Am I misunderstanding it? – walnut Sep 24 '19 at 04:32
  • @uneven_mark Edited answer to add that so-called "expression type" (lol) – curiousguy Sep 24 '19 at 04:46
  • 3
    Interestingly you can use the *function of parameter-type-list cv returning T* as type and to declare member functions: `typedef void F() const; struct A{ F f; }` declares a `const` member function, although you cannot use `F` to define the function. And `F` itself as type can be used e.g. in templates. But I see your point that actual expressions of this type essentially only make sense directly at the call. – walnut Sep 24 '19 at 07:14
  • @uneven_mark Never in many years of looking at fine details of the C++ had I given any thought to the nature of member function declarators and how you can use member attributes outside a class! The text of the std doesn't even discuss binding a member a member function to an object and later evaluating a function call operator on the *so called* prvalue. It's a little bit dirty and irregular. Note that floating points prvalues also are irregular as they may not be representable by an actual object so they cannot be in lvalue (f.ex. fused ops with intermediate values in infinite precision). – curiousguy Sep 25 '19 at 00:31
-1

In addition to what others said:

a->~A(); delete a;

This program has a memory leak which itself is technically not undefined behavior. However, if you called delete a; to prevent it - that should have been undefined behavior because delete would call a->~A() second time [Section 12.4/14].

a->~A()

Otherwise in reality this is as others suggested - compiler generates machine code along the lines of A* a = malloc(sizeof(A)); a->A(); a->~A(); a->f(0);. Since no member variables or virtuals all three member functions are empty ({return;}) and do nothing. Pointer a even still points to valid memory. It will run but debugger may complain of memory leak.

However, using any nonstatic member variables inside f() could have been undefined behavior because you are accessing them after they are (implicitly) destroyed by compiler-generated ~A(). That would likely result in a runtime error if it was something like std::string or std::vector.

delete a

If you replaced a->~A() with expression that invoked delete a; instead then I believe this would have been undefined behavior because pointer a is no longer valid at that point.

Despite that, the code should still run without errors because function f() is empty. If it accessed any member variables it may have crashed or led to random results because the memory for a is deallocated.

new(a) A

auto a = new A; new(a) A; is itself undefined behavior because you are calling A() a second time for the same memory.

In that case calling f() by itself would be valid because a exists but constructing a twice is UB.

It will run fine if A does not contain any objects with constructors allocating memory and such. Otherwise it could lead to memory leaks, etc, but f() would access the "second" copy of them just fine.

Jack White
  • 896
  • 5
  • 7
  • 1
    `new(a) A` is a placement-new and completely fine. Since the destructor is trivial, I don't need to call it beforehand either. I also think (but am not sure) that calling a trivial destructor multiple times is fine as well, see the edit to my question. – walnut Sep 24 '19 at 06:34
  • @uneven_mark Even w/ a non trivial dtor, you only need to call it if it does any useful work. – curiousguy Sep 25 '19 at 00:33
-3

I'm not a language lawyer but I took your code snippet and modified it slightly. I wouldn't use this in production code but this seems to produce valid defined results...

#include <iostream>
#include <exception>

struct A {
    int x{5};
    void f(int){}
    int g() { std::cout << x << '\n'; return x; }
};

int main() {
    try {
        auto a = new A;
        a->f((a->~A(), a->g()));
    catch(const std::exception& e) {
        std::cerr << e.what();
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

I'm running Visual Studio 2017 CE with compiler language flag set to /std:c++latest and my IDE's version is 15.9.16 and I get the follow console output and exit program status:

console output

5

IDE exit status output

The program '[4128] Test.exe' has exited with code 0 (0x0).

So this does seem to be defined in the case of Visual Studio, I'm not sure how other compilers will treat this. The destructor is being invoked, however the variable a is still in dynamic heap memory.


Let's try another slight modification:

#include <iostream>
#include <exception>

struct A {
    int x{5};
    void f(int){}
    int g(int y) { x+=y; std::cout << x << '\n'; return x; }
};

int main() {
    try {
        auto a = new A;
        a->f((a->~A(), a->g(3)));
    catch(const std::exception& e) {
        std::cerr << e.what();
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

console output

8

IDE exit status output

The program '[4128] Test.exe' has exited with code 0 (0x0).

This time let's not change the class anymore, but let's make call on a's member afterwards...

int main() {
    try {
        auto a = new A;
        a->f((a->~A(), a->g(3)));
        a->g(2);
    } catch( const std::exception& e ) {
        std::cerr << e.what();
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

console output

8
10

IDE exit status output

The program '[4128] Test.exe' has exited with code 0 (0x0).

Here it appears that a.x is maintaining its value after a->~A() is called since new was called on A and delete has not yet been called.


Even more if I remove the new and use a stack pointer instead of allocated dynamic heap memory:

int main() {
    try {
        A b;
        A* a = &b;    
        a->f((a->~A(), a->g(3)));
        a->g(2);
    } catch( const std::exception& e ) {
        std::cerr << e.what();
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

I'm still getting:

console output

8
10

IDE exit status output


When I change my compiler's language flag setting from /c:std:c++latest to /std:c++17 I'm getting the same exact results.

What I'm seeing from Visual Studio it appears to be well defined without producing any UB within the contexts of what I've shown. However as from a language perspective when it concerns the standard I wouldn't rely on this type of code either. The above also doesn't consider when the class has internal pointers both stack-automatic storage as well as dynamic-heap allocation and if the constructor calls new on those internal objects and the destructor calls delete on them.

There are also a bunch of other factors than just the language setting for the compiler such as optimizations, convention calling, and other various compiler flags. It is hard to say and I don't have an available copy of the full latest drafted standard to investigate this any deeper. Maybe this can help you, others who are able to answer your question more thoroughly, and other readers to visualize this type of behavior in action.

Francis Cugler
  • 7,788
  • 2
  • 28
  • 59
  • Thanks for your efforts, some comments will follow: – walnut Sep 24 '19 at 10:53
  • 2
    With regards to the standard, what you are testing is a slightly different question. In `(a->~A(), a->g())`, the second call is always sequenced after the first. Therefore, this expression itself causes undefined behavior if `a->~A()` ends the lifetime of the object. As I quoted in my question this probably depends on whether the destructor is trivial and the standard version. – walnut Sep 24 '19 at 10:54
  • 1
    It is also quite clear that my example and yours are likely to be compiled to something that works as expected, because the destructor doesn't actually do anything anyway. However the question of whether there is undefined behavior is more interesting with regards to the optimizations the compiler can make. Is it for example allowed to assume that a destructor with a global side effect has not been completed yet on `*this` when a member function is entered? I am not sure whether any compiler makes use of such optimization, but in principal they could if my example is undefined behavior. – walnut Sep 24 '19 at 10:59
  • @uneven_mark thank you for pointing that out; I corrected it! – Francis Cugler Sep 24 '19 at 11:28
  • @uneven_mark I tend to agree but I can not state that as 100% fact as I do not know. Yet I was trying to illustrate basic examples of the trivial destructor as being defined. Now if there are objects in the class that are created on the heap and the destructor actually does do work and those internal objects become deleted, then the `*this` pointer pointing to those objects I think would then become invalid and lead to UB. That's why I did state that I would not rely on this nor use it in production code. – Francis Cugler Sep 24 '19 at 11:34
  • @uneven_mark But I did like your question as it does have a high implication of needing to be answered with clarity. I figured I'd post this as a reference, reinforcement or confirmation to your original question. If you read my comment on your question about the analogy of the gas tank situation. I wasn't referring to changing the value 0 to a value of 1 or a basic internal member. It was meant to be in line with members that are pointers to other objects and objects that have memory allocation. – Francis Cugler Sep 24 '19 at 11:42