Skip to content

incorrect union passing on 32bit linux #14177

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.

Sign up for GitHub

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

Jump to bottom
Closed
metajack opened this issue May 13, 2014 · 2 comments · Fixed by #14191
Closed

incorrect union passing on 32bit linux #14177

metajack opened this issue May 13, 2014 · 2 comments · Fixed by #14191
Labels
A-codegen Area: Code generation A-FFI Area: Foreign function interface (FFI)

Comments

@metajack
Copy link
Contributor

Spidermonkey's jsval type is a union of size 64 bits. We represent this as struct JSVal { val: u64 } in Rust. This works great on 64bit platforms.

On 32bit platforms, the jsval values get mangled when passed to the C library functions. This seems to be an ABI issue. We've encountered this before with servo/servo#467 which was about 32bit ARM.

This gist contains sample programs and the generated IR for minimal test programs using rust-mozjs: https://gist.github.com/metajack/ddc94ef7e8e3ed838054

Note that you will need my 32bit branch of rust-mozjs: https://github.com/metajack/rust-mozjs/tree/build-32bit

You can put debugging printfs in jsapi.cpp in JS_SetReservedSlot to see the value on the C side.

The only notable thing I see in the IR is that the clang IR has byval when passing the jsval.

This seems like a Rust bug, but I'd also be interested if there is a workaround that will work on all platforms.
@metajack
Copy link
Contributor Author

cc @Ms2ger @jdm

@luqmana luqmana mentioned this issue May 14, 2014
Merged
bors added a commit that referenced this issue May 14, 2014
@bors
auto merge of #14191 : luqmana/rust/eu, r=alexcrichton
d78718a 
We were correctly determining the attributes needed for the parameters for extern fns, but when that extern fn was from another crate we never bothered to pass that information along to LLVM. (i.e never called `foreign::add_argument_attributes`).

I've just changed both local and non-local (crate) extern fn's to be dealt with together (through `foreign::register_foreign_item_fn`) so we don't run into something like again.

Fixes #14177.
@pnkfelix
Copy link
Member

cc me

lnicola pushed a commit to lnicola/rust that referenced this issue Jun 23, 2024
@bors
Auto merge of rust-lang#17419 - ishanjain28:filter_builtin_macro_expa…
e871775 
…nsion, r=Veykril

Filter builtin macro expansion

This PR adds a filter on the types of built in macros that are allowed to be expanded.

Currently, This list of allowed macros contains, `stringify, cfg, core_panic, std_panic, concat, concat_bytes, include, include_str, include_bytes, env` and `option_env`.

Fixes rust-lang#14177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-codegen Area: Code generation A-FFI Area: Foreign function interface (FFI)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

librustc: Make sure to add argument attributes to extern fns from non-local crates. luqmana/rust
2 participants
@metajack @pnkfelix