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

Support stable rust? #315

Open
xu-cheng opened this issue Feb 9, 2021 · 19 comments
Open

Support stable rust? #315

xu-cheng opened this issue Feb 9, 2021 · 19 comments

Comments

@xu-cheng
Copy link
Contributor

xu-cheng commented Feb 9, 2021

Why nightly rust is needed? Using nightly rust has several major disadvantages:

  • Need to be constantly updated to keep sync with upstream. Currently, the pinned nightly version (1.49.0 nightly-2020-10-25) is already lagged behind the current stable rust (1.49.0 2020-12-29).
  • Nightly rust is of course not stable and may be error prone.

Can stable rust be supported, specially if the enclave code is written in no_std?

@dingelish
Copy link
Contributor

Hi @xu-cheng ,

Rust builds itself by using beta toolchain with backdoor environment variable RUSTC_BOOTSTRAP=1, which enables all nightly feature gates in beta channel. It's almost equivalent to nightly compiler. As a consequence, using stable channel Rust compiler with RUSTC_BOOTSTRAP=1 feature is doable, but does not benefit at all.

@dingelish
Copy link
Contributor

If you look at xargo and cargo-xbuild, you'll find that the community has reached to consensus to build stds with nightly toolchain

rust-osdev/cargo-xbuild#53

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

As far as I understand, there are two ways to use this SDK to write enclave codes: (a) build SGX's version of the std and use it normally; (b) write the enclave codes in no_std.

Can stable rust be used if the second way is used? Here we don't need to deal with xcargo or std aware cargo. Or am I understanding incorrectly?

@dingelish
Copy link
Contributor

dingelish commented Feb 9, 2021

As far as I understand, there are two ways to use this SDK to write enclave codes: (a) build SGX's version of the std and use it normally; (b) write the enclave codes in no_std.

Yes you are correct. But both of them are logically building a sysroot, so I don't recommend to use stable Rust.

Can stable rust be used if the second way is used? Here we don't need to deal with xcargo or std aware cargo. Or am I understanding incorrectly?

Yes you can. Just make sure ${projectroot}/rust-toolchain 's content is something like stable-2020-xx-xx or stable and export RUSTC_BOOTSTRAP=1. I'm not sure if current code can be built successfully. But if you need, I can create a branch to support you. Please specify the stable toolchain you want to use and I'd be working on that.

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

FYI, you may want to look at https://github.com/mobilecoinfoundation/mobilecoin/tree/master/sgx, which is essentially a fork of this project. However, it introduces several major differences in design.

  • Being lightweight and only support no_std.
  • It completely avoids forking and patching common third party crates. IMHO, these patched crates are red herring in terms of maintenance, stability, or even security (as bug fixes in upstream may not be applied in time). Not mentioning it splits the eco systems.
  • It replaces the Makefile build system with rust crate, which seems to be more idiomatic.

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

Yes you can. Just make sure ${projectroot}/rust-toolchain 's content is something like stable-2020-xx-xx or stable and export RUSTC_BOOTSTRAP=1. I'm not sure if current code can be built successfully.

Thanks, I will try and report back.

But if you need, I can create a branch to support you. Please specify the stable toolchain you want to use and I'd be working on that.

I don't think creating a separated branch is a good idea. Would it be possible to officially support this no_std + stable rust approach? If so, a CI workflow can be created to test this approach and ensure it working in the future.

@mssun
Copy link
Member

mssun commented Feb 9, 2021

@xu-cheng @dingelish, I found that the mentioned mobilecoinfoundation/mobilecoin project is under GPLv3 License.

Please don't read and refer to any GPL-licensed code. Thank you so much.

Since we are an Apache Incubator project and under the Apache license, please read this article about GPL-compatibility: https://www.apache.org/licenses/GPL-compatibility.html

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

Please don't read and refer to any GPL-licensed code. Thank you so much.

Sorry, I didn't realize there is a license incompatibility. I merely mention it to point a different design.

@mssun
Copy link
Member

mssun commented Feb 9, 2021

That's OK.

You may look at this issue #311 about a better implementation of the SDK using std-aware Cargo.

@dingelish
Copy link
Contributor

thanks @xu-cheng for the pointer!

the sad fact is that: people wants std here :-( that's why things are so complicated. and std-aware cargo is sometimes buggy :-( only no_std make things clean and tidy. but we don't have this choice.

I'd try to support both stable and nightly in the same branch. but I do have users who wants to always keep updated with latest nightly. and recent frequent breaking changes in libcore::alloc make things much more complicated than before. I want to make every one happy but it's bit of hard :-(

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

Thanks, I will try and report back.

Ok, I tested a bit and got the following errors:

error[E0658]: auto traits are experimental and possibly buggy
   --> /<snip>/rust-sgx-sdk/sgx_tstd/src/panic.rs:118:1
    |
118 | pub auto trait UnwindSafe {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #13231 <https://github.com/rust-lang/rust/issues/13231> for more information
    = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable

error[E0658]: auto traits are experimental and possibly buggy
   --> /<snip>/rust-sgx-sdk/sgx_tstd/src/panic.rs:131:1
    |
131 | pub auto trait RefUnwindSafe {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #13231 <https://github.com/rust-lang/rust/issues/13231> for more information
    = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable

error: aborting due to 2 previous errors
$ rustc --version
rustc 1.49.0 (e1884a8e3 2020-12-29)

I'd try to support both stable and nightly in the same branch. but I do have users who wants to always keep updated with latest nightly. and recent frequent breaking changes in libcore::alloc make things much more complicated than before. I want to make every one happy but it's bit of hard :-(

Understood. Thanks for creating this SDK. If you want to support both versions of Rust, it may be a good idea to setup CI to test them periodically.

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

Rust builds itself by using beta toolchain with backdoor environment variable RUSTC_BOOTSTRAP=1, which enables all nightly feature gates in beta channel. It's almost equivalent to nightly compiler. As a consequence, using stable channel Rust compiler with RUSTC_BOOTSTRAP=1 feature is doable, but does not benefit at all.

Thinking a bit more. I think this is right that stable + RUSTC_BOOTSTRAP=1 is not truly stable rust. The point of stable rust is the assurance on its stability that updating rust will not break the codes. So I guess it defeats my original purpose.

@dingelish
Copy link
Contributor

Hi @xu-cheng

please checkout beta/stable support at https://github.com/apache/incubator-teaclave-sgx-sdk/tree/stable-support

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Feb 9, 2021

please checkout beta/stable support at https://github.com/apache/incubator-teaclave-sgx-sdk/tree/stable-support

Thanks. It works.

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Apr 2, 2021

@dingelish Any chance to merge https://github.com/apache/incubator-teaclave-sgx-sdk/tree/stable-support into the master branch?

Furthermore, is it possible to bump the compiler support to the latest nightly?

I saw the following error and warning when compiling with the latest nightly.

error[E0432]: unresolved import `core::alloc::AllocRef`
  --> /[snip]/incubator-teaclave-sgx-sdk/sgx_alloc/src/system.rs:26:17
   |
26 |     AllocError, AllocRef, GlobalAlloc, Layout,
   |                 ^^^^^^^^ no `AllocRef` in `alloc`
warning: use of deprecated associated function `std::sync::atomic::AtomicI32::compare_and_swap`: Use `compare_exchange` or `compare_exchange_weak` instead
  --> /[snip]/incubator-teaclave-sgx-sdk/sgx_urts/src/event.rs:42:32
   |
42 |                     self.event.compare_and_swap(-1, 0, Ordering::SeqCst);
   |                                ^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

@celaus
Copy link

celaus commented Apr 15, 2021

Hey - just to emphasize, a compiler update for the SDK is really, really crucial moving forward.

https://github.com/rust-lang/rust/releases 1.50 introduced const generics which - since it's considered stable - will make its way into all other crates down the line, since it's a very handy upgrade. Case in point, arrayvec already made the switch: https://docs.rs/arrayvec/0.7.0/arrayvec/#rust-version

This means that whether inside or outside the enclave, we'd be locked to crate versions that build with 1.49. What's blocking the move to stable Rust exactly?

@xu-cheng
Copy link
Contributor Author

xu-cheng commented Aug 1, 2021

Hi @dingelish,

I am wondering whether it is possible to update this SDK to support the latest version of stable or nightly Rust. This SDK really needs update to support mini const generics.

@celaus
Copy link

celaus commented Aug 4, 2021

Hey everyone - I created a quick fix for using Rust 1.55 nightly. Use at your own peril 😅 PR #349

@mssun
Copy link
Member

mssun commented Aug 18, 2021

Another solution to supporting stable rust: https://www.crowdsupply.com/sutajio-kosagi/precursor/updates/adding-rust-stable-libstd-support-for-xous

In a nutshell, we can put compiled std rlib to sysroot.

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

Successfully merging a pull request may close this issue.

4 participants