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

Use enum class in c abi may cause unsafety #1282

Open
Lambdaris opened this issue Nov 6, 2023 · 0 comments
Open

Use enum class in c abi may cause unsafety #1282

Lambdaris opened this issue Nov 6, 2023 · 0 comments

Comments

@Lambdaris
Copy link

Lambdaris commented Nov 6, 2023

Example

At present the generated code for

#[cxx::bridge]
mod ffi {
    #[derive(PartialEq, Eq)]
    pub enum En {
        A,
        B,
    }

    extern "Rust" {
        fn match_en(e: En);
    }
}

looks like

enum class En : ::std::uint8_t {
  A = 0,
  B = 1,
};
extern "C" {
void cxxbridge1$match_en(::En e) noexcept;
}
void match_en(::En e) noexcept {
  cxxbridge1$match_en(e);
}

But it's undefined behavior to use c++ enum class in c abi.

According to x86 psABI (see figure 3.1), enum is represented with signed 4 byte. But in cpp the layout of enum class should be keep with the assigned type and no implicit conversion permitted.

We have met a bug because:

  1. In cpp we pass an En::A to match_en, the compiler set register dl (the lower 8 bits of edx) to zero and pass edx to the cxxbridge1$match_end;
  2. Normally in rust it only use the least 8 bits of edx ( rust takes the argument as u8). But once if lto is opened the compiler will check the interfaces and find the parameter is from c abi. Then the compiler assumes edx is properly extend zeros and directly use it to reduce instructions.
  3. But the higher bits of edx stored junk data that are not zeros,leading to an unreachable branch.

Firefox had also met similar bug 4 years ago on non-lto environment.

Possible Solution

It's may be better to use the representing types (u8 for enums with repr(u8) .etc) when generate extern C interfaces and static_cast them on cpp side before it is passed to the extern C functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant