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

C++ interpreter cross-compilation from 64-bit host to 32-bit target broken #3653

Closed
tronical opened this issue Oct 11, 2023 · 2 comments · Fixed by #3810
Closed

C++ interpreter cross-compilation from 64-bit host to 32-bit target broken #3653

tronical opened this issue Oct 11, 2023 · 2 comments · Fixed by #3810
Labels
a:language-c++ C++ API, codegen, CMake build system (mS,mO)
Milestone

Comments

@tronical
Copy link
Member

Cross-compiling with cmake on 64-bit host to 32-bit target and enabling the interpreter breaks the build complaining that ValueOpaque is missing.

The generated slint_interpreter_internal.h is indeed missing the 32-bit variant:

#if defined(SLINT_TARGET_64)
struct ValueOpaque {
    uintptr_t _0[7];
};
#endif

In ffi.rs this is how it is declared:

#[repr(C)]
#[cfg(target_pointer_width = "64")]
pub struct ValueOpaque([usize; 7]);
#[repr(C)]
#[cfg(target_pointer_width = "32")]
#[repr(align(8))]
pub struct ValueOpaque([usize; 9]);
/// Asserts that ValueOpaque is as large as Value and has the same alignment, to make transmute safe.
const _: [(); std::mem::size_of::<ValueOpaque>()] = [(); std::mem::size_of::<Value>()];
const _: [(); std::mem::align_of::<ValueOpaque>()] = [(); std::mem::align_of::<Value>()];

cbindgen is configured to emit based on the target_pointer_width, which works for many types we emit. But somehow the #[repr(align(8))] breaks that. If I remove it (and comment out the assertion), then the generated header file is correct.

@tronical tronical added the a:language-c++ C++ API, codegen, CMake build system (mS,mO) label Oct 11, 2023
@tronical tronical added this to the 1.3 milestone Oct 11, 2023
@ogoffart
Copy link
Member

I'm thinking we should put C++ Value on the heap.
The current situation is hard to maintain and may even depend on the rust version or something.

@tronical
Copy link
Member Author

Sounds good to me. Thanks for the suggestion .

tronical added a commit that referenced this issue Oct 31, 2023
…3810)

cbindgen doesn't reliably tranfser the 64-bit/32-bit #ifdefs around ValueOpaque.
Instead, move Value onto the heap by using a Box.

Fixes #3653
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:language-c++ C++ API, codegen, CMake build system (mS,mO)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants