Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lambda ABI mismatch #141

Open
nandor opened this issue May 19, 2022 · 4 comments
Open

Lambda ABI mismatch #141

nandor opened this issue May 19, 2022 · 4 comments

Comments

@nandor
Copy link

nandor commented May 19, 2022

Compiler inlining of methods which define lambdas leads to issues. If the creation site is inlined and compiled with a separate compiler, the linker can still decide to replace the function with an implementation from another compiler capturing items in a different order. Since the creation site and the lambda implementation do not match, incorrect parameters are passed to the function.

Is this undefined behaviour or an issue with the ABI?

A minimal example to reproduce the error can be found here: https://github.com/nandor/lambda-abi-error

@RokerHRO
Copy link
Contributor

I wonder why you can give a lambda with non-empty catch clause to a std::function<void(void)>.

https://en.cppreference.com/w/cpp/language/lambda#ClosureType::operator.28.29.28params.29

This user-defined conversion function is only defined if the capture list of the lambda-expression is empty.

@aaronpuchert
Copy link

I wonder why you can give a lambda with non-empty catch clause to a std::function<void(void)>.

https://en.cppreference.com/w/cpp/language/lambda#ClosureType::operator.28.29.28params.29

This user-defined conversion function is only defined if the capture list of the lambda-expression is empty.

The user-defined conversion function to function pointer type shouldn't be used here, because std::function can directly be constructed from a Callable object, that's overload (5) here. The usual implementation is that it copies over the Callable object into dynamically allocated storage. See the note below:

When the target is a function pointer or a std::reference_wrapper, small object optimization is guaranteed, that is, these targets are always directly stored inside the std::function object, no dynamic allocation takes place. Other large objects may be constructed in dynamic allocated storage and accessed by the std::function object through a pointer.

@rjmccall
Copy link
Collaborator

rjmccall commented May 20, 2022

We've previously discussed this here. Our conclusion then appears to have been that we should force lambdas to have internal linkage. Clang seems to have never implemented that; in r151029, Doug Gregor gave lambdas external linkage in the situations where they have a stable mangling. It looks like GCC and ICC are doing the same. So effectively compilers are implementing option 1 from that mailing list post.

If compilers aren't going to give lambdas internal linkage in these situations, the ABI needs to define a layout for them. As far as I can tell, Clang, GCC, and ICC are all trying to use the same layout rule: explicit captures are added in declaration order, then implicit captures are added in the source order of their ODR use in the lambda body. The exception which causes this incompatibility is that GCC seems to order implicit this arguments (and only implicit ones) after the formal call arguments. I would argue that that's just a GCC bug, and the this argument should be ordered as if it were written in source. @jicama, do you agree?

nandor added a commit to llvm/circt that referenced this issue May 30, 2022
Code compiled with Clang cannot be linked with GCC due to
incompatibilities related to lambda closure capture lists.

This patch builds and caches LLVM with Clang to be used
with the Clang Release build and with GCC to be used with
the GCC Debug build of CIRCT.

The ABI issue and a potential fix is being tracked here:
itanium-cxx-abi/cxx-abi#141
nandor added a commit to llvm/circt that referenced this issue Jun 1, 2022
Code compiled with Clang cannot be linked with GCC due to
incompatibilities related to lambda closure capture lists.

This patch builds and caches LLVM with Clang to be used
with the Clang Release build and with GCC to be used with
the GCC Debug build of CIRCT.

The ABI issue and a potential fix is being tracked here:
itanium-cxx-abi/cxx-abi#141
nandor added a commit to llvm/circt that referenced this issue Jun 2, 2022
Code compiled with Clang cannot be linked with GCC due to incompatibilities related to lambda closure capture lists.

This patch builds and caches LLVM with Clang to be used with the Clang Release build and with GCC to be used with the GCC Debug build of CIRCT.

The ABI issue and a potential fix is being tracked here:
itanium-cxx-abi/cxx-abi#141
@joker-eph
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants