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

arm-none-eabi (etc) have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) #87917

Closed
Manishearth opened this issue Aug 10, 2021 · 6 comments · Fixed by #87922
Closed

arm-none-eabi (etc) have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) #87917

Manishearth opened this issue Aug 10, 2021 · 6 comments · Fixed by #87922
Labels
A-layout Area: Memory layout of types C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state

Comments

@Manishearth
Copy link
Member

Manishearth commented Aug 10, 2021

We're building code for a Cortex-M33 running FreeRTOS, and using C/Rust FFI.

The C code is being compiled with:

compiler is arm-none-eabi-g++ (GNU Arm Embedded Toolchain 10-2020-q4-major) 10.2.1 20201103 (release)
target flags -mcpu=cortex-m33 -mthumb -mfloat-abi=hard -mfpu=fpv5-sp-d16

The rust code is being compiled with:

RUSTFLAGS=”-Ctarget-cpu=cortex-m33” cargo +nightly build -Zbuild-std --target=thumbv8m.main-none-eabihf

The following enum:

#[repr(C)]
enum Foo {A, B}

has a size of 4 on the rust side and 1 on the C side.

The specific choice of Cortex CPU is not relevant here, a reduce testcase that enables us to see the size in a compiler error (so we don't need to actually run the code) is as follows:

// arm-none-eabi-g++ test.c -mthumb -mfloat-abi=hard -mfpu=fpv5-sp-d16
typedef enum Foo {A, B} Foo;
char (*error)[sizeof( Foo )] = 1;

Produces the error: error: invalid conversion from 'int' to 'char (*)[1]', whereas:

// cargo build --target=thumbv8m.main-none-eabihf -Zbuild-std
#![no_std]
#[repr(C)]
enum Foo { A, B }
const BAR: [u8; ::core::mem::size_of::<Foo>()] = [];

Produces the error expected an array with a fixed size of 4 elements, found one with 0 elements

This bug traces back to this comment:

// WARNING: the ARM EABI has two variants; the one corresponding
// to `at_least == I32` appears to be used on Linux and NetBSD,
// but some systems may use the variant corresponding to no
// lower bound. However, we don't run on those yet...?
"arm" => min_from_extern = Some(I32),
(see also)

I am able to reproduce this for both thumb and non-thumb, with and without hard floats: arm-none-eabi-g++ test.c, regardless of the -mthumb and -mfloat-abi=hard flags, will always think the enum is of size 1, whereas I tried a bunch of ARM targets on the Rust side and they all think it is of size 4.

arm-linux-gnueabi-g++ agrees with Rust about the size of the enum, so I think this has to do specifically with -none-eabi and -none-eabihf. I feel like we should either follow GCC's convention (which is presumably clang's convention), or expose a codegen flag that flips this.

cc @joshtriplett @cuviper @japaric who typically care about stuff like this

@Manishearth Manishearth added the C-bug Category: This is a bug. label Aug 10, 2021
@jonas-schievink jonas-schievink added A-layout Area: Memory layout of types O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state labels Aug 10, 2021
@Manishearth Manishearth changed the title thumb*-none-eabihf and arm-none-eabi have incorrect enum layout thumb*-none-eabihf and arm-none-eabi have incorrect repr(C) enum layout Aug 10, 2021
@Manishearth
Copy link
Member Author

I think this difference matches to the -mabi=aapcs vs -mabi=aapcs-linux flags on GCC, and GCC is picking different defaults here based on platform.

@Manishearth Manishearth changed the title thumb*-none-eabihf and arm-none-eabi have incorrect repr(C) enum layout thumb*-none-eabihf and arm-none-eabi have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) Aug 10, 2021
@Manishearth Manishearth changed the title thumb*-none-eabihf and arm-none-eabi have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) arm-none-eabi (etc) have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) Aug 10, 2021
@eddyb
Copy link
Member

eddyb commented Aug 10, 2021

That comment has been unchanged since #9613 - I guess we don't do enough FFI testing?

@Manishearth
Copy link
Member Author

lol

perhaps the fix is to make it u8 for arm/thumb-none?

@eddyb
Copy link
Member

eddyb commented Aug 10, 2021

Maybe we want to make it part of the target specification? Feels very wrong to have architecture names hardcoded in ty::layout to begin with.

@Manishearth
Copy link
Member Author

oh, we can just make it None

@Manishearth
Copy link
Member Author

Makes sense. Perhaps also a codegen flag.

@bors bors closed this as completed in 692833a Aug 13, 2021
chrisnc added a commit to chrisnc/rust that referenced this issue Jul 24, 2023
GCC uses the `-fshort-enums` ABI for arm-none and the `int`-sized enum
ABI for arm-linux.
Clang uses the `int`-sized enum ABI for all arm targets.

Both options are permitted by AAPCS.

Rust is matching GCC's behavior for these targets, as interop with code
code compiled by GCC is desirable in the bare-metal context. See rust-lang#87917.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: Memory layout of types C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants