search

briansmith / ring

Safe, fast, small crypto using Rust
Other
3.72k stars 703 forks source link

Add "unknown" to LINUX_ABI #2054

Closed exFalso closed 4 months ago

exFalso commented 4 months ago

This PR adds unknown to the LINUX_ABI array in build.rs. This is specifically to support --target=x86_64-fortanix-unknown-sgx, this target currently fails to link altogether because of the missing object files. With unknown added, everything works as expected.

exFalso commented 4 months ago

Also, a comment on build.rs: the is_git branching and the pregenerated/ folder flow makes it difficult to work with the project. It essentially means that the git repo does not reflect the sources that are on crates.io, which also means that the crate cannot be referred to as a cargo git dependency. cargo vendoring also breaks, and in general build tooling (like nix) that assumes the repo contains all sources required for the build stop working.

Suggestion: include the pregenerated/ folder in the repository itself

briansmith commented 4 months ago

Obviously every "unknown" OS isn't guaranteed to have the same ABI as Linux. A more precise change to build.rs is needed to support SGX.

exFalso commented 4 months ago

Just noting: in SGX there is no operating system, it's all straight x86 code.

Also, it turned out that even with the project linking, at runtime the C code faulted, so we ended up completely replacing the ring dependency.

briansmith commented 4 months ago

Also, it turned out that even with the project linking, at runtime the C code faulted, so we ended up completely replacing the ring dependency.

That's because SGX requires its own implementation of the CPU feature detection in ring::cpu::intel. See the discussion in #775.

briansmith commented 4 months ago

Just noting: in SGX there is no operating system, it's all straight x86 code.

It is true that for SGX targets, "unknown" is the target OS, and this assembly code can be (AFAICT) used in SGX.

However, it isn't true that just because the OS is "unknown" then the assembly code is guaranteed to work. Consider x86_64-unknown-none, for example; this assembly code doesn't work for that target because it uses SIMD instructions, but SIMD instructions aren't (in general) available in x86_64-unknown-none, even if CPUID says they are supported.

exFalso commented 4 months ago

Makes sense, fair enough, thank you for the context!