CODEWITHSUNDEEP
c++gcclambdac++17gcc8
GPMueller
GPMueller

Reputation: 3149

Segfault when using std::function class member with compiler optimizations

I have a quite simple code example which crashes when optimized with -O2 under gcc 8.2.0

#include <vector>
#include <functional>
#include <iostream>

template<typename T, typename Container>
class Lambda_Expression
{
    using Lambda = std::function<T()>;
    const Lambda & _lambda;

public:
    Lambda_Expression(const Lambda & l) : _lambda(l) {}

    T operator[](const std::size_t i)
    {
        std::cerr << "inside expression [] " << i << std::endl;
        return _lambda();
    }
};

auto lambda = []() -> double
{
    return 1.0;
};

int main()
{
    int N = 10;
    std::vector<double> res(N, 0.0);

    double x = lambda();

    std::cerr << "before for loop " << x << std::endl;

    auto test_expression = Lambda_Expression<double, std::vector<double>>(lambda);

    for( int idx=0; idx<N; ++idx )
    {
        std::cerr << "loop " << idx << std::endl;
        double x = test_expression[idx];
    }
}

Using also -std=c++17, in case that makes a difference.

I get

before for loop 1
loop 0
inside expression [] 0
[1]    5288 segmentation fault  ./bench_lambdas

whereas I would expect the loop to run for 10 iterations. This segfault does not appear with optimization level less than 2.

The above example looks like fairly harmless code to me and as far as I know level 2 optimizations should not break correct code.

Question: Is there undefined behaviour or incorrect code in my example or what might be the issue?

Upvotes: 3

Views: 503

Answers (1)

max66
max66

Reputation: 66240

As far I know, it's undefined behaviour.

The problem is that your class register the reference

// ..........V  reference !!!
const Lambda & _lambda;

of the argument of the constructor

Lambda_Expression(const Lambda & l) : _lambda(l) {}

that is a std::function

using Lambda = std::function<T()>;

But when you call the constructor with a lambda (as in main())

auto test_expression = Lambda_Expression<double, std::vector<double>>(lambda);

you save in _lambda the reference to a temporary object because lambda isn't a std::function so it's created a temporary object, of type std::function<double()>, initialized with lambda.

So the problem: the reference to the temporary object become a dangling reference at the end of the construction of test_expression so, when you call test_expression[idx], you use _lambda that is pointing (potentially) to garbage.

I suggest to avoid this sort of problems avoiding the reference part (make _lambda a regular member of type std::function

const Lambda _lambda;  // <-- no more reference

so you copy the temporary object)

But if you really want that _lambda is a reference to a std::function, you should write something as follows

std::function<double()>  f{lambda};

auto test_expression = Lambda_Expression<double, std::vector<double>>{f};

This way the constructor receive a reference to a std::function object (f) that survive to his call.

Upvotes: 6

Related Questions