Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ lambda not capturing variable on 2nd expansion in template?

I have some tortuous code in a template that uses @R. Martinho Fernandes's trick to loop unroll some packed parameters in a variadic template and invoke the same code on each argument in the argument list.

However, it seems as though the lambdas are not being initialized properly and they are instead sharing variables across functor(?) instances, which seems wrong.

Given this code:

#include <iostream>
#include <functional>

template<typename... Args>
void foo(Args ... args) {
  int * bar = new int();
  *bar = 42;

  using expand_type = int[];
  expand_type{(
    args([bar]() {
        std::cerr<<std::hex;
        std::cerr<<"&bar="<<(void*)&bar<<std::endl;
        std::cerr<<"  bar="<<(void*)bar<<std::endl;
        std::cerr<<"  bar="<<*bar<<std::endl<<std::endl;
    }),
    0) ... 
  };
};

int main() {
  std::function<void(std::function<void()>)> clbk_func_invoker = [](std::function<void()> f) { f(); };
  foo(clbk_func_invoker, clbk_func_invoker);

  return 0;
}

I get the following output:

&bar=0x7ffd22a2b5b0
  bar=0x971c20
  bar=2a

&bar=0x7ffd22a2b5b0
  bar=0
Segmentation fault (core dumped)

So, what I believe I'm seeing is that the two functor instances share the same address for captured variable bar, and after the invocation of the first functor, bar is being set to nullptr, and then the second functor seg'-faults when it tries to dereference the same bar variable ( in the exact same address ).

