Skip to content

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

Closed

Description

@Manishearth

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:

// 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

Activity

  1. added
    A-layoutArea: Memory layout of types
    O-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state
    on Aug 10, 2021
  2. 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[/+] on Aug 10, 2021
  3. Manishearth commented on Aug 10, 2021

    @Manishearth
    MemberAuthor

    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.

  4. 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)[/+] on Aug 10, 2021
  5. 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)[/+] on Aug 10, 2021
  6. eddyb commented on Aug 10, 2021

    @eddyb
    Contributor

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

  7. Manishearth commented on Aug 10, 2021

    @Manishearth
    MemberAuthor

    lol

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

  8. eddyb commented on Aug 10, 2021

    @eddyb
    Contributor

    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.

  9. Manishearth commented on Aug 10, 2021

    @Manishearth
    MemberAuthor

    oh, we can just make it None

  10. Manishearth commented on Aug 10, 2021

    @Manishearth
    MemberAuthor

    Makes sense. Perhaps also a codegen flag.

  11. added a commit that references this issue on Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-layoutArea: Memory layout of typesC-bugCategory: This is a bug.O-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions