target_env collision on fortanix-sgx

[dingelish]

Hi there,

I'm Yu Ding from Baidu and I'm maintaining Baidu's rust-sgx-sdk. It has been open-sourced for more than 2 years and adopted by many blockchain companies. And I presented a talk on RustFest'18 Paris about this project. It shares the same goal with Fortanix's target -- fortanix-sgx, while it is significantly differs from fortanix.

Currently, my strategy is to build up the rust+sgx ecosystem on top of libcore+rustc+cargo/xargo and link Intel's SGX libraries to provide basic SGX functions. Fortanix's solution does not always depends on Intel's SGX libraries -- they re-implement many of the Intel's codes in Rust.

There is no need to discuss which is better. These two solutions differ from each other based on their different assumptions about trust. So they cannot be compared directly. And I'm pretty much happy to see that Fortanix guys are doing cool things.

The problem I want to say is that Fortanix's PR occupies 'sgx' as a target_env. This results in a disaster in my project -- I'm using 'sgx' as target_env as well. So a large amount of existing Rust+SGX projects based on rust-sgx-sdk could not be built using xargo due to the collision of target_env.

I wonder if the built-in target could be renamed as fortanix-sgx or some other identifier other than sgx. This would be very much helpful to the existing Rust-SGX ecosystem. A number of rust-sgx projects such as MIT's enigma, UC Berkeley's ekiden, are using xargo to build their Rust-SGX enclaves. As you can see that rust-sgx-sdk has 466 github stars which is more popular than Intel's SDK (395 stars).

I can provide PR if needed. Thank you guys so much.

Best,
Yu

[petrochenkov]

cc @jethrogb

[jethrogb]

Which issues are you running into specifically? AFAIK all patches that we have submitted to crates.io crates should be generic enough that they'd work in any SGX environment, there have not been any dependencies on the Fortanix-specific std::os::fortanix_sgx so far. If you're talking about std itself, I'd say that porting incompatibilities are the responsibility of downstream forks, you are encouraged to upstream your changes to avoid incompatibilities in the future.

In any case, I propose target_vendor is used to distinguish the targets (I suggest yours should be target_vendor = "intel").

Fortanix's solution does not always depends on Intel's SGX libraries -- they re-implement many of the Intel's codes in Rust.

Just FYI, there is no dependency on Intel's libraries whatsoever.

[nagisa]

IMO this is not a bug. Both projects are -sgx and both use target_env=sgx rightfully and correctly. As @jethrogb said, to ensure the desired target detection target_env should be used in combination with some other target_* cfg to uniquely identify the target.

[dingelish]

Let me clarify.

When I'm using xargo to build the sysroot, libpanic_abort would be built resulting in the following error:

Compiling panic_abort v0.0.0 (/home/ding/.rustup/toolchains/nightly-2018-12-30-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libpanic_abort)
error[E0428]: the name `abort` is defined multiple times
  --> /home/ding/.rustup/toolchains/nightly-2018-12-30-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libpanic_abort/lib.rs:61:5
   |
48 |     unsafe fn abort() -> ! {
   |     ---------------------- previous definition of the value `abort` here
...
61 |     unsafe fn abort() -> ! {
   |     ^^^^^^^^^^^^^^^^^^^^^^ `abort` redefined here
   |
   = note: `abort` must be defined only once in the value namespace of this block

error: aborting due to previous error

For more information about this error, try `rustc --explain E0428`.
error: Could not compile `panic_abort`.

To learn more, run the command again with --verbose.
error: `"cargo" "build" "--release" "--manifest-path" "/tmp/xargo.Zwy6GtlBtmjL/Cargo.toml" "--target" "x86_64-unknown-linux-sgx" "-p" "panic_abort"` failed with exit code: Some(101)

In my environment, unix is defined as well as target_env = sgx. So both of these two definitions are activated.

Please correct me if i'm wrong. If we apply @nagisa 's solution, target_env = sgx must appear together with target_vendor = fortanix. This change should be made in the entire Rust source code directory.

[nagisa]

In my environment, unix is defined as well as target_env = sgx. So both of these two definitions are activated.

That’s a bug with how unix cfg is activated, because sgx is nothing like an unix.

EDIT: That being said, I do agree that most of the libstd code that is for fortanix-sgx should be cfgd out with #[cfg(all(target_vendor=fortanix, target_env=sgx))].

[dingelish]

@nagisa Thanks for the advise. I'm working on a PR.

[dingelish]

Submitted PR 57243

[jethrogb]

target_vendor is not stable?