Deleting a std::function object within itself

Question

Is this well defined behavior?

#include <functional>

void foo() {
    auto f = new std::function<void()>;
    *f = [f]() { delete f; };
    (*f)();
    f = nullptr;
}

int main() {
    foo();
}

Using the most recent g++, if I do this within a template it causes invalid reads while running under valgrind, otherwise it works fine. Why? Is this a bug in g++?

#include <functional>

template<std::size_t>
void foo() {
    auto f = new std::function<void()>;
    *f = [f]() { delete f; };
    (*f)();
    f = nullptr;
}

int main() {
    foo<0>();
}

Answer 1

This program has well-defined behavior and demonstrates a g++ bug.

The only questionable part of runtime is during the statement (*f)();. The behavior of that line can be picked apart piece by piece. The Standard section numbers below are from N3485; apologies if some don't match C++11.

*f is just the built-in unary operator on a raw pointer to class type. No problem here. The only other evaluation is the function-call expression (*f)(), which invokes void std::function<void()>::operator() const. Then that full-expression is a discarded value.

20.8.11.2.4:

R operator()(ArgTypes... args) const

Effects: INVOKE(obj, std::forward<ArgTypes>(args)..., R) where obj is the target object of *this.

(I've replaced "f" in the Standard with "obj" to reduce confusion with main's f.)

Here obj is a copy of the lambda object, ArgTypes is the empty parameter pack from the specialization std::function<void()>, and R is void.

The INVOKE pseudo-macro is defined in 20.8.2. Since the type of obj is not a pointer-to-member, INVOKE(obj, void) is defined to be obj() implicitly converted to void.

5.1.2p5:

The closure type for a lambda-expression has a public inline function call operator ...

... with exactly described declaration. In this case it turns out to be void operator() const. And its definition is exactly described too:

5.1.2p7:

The lambda-expression's compound-statement yields the function-body of the function call operator, but for purposes of name lookup, determining the type and value of this and transforming id-expressions referring to non-static class members into class member access expressions using (*this), the compound-statement is considered in the context of the lambda-expression.

5.1.2p14:

For each entity captured by copy, an unnamed non-static data member is declared in the closure type.

5.1.2p17:

Every id-expression that is an odr-use of an entity captured by copy is transformed into an access to the corresponding unnamed data member of the closure type.

So the lambda function call operator must be equivalent to:

void __lambda_type::operator() const {
    delete __unnamed_member_f;
}

