Skip to content

extern "C" fn ABI on aarch64 violates repr(transparent) #115664

New issue

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

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

Already on GitHub? # to your account

Closed
RalfJung opened this issue Sep 8, 2023 · 3 comments · Fixed by #115708
Closed

extern "C" fn ABI on aarch64 violates repr(transparent) #115664

RalfJung opened this issue Sep 8, 2023 · 3 comments · Fixed by #115708
Labels
A-ABI Area: Concerning the application binary interface (ABI) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-AArch64 Armv8-A or later processors in AArch64 mode

Comments

@RalfJung
Copy link
Member

RalfJung commented Sep 8, 2023

On aarch64-unknown-linux-gnu (a tier 1 target), consider this testcase:

#![feature(rustc_attrs)]

type T = (f32, f32, f32);

#[repr(transparent)]
struct Wrapper2<T>((), T);

#[rustc_abi(debug)]
extern "C" fn test1(_x: T) {}

#[rustc_abi(debug)]
extern "C" fn test2(_x: Wrapper2<T>) {}

fn main() {}

test1 passes its argument as

                   mode: Cast(
                       CastTarget {
                           prefix: [
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                           ],
                           rest: Uniform {
                               unit: Reg {
                                   kind: Float,
                                   size: Size(4 bytes),
                               },
                               total: Size(12 bytes),
                           },
                           attrs: ArgAttributes {
                               regular: (empty),
                               arg_ext: None,
                               pointee_size: Size(0 bytes),
                               pointee_align: None,
                           },
                       },
                       false,
                   ),

test2 uses

                   mode: Cast(
                       CastTarget {
                           prefix: [
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                               None,
                           ],
                           rest: Uniform {
                               unit: Reg {
                                   kind: Integer,
                                   size: Size(8 bytes),
                               },
                               total: Size(12 bytes),
                           },
                           attrs: ArgAttributes {
                               regular: (empty),
                               arg_ext: None,
                               pointee_size: Size(0 bytes),
                               pointee_align: None,
                           },
                       },
                       false,
                   ),

IOW, test1 uses float registers and test2 uses int registers. This violates our promise that repr(transparent) types are ABI-compatible.

I think this might indicate a bug in the homogeneous_aggregate function? At least I don't see anything in the aarch64 adjustment that would be obviously wrong.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 8, 2023
@RalfJung RalfJung added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-AArch64 Armv8-A or later processors in AArch64 mode A-ABI Area: Concerning the application binary interface (ABI) labels Sep 8, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 8, 2023
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 8, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Sep 8, 2023

This also affects arm32 targets like arm-unknown-linux-gnueabihf.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 9, 2023

I think this might indicate a bug in the homogeneous_aggregate function?

Yes, that is definitely the issue -- it claims that Wrapper2<T> is heterogeneous.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 9, 2023

Should be fixed by #115708.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 11, 2023
fix homogeneous_aggregate not ignoring some ZST

This is an ABI-breaking change, because it fixes bugs in our ABI code. I'm not sure what that means for this PR, we don't really have a process for such changes, do we? I can only hope nobody relied on the old buggy behavior.

Fixes rust-lang#115664
@bors bors closed this as completed in 279e257 Sep 11, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 11, 2023
Rollup merge of rust-lang#115708 - RalfJung:homogeneous, r=davidtwco

fix homogeneous_aggregate not ignoring some ZST

This is an ABI-breaking change, because it fixes bugs in our ABI code. I'm not sure what that means for this PR, we don't really have a process for such changes, do we? I can only hope nobody relied on the old buggy behavior.

Fixes rust-lang#115664
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-AArch64 Armv8-A or later processors in AArch64 mode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants