Repository navigation
arm-none-eabi (etc) have incorrect repr(C) enum layout (AAPCS vs AAPCS-linux) #87917
Description
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:
Details
// 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:
rust/compiler/rustc_middle/src/ty/layout.rs
Lines 134 to 138 in ae90dcf
| // 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), |
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
Activity
[-]thumb*-none-eabihf and arm-none-eabi have incorrect enum layout[/-][+]thumb*-none-eabihf and arm-none-eabi have incorrect repr(C) enum layout[/+]Manishearth commentedon Aug 10, 2021
I think this difference matches to the
-mabi=aapcsvs-mabi=aapcs-linuxflags on GCC, and GCC is picking different defaults here based on platform.[-]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)[/+][-]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)[/+]eddyb commentedon Aug 10, 2021
That comment has been unchanged since #9613 - I guess we don't do enough FFI testing?
Manishearth commentedon Aug 10, 2021
lol
perhaps the fix is to make it u8 for
arm/thumb-none?eddyb commentedon Aug 10, 2021
Maybe we want to make it part of the target specification? Feels very wrong to have architecture names hardcoded in
ty::layoutto begin with.Manishearth commentedon Aug 10, 2021
oh, we can just make it None
Manishearth commentedon Aug 10, 2021
Makes sense. Perhaps also a codegen flag.
Rollup merge of rust-lang#87922 - Manishearth:c-enum-target-spec, r=n…
Fix comments about GCC/Clang's enum width for arm-none targets.