(where I've invented some names for the unnamed lambda type and unnamed data member.)

The single statement of that call operator is of course equivalent to delete (*this).__unnamed_member_f; So we have:

• The built-in unary operator* dereference (on the prvalue this)
• A member access expression
• A value computation (aka lvalue-to-rvalue conversion) for the member subobject
• A scalar delete expression
  • Invokes std::function<void()>::~function()
  • Invokes void operator delete(void*)

And finally, in 5.3.5p4:

The cast-expression in a delete-expression shall be evaluated exactly once.

(Here is where g++ is wrong, doing a second value computation on the member subobject between the destructor call and the deallocation function.)

This code cannot cause any other value computations or side effects after the delete expression.

There are some allowances for implementation-defined behavior in lambda types and lambda objects, but none that affect anything above:

5.1.2p3:

An implementation may define the closure type differently from what is described below provided this does not alter the observable behavior of the program other than by changing:

• the size and/or alignment of the closure type,

• whether the closure type is trivially copyable,

• whether the closure type is a standard-layout class, or

• whether the closure type is a POD class.

Answer 2

It is certainly not well-defined behaviour in general.

Between the end of execution of function object, and the end of the call to operator(), the member operator() is executing on a deleted object. If the implementation reads or writes through this, which it is perfectly allowed to do, then you'll get read or write of a deleted object.

More specifically, the object was only just deleted by this very thread, so it's highly unlikely that any thread actually got around to using it between deletion and the read/write or it was unmapped, so it's quite unlikely to actually cause problems in a simple program. In addition, there's little apparent reason for the implementation to read or write to this after it returns.

However, Valgrind is quite correct that any such read or write would be very invalid and in some circumstances, could lead to random crashes or memory corruption. It's easy to suggest that, between deleting this and a hypothetical read/write, this thread was pre-empted and another thread allocated and used that memory. Alternatively, the memory allocator decided it had enough cached memory of this size and returned this segment to the OS immediately upon freeing it. This is an excellent candidate for a Heisenbug since the conditions to cause it would be relatively rare and only apparent in real complex executing systems rather than trivial test programs.

You could get away with it if you can prove that there are no reads or writes after the function object finishes returning. This basically means guaranteeing the implementation of std::function<Sig>::operator().

Edit:

Mats Peterson's answer raises an interesting question. GCC appears to have implemented the lambda by doing something like this:

struct lambda { std::function<void()>* f; };
void lambda_operator(lambda* l) {
    l->f->~std::function<void()>();
    ::operator delete(l->f);
}

As you can see, the call to operator delete makes a load from l after it's just been deleted, which is exactly the scenario I described above. I'm not actually sure what C++11's memory model rules say about this, I would have thought it was illegal but not necessarily. It might not be defined either way. If it is not illegal, you are definitely screwed.

Clang, however, seems to generate this operator:

void lambda_operator(lambda* l) {
    auto f = l->f;
    f->~std::function<void()>();
    ::operator delete(f);
}

Here when l is deleted it doesn't matter because f was copied into local storage.

To a certain extent, this definitively answers your question- GCC absolutely does load from the memory of the lambda after it's been deleted. Whether or not that's Standard-legal or not, I'm not sure. You could definitely work around this by employing a user-defined function. You'd still have the problem of the std::function implementation issuing loads or stores to this, though.

Answer 3

The problem is not related to lambdas or std::function, but rather the semantics of delete. This code exhibits the same problem:

class A;

class B {
    public:
        B(A *a_) : a(a_) {}
        void foo();
    private:
        A *const a;
};

class A {
    public:
        A() : b(new B(this)) {}
        ~A() {
            delete b;
        }
        void foo() { b->foo(); }
    private:
        B *const b;
};

void B::foo() {
    delete a;
}

int main() {
    A *ap = new A;
    ap->foo();
}

The issue is the semantics of delete. Is it allowed to load the operand again from memory after calling its destructor, in order to free its memory?

Answer 4

See http://cplusplus.github.io/LWG/lwg-active.html#2224.

Accessing a library type after its destructor has started is undefined behavior. Lambdas are not library types, so they don't have such a limitation. Once a destructor of a library type has been entered, the invariants of that library type no longer hold. The language doesn't enforce such a limitation because invariants are by and large a library concept, not a language concept.

Answer 5

It will probably not crash in the a general case, but WHY on earth would you ever want to do something like that in the first place.

But here's my analysis:

valgrind produces:

==7323==    at 0x4008B5: _ZZ3fooILm0EEvvENKUlvE_clEv (in /home/MatsP/src/junk/a.out)
==7323==    by 0x400B4A: _ZNSt17_Function_handlerIFvvEZ3fooILm0EEvvEUlvE_E9_M_invokeERKSt9_Any_data (in /home/MatsP/src/junk/a.out)
==7323==    by 0x4009DB: std::function<void ()()>::operator()() const (in /home/MatsP/src/junk/a.out)
==7323==    by 0x40090A: void foo<0ul>() (in /home/MatsP/src/junk/a.out)
==7323==    by 0x4007E8: main (in /home/MatsP/src/junk/a.out)

This points at the code here (which is indeed the lambda function in your original code):

000000000040088a <_ZZ3fooILm0EEvvENKUlvE_clEv>:
  40088a:   55                      push   %rbp
  40088b:   48 89 e5                mov    %rsp,%rbp
  40088e:   48 83 ec 10             sub    $0x10,%rsp
  400892:   48 89 7d f8             mov    %rdi,-0x8(%rbp)
  400896:   48 8b 45 f8             mov    -0x8(%rbp),%rax
  40089a:   48 8b 00                mov    (%rax),%rax
  40089d:   48 85 c0                test   %rax,%rax   ;; Null check - don't delete if null. 
  4008a0:   74 1e                   je     4008c0 <_ZZ3fooILm0EEvvENKUlvE_clEv+0x36>
  4008a2:   48 8b 45 f8             mov    -0x8(%rbp),%rax
  4008a6:   48 8b 00                mov    (%rax),%rax
  4008a9:   48 89 c7                mov    %rax,%rdi
;; Call function destructor
  4008ac:   e8 bf ff ff ff          callq  400870 <_ZNSt8functionIFvvEED1Ev>  
  4008b1:   48 8b 45 f8             mov    -0x8(%rbp),%rax
  4008b5:   48 8b 00                mov    (%rax),%rax           ;; invalid access
  4008b8:   48 89 c7                mov    %rax,%rdi
;; Call delete. 
  4008bb:   e8 b0 fd ff ff          callq  400670 <_ZdlPv@plt>   ;; delete
  4008c0:   c9                      leaveq 
  4008c1:   c3                      retq   

Interestingly, it "works" using clang++ (version 3.5, built from git sha1 d73449481daee33615d907608a3a08548ce2ba65, from March 31st):

0000000000401050 <_ZZ3fooILm0EEvvENKUlvE_clEv>:
  401050:   55                      push   %rbp
  401051:   48 89 e5                mov    %rsp,%rbp
  401054:   48 83 ec 10             sub    $0x10,%rsp
  401058:   48 89 7d f8             mov    %rdi,-0x8(%rbp)
  40105c:   48 8b 7d f8             mov    -0x8(%rbp),%rdi
  401060:   48 8b 3f                mov    (%rdi),%rdi
  401063:   48 81 ff 00 00 00 00    cmp    $0x0,%rdi   ;; Null check. 
  40106a:   48 89 7d f0             mov    %rdi,-0x10(%rbp)
  40106e:   0f 84 12 00 00 00       je     401086 <_ZZ3fooILm0EEvvENKUlvE_clEv+0x36> 
  401074:   48 8b 7d f0             mov    -0x10(%rbp),%rdi
  401078:   e8 d3 fa ff ff          callq  400b50 <_ZNSt8functionIFvvEED2Ev>   
;; Function destructor 
  40107d:   48 8b 7d f0             mov    -0x10(%rbp),%rdi
  401081:   e8 7a f6 ff ff          callq  400700 <_ZdlPv@plt>    ;; delete. 
  401086:   48 83 c4 10             add    $0x10,%rsp
  40108a:   5d                      pop    %rbp
  40108b:   c3                      retq   

Edit: It doesn't really make any sense - I don't see why there is a memory access to the first element inside the function class in gcc's code and not in clang's - they are supposed to do the same thing...

Answer 6

The allocation auto f = new std::function<void()>; of course is alright. The definition of the lambda *f = [f]() { delete f; }; works as well, as it is not executed yet.

Now the interesting thing is (*f)();. First it dereferences f, then calls operator() and finally executes delete f. Calling delete f in the class member function function<>::operator() is the same as calling delete this. Under certain cirqumstances this is legal.

So it depends on how operator() for std::function and lamdabs is implemented. Your code would be valid if it is guaranteed that no member function, member variable or the this pointer itself are used or even touched by operator() after executing your encapsulated lambda.

I would say there is no need for std::function to call other member functions or use member variables in operator() after executing your lambda. So you will probably find implementations for which your code is legal, but in general it is probably not safe to assume so.