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

Ensure ABI is correct for all targets #38

Closed
GabrielMajeri opened this issue Sep 24, 2018 · 8 comments · Fixed by #104
Closed

Ensure ABI is correct for all targets #38

GabrielMajeri opened this issue Sep 24, 2018 · 8 comments · Fixed by #104
Labels

Comments

@GabrielMajeri
Copy link
Collaborator

GabrielMajeri commented Sep 24, 2018

All the UEFI functions are declared with the extern "C" calling convention, which matches the specification.

However, this raises an issue regarding the ABI on x64: extern "C" refers to MS's ABI when compiling for Windows-like targets, but refers to the System-V ABI when compiling for UNIX targets.

This is OK when building with the example target.json, since it tells LLVM to compile for the x86_64-pc-windows-gnu target, which makes it use the MS ABI.

This can cause an issue if the calling executable does not use the MS ABI: if an OS kernel were to be built as an ELF executable, and use this crate for calling UEFI runtime services, it would lead to undefined behavior, since the ELF ABI, gnuabi, is not the same as UEFI's ABI, msabi.

With C compilers we could make a distinction with __attribute__((msabi)) and __attribute__((gnuabi)), but Rust doesn't seem to make that kind of distinction.

Basically, we need to find a way to have functions declared as extern "C" when compiling for all targets except x86_64, where we'll have to use extern "win64" (because otherwise extern "C" becomes extern "sysv64", which is incorrect).

@GabrielMajeri
Copy link
Collaborator Author

This might be part of a bigger issue with supporting 32-bit UEFI. I've tried building for IA32 and it looks like there are a lot of other issues with it (issues with linker / name mangling, lack of compiler-builtins support for some functions).

I think the easiest solution for now would be to simply use extern "win64" everywhere, and drop support for 32-bit.

@HadrienG2
Copy link
Contributor

Related (unfortunately stalled) discussion: rust-lang/rust#54527

@gurry
Copy link
Contributor

gurry commented Nov 2, 2018

Hi guys. We're writing a crate called efi which is very similar to uefi-rs and has almost the same goals. Just like you we're also planning an x86 build and have run into the problem of missing compiler-builtins for 32-bit. Do you happen to know the reasons for them not being implemented? Is it simply that 32-bit is low priority and nobody got to implementing them or there's a technical reason? I know you're not the maintainers of compiler_builtins, but I figured you might already have done some research on this topic :D.

We're building on Windows, by the way, and did not run into any tooling issues like @GabrielMajeri mentioned above fortunately. That's probably because Windows is a more natural home for UEFI development given its heritage.

cc @imor

@GabrielMajeri
Copy link
Collaborator Author

Is it simply that 32-bit is low priority

Unless somebody comes and contributes implementations for the specific functions, there will be no 32-bit support. compiler-builtins is, like most projects, a community-driven effort.

there's a technical reason

In practice there are very few PCs with 32-bit-only UEFI implementations, and it's likely they will become even rarer as time goes on. 32-bit UEFI is also more likely to be buggy, due to less testing being done on it.

@gurry
Copy link
Contributor

gurry commented Nov 3, 2018

Thanks Gabriel 👍. UEFI 32-bit being rare is very useful info for us. As we continue further work on our crate, we'll see if one of us can find time to implement missing builtins.

@roblabla
Copy link
Contributor

I personally need x86 support. As it stands, uefi-rs doesn't support anything that is not x86_64. This means no support for ARM, 32-bit x86, or Itanium. I have two questions:

  • Would simply adding an EFICALL ABI in upstream rust be sufficient? I was thinking that adding an ABI to the list of ABIs and have it select the right underlying ABI in adjust_abi would be enough for our purposes.
  • In the absence of the above, would a proc macro not work? Using syn's VisitorMut, we could make a very simple proc macro like this playground. This macro could be an attribute macro tacked on structs and functions alike, handling both in the same way, which makes for a fairly elegant solution.

@GabrielMajeri
Copy link
Collaborator Author

@roblabla The idea of adding a new ABI to the compiler sounds good to me. The discussion over on rust-lang/rust stalled because the compiler team wanted to move ABI definitions to a JSON format, but that's beyond the scope of the issue. I'd say you could come up with a PR for `extern "uefi``.

As for the proc macro, sure, we could do that, but the initial idea was to get this integrated in the compiler to ensure consistency across different UEFI crates.

@roblabla
Copy link
Contributor

Submitted a PR to rust-lang/rust: rust-lang/rust#65809

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

Successfully merging a pull request may close this issue.

4 participants