When creating a function pointer from a stateless generic lambda that accepts the argument of generic type by value, gcc 8+ on x86_64 generates wrong code, invoking a move constructor on a pointer-to-pointer to the moved-from object rather than just a pointer to it. This occurs regardless of optimizations as far as I can tell. $ cat t.cc extern "C" int printf(const char *fmt, ...); struct nontrivial { nontrivial() { printf("default-construct %p\n", this); } nontrivial(const nontrivial& other) { printf("copy-construct %p from %p\n", this, &other); } nontrivial(nontrivial&& other) noexcept { printf("move-construct %p from %p\n", this, &other); } ~nontrivial() { printf("destroy %p\n", this); } }; using cb_t = void(*)(nontrivial); cb_t test() { return [](auto val) { printf("called %p\n", &val); }; } int main() { volatile cb_t cb = test(); cb({}); return 0; } $ g++-7 -o t.ok t.cc -std=c++14 $ ./t.ok default-construct 0x7ffc6cf08b0f move-construct 0x7ffc6cf08adf from 0x7ffc6cf08b0f called 0x7ffc6cf08adf destroy 0x7ffc6cf08adf destroy 0x7ffc6cf08b0f $ g++-8 -o t.fail t.cc -std=c++14 $ ./t.fail default-construct 0x7fffd8a636ff move-construct 0x7fffd8a636c7 from 0x7fffd8a636c8 called 0x7fffd8a636c7 destroy 0x7fffd8a636c7 destroy 0x7fffd8a636ff In the second example, the object is move-constructed from an address at which no object has been constructed. Examining in a debugger, 0x7fffd8a636c8 contains the address 0x7fffd8a636ff, i.e., there is one level of indirection too many. Tested with godbolt.org, bug is absent on 7.1.0, 7.2.0, 7.3.0, and present on 8.1.0, 8.2.0, and 9.0.0 20180812.
Confirmed, started with r239268.
extern "C" int printf (const char *fmt, ...); struct S { S () { printf ("default-construct %p\n", this); } S (const S &x) { printf ("copy-construct %p from %p\n", this, &x); } S (S &&x) noexcept { printf ("move-construct %p from %p\n", this, &x); } ~S () { printf ("destroy %p\n", this); } }; using F = void (*) (S); F foo () { return [] (auto val) { printf ("called %p\n", &val); }; } int main () { volatile F cb = foo (); cb ({}); return 0; } The extra indirection is because in _FUN we have: <<cleanup_point <<< Unknown tree: expr_stmt foo()::<lambda(auto:1)>::operator()<S> (0B, &TARGET_EXPR <D.2164, <<< Unknown tree: aggr_init_expr 5 __ct_comp D.2164 (struct S *) <<< Unknown tree: void_cst >>> (struct S &) &D.2160 >>>>) >>>>>; return; and CALL_FROM_THUNK_P is set on the operator() call. cp_genericize on _FUN changes the D.2160 argument, originally with type S, is changed to S & and the PARM_DECL turned into DECL_BY_REFERENCE one, but because the call is CALL_FROM_THUNK_P, no adjustment of the arguments is done: /* Don't dereference parms in a thunk, pass the references through. */ if ((TREE_CODE (stmt) == CALL_EXPR && CALL_FROM_THUNK_P (stmt)) || (TREE_CODE (stmt) == AGGR_INIT_EXPR && AGGR_INIT_FROM_THUNK_P (stmt))) { *walk_subtrees = 0; return NULL; } Do we want to just prevent changing the direct PARM_DECL arguments of such calls, but still recurse into more complex arguments? Or is it incorrect that CALL_FROM_THUNK_P is set here?
And, note that clang 5/6 don't call the move ctor at all and only one dtor. What is correct?
I haven't analyzed the code to say what it should do, but EDG agrees with Clang.
I see the wrong behaviour for GCC 7.3.1 so it seems to have regressed since 7.3.0 I think GCC is allowed to use a default constructor then move constructor in C++14 mode, although ideally it would elide the move. In C++17 mode I think it's required to elide it. So Clang and EDG's behaviour is preferable for C++14, and required for C++17.
I might be wrong about the C++14 rules ... maybe the {} initializer should directly initialize the parameter, without any move. I don't know if we need to fix the incorrect argument to the move constructor as well as prevent that move, so the incorrect move doesn't remain latent.
--- gcc/cp/cp-gimplify.c.jj 2018-11-17 00:16:41.924381941 +0100 +++ gcc/cp/cp-gimplify.c 2018-11-23 17:29:24.764459373 +0100 @@ -1108,6 +1108,18 @@ cp_genericize_r (tree *stmt_p, int *walk || (TREE_CODE (stmt) == AGGR_INIT_EXPR && AGGR_INIT_FROM_THUNK_P (stmt))) { *walk_subtrees = 0; + /* For CALL_EXPRs, if the arguments aren't decls, recurse into them + though. See PR86943. */ + if (TREE_CODE (stmt) == CALL_EXPR) + { + unsigned int n = call_expr_nargs (stmt); + for (unsigned int i = 0; i < n; i++) + { + tree &arg = CALL_EXPR_ARG (stmt, i); + if (!DECL_P (arg)) + cp_walk_tree (&arg, cp_genericize_r, data, NULL); + } + } return NULL; } seems to fix the double indirection bug (and in make check-c++-all doesn't seem to regress anything else). That said, the move constructor is still called even with -std=c++17 or -std=c++2a.
The non-normative note in [expr.prim.lambda.closure] p8 does seem to support GCC's semantics. It suggests the returned function pointer is to an invoker function not the operator() itself. The {} initializes the argument of the invoker, which then forwards it to the operator() resulting in a move construction.
See the glambda example in http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3649.html Core 1937 seems relevant here.
pt.c calls here: ret = (build_new_method_call (instance, fn, &call_args, NULL_TREE, qualified_p ? LOOKUP_NONVIRTUAL : LOOKUP_NORMAL, /*fn_p=*/NULL, complain)); and doesn't in any way tell it that CALL_FROM_THUNK_P (t) is true and that it should avoid some or all? argument conversions. Or shall it call a different function in that case instead? In any case, not working on this PR anymore.
I'll just include the testcase for the testsuite from the above patch: // PR c++/86943 // { dg-do run { target c++14 } } int c[3]; struct S { S () : s (1234) { c[0]++; } S (const S &) { __builtin_abort (); } S (S &&x) noexcept { if (x.s != 1234) __builtin_abort (); s = 1234; x.s = 2345; c[1]++; } ~S () { if (s != 1234 && s != 2345) __builtin_abort (); c[2]++; } int s; }; using F = void (*) (S); F foo () { return [] (auto val) { if (val.s != 1234) __builtin_abort (); }; } int main () { { volatile F f = foo (); f ({}); } if (c[0] + c[1] != c[2]) __builtin_abort (); }
Author: jason Date: Wed Jan 30 02:43:04 2019 New Revision: 268377 URL: https://gcc.gnu.org/viewcvs?rev=268377&root=gcc&view=rev Log: PR c++/86943 - wrong code converting lambda to function pointer. In this PR, instantiating the static thunk returned from the generic lambda conversion function template was using normal overload resolution, which meant calling an extra constructor when forwarding its argument. Fixed by special-casing thunk calls significantly more. * lambda.c (maybe_add_lambda_conv_op): Use a template-id in the call. Only forward parms for decltype. * pt.c (tsubst_copy_and_build) [CALL_EXPR]: Handle CALL_FROM_THUNK_P specially. * typeck.c (check_return_expr): Don't mess with a thunk call. Added: trunk/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-conv13.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/lambda.c trunk/gcc/cp/pt.c trunk/gcc/cp/typeck.c
Fixed on trunk so far.
Author: jason Date: Fri Feb 8 14:49:43 2019 New Revision: 268700 URL: https://gcc.gnu.org/viewcvs?rev=268700&root=gcc&view=rev Log: PR c++/86943 - wrong code converting generic lambda to pointer. * pt.c (tsubst_copy_and_build): Don't propagate CALL_FROM_THUNK_P. Added: branches/gcc-8-branch/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-conv13.C Modified: branches/gcc-8-branch/gcc/cp/ChangeLog branches/gcc-8-branch/gcc/cp/pt.c
Fixed for 8.3 too.
GCC 8.3 has been released.
GCC 8.4.0 has been released, adjusting target milestone.
Fixed in all active releases.