FYI, I realize that I can work around this issue by moving the [bar](){... functor into a variable std::function variable and then capturing that variable. However, I would like to understand why the second functor instance is using the exact same bar address and why it is getting a nullptr value.

I ran this with GNU's g++ against their trunk version retrieved and compiled yesterday.

like image 649
Ross Rogers Avatar asked Aug 24 '16 16:08

Ross Rogers


2 Answers

Parameter packs with lambdas in them tend to give compilers fits. One way to avoid that is to move the expansion part and the lambda part separate.

template<class F, class...Args>
auto for_each_arg( F&& f ) {
  return [f=std::forward<F>(f)](auto&&...args){
    using expand_type = int[];
    (void)expand_type{0,(void(
      f(decltype(args)(args))
    ),0)...};
  };
}

This takes a lambda f and returns an object that will invoke f on each of its arguments.

We can then rewrite foo to use it:

template<typename... Args>
void foo(Args ... args) {
  int * bar = new int();
  *bar = 42;

  for_each_arg( [bar](auto&& f){
    f( [bar]() {
      std::cerr<<std::hex;
      std::cerr<<"&bar="<<(void*)&bar<<std::endl;
      std::cerr<<"  bar="<<(void*)bar<<std::endl;
      std::cerr<<"  bar="<<*bar<<std::endl<<std::endl;
    } );
  } )
  ( std::forward<Args>(args)... );
}

live example.

I initially thought it had to do with the std::function constructor. It does not. A simpler example without a std::function that crashes the same way:

template<std::size_t...Is>
void foo(std::index_sequence<Is...>) {
  int * bar = new int();
  *bar = 42;

  using expand_type = int[];
  expand_type{(
    ([bar]() {
      std::cerr<<"bar="<<*bar<<'\n';
    })(),
    (int)Is) ... 
  };
}

int main() {
  foo(std::make_index_sequence<2>{});

  return 0;
}

we can invoke the segfault without the cerr, giving us disassembly that is easier to read:

void foo<3, 0ul, 1ul>(std::integer_sequence<unsigned long, 0ul, 1ul>)::{lambda()#1}::operator()() const:
    pushq   %rbp
    movq    %rsp, %rbp
    movq    %rdi, -8(%rbp)
    movq    -8(%rbp), %rax
    movq    (%rax), %rax
    movl    $3, (%rax)
    nop
    popq    %rbp
    ret
void foo<3, 0ul, 1ul>(std::integer_sequence<unsigned long, 0ul, 1ul>):
    pushq   %rbp
    movq    %rsp, %rbp
    pushq   %rbx
    subq    $40, %rsp
    movl    $4, %edi
    call    operator new(unsigned long)
    movl    $0, (%rax)
    movq    %rax, -24(%rbp)
    movq    -24(%rbp), %rax
    movl    $42, (%rax)
    movq    -24(%rbp), %rax
    movq    %rax, -48(%rbp)
    leaq    -48(%rbp), %rax
    movq    %rax, %rdi
    call    void foo<3, 0ul, 1ul>(std::integer_sequence<unsigned long, 0ul, 1ul>)::{lambda()#1}::operator()() const
    movabsq $-4294967296, %rax
    andq    %rbx, %rax
    movq    %rax, %rbx
    movq    $0, -32(%rbp)
    leaq    -32(%rbp), %rax
    movq    %rax, %rdi
    call    void foo<3, 0ul, 1ul>(std::integer_sequence<unsigned long, 0ul, 1ul>)::{lambda()#1}::operator()() const
    movl    %ebx, %edx
    movabsq $4294967296, %rax
    orq     %rdx, %rax
    movq    %rax, %rbx
    nop
    addq    $40, %rsp
    popq    %rbx
    popq    %rbp
    ret

I have yet to parse the disassembly, but it obviously trashes the state of the second lambda when playing with the first.

like image 82
Yakk - Adam Nevraumont Avatar answered Oct 10 '22 11:10

Yakk - Adam Nevraumont


First of all I don't have a solution, I'd want to add this extra information as a comment, but unfortunately I can't comment yet.

I tried your previous code with Intel 17 c++ compiler and worked fine:

&bar=0x7fff29e40c50
  bar=0x616c20
  bar=2a

&bar=0x7fff29e40c50
  bar=0x616c20
  bar=2a

In some cases the &bar (the address of the new variable used to store the captured value) was different between the first call and the second, but it also worked.

I also tried your code with GNU's g++ changing the type of bar from int* to int. The captured value was wrong in the second and successive calls even in this case:

&bar=0x7fffeae12480
  bar=2a
&bar=0x7fffeae12480
  bar=0
&bar=0x7fffeae12480
  bar=0

Finally I tried modifying a bit the code and passing by value and object, so the copy constructor must be called:

#include <iostream>
#include <functional>

struct  A {
    A(int x) : _x(x) { 
      std::cerr << "Constructor!" << n++ << std::endl;
    }
    A(const A& a) : _x(a._x) {
      std::cerr << "Copy Constructor!"  << n++ << std::endl;
    }
    static int n;
    int _x;  
};

int A::n = 0;

template<typename... Args>
void foo(Args ... args) {
  A a(42);

  std::cerr << "-------------------------------------------------" << std::endl;
  using expand_type = int[];
  expand_type  {
   (args( [a]() {
          std::cerr << "&a, "<< &a << ", a._x," << a._x << std::endl;
         }
       ),
    0) ... 
  };
std::cerr << "-------------------------------------------------" << std::endl;
}

int main() {
  std::function<void(std::function<void()>)> clbk_func_invoker = [](std::function<void()> f) { f(); };
  foo(clbk_func_invoker, clbk_func_invoker, clbk_func_invoker);
  return 0;
}

My current version of g++ (g++ (GCC) 6.1.0) is not able to compile this code. I also tried with Intel and it worked, although I don't fully understand why the copy constructor is called so many times:

Constructor!0
-------------------------------------------------
Copy Constructor!1
Copy Constructor!2
Copy Constructor!3
&a, 0x617c20, a._x,42
Copy Constructor!4
Copy Constructor!5
Copy Constructor!6
&a, 0x617c20, a._x,42
Copy Constructor!7
Copy Constructor!8
Copy Constructor!9
&a, 0x617c20, a._x,42
-------------------------------------------------

That's all I tested so far.

like image 28
smateo Avatar answered Oct 10 '22 09:10

smateo