-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add SGX target to std and dependencies #56066
Conversation
r? @KodrAus (rust_highfive has picked a reviewer for you, use r? to override) |
|
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@@ -482,7 +484,7 @@ impl Builder { | |||
}; | |||
|
|||
Ok(JoinHandle(JoinInner { | |||
native: Some(imp::Thread::new(stack_size, Box::new(main))?), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliver-giersch I have now implemented my suggestion from #55132 here.
cc @nathanawmk |
No portable code written against This is our fault for looking the other way to get |
@gnzlbg Hmm, as far as I know we haven't really got an alternative solution right now, and I think short of rejecting any new implementations until we do at least keeping patchy |
All targets in this space solve this problem by just depending on There is one single experimental unstable target ( I think that adding more targets like So given that there are alternative solutions to this problem that work, are used successfully by many other similar targets, and that have consensus, why should we make an exception for this target ? EDIT: FWIW I am semi-convinced that the approach being pursued by |
I'm not entirely sure what you mean by "this space". In any case, this is also not true. There are many targets in
There are no alternative solutions that work. Just providing Yes, once we have the portability lint and whatever has been proposed but never implemented in this space, we might be able to work on a different, more general, solution but that shouldn't prevent implementations of new targets in the mean time. |
The Redox, Fuchsia, etc. targets all plan to support full OTOH,
I might have misunderstood you on chat and in the libc PR, but what I understood from your explanations there is that this target is just like
The split of the standard library into |
No. The libc situation is just like wasm. As mentioned in the initial post under “overview”, only |
Then that explains why I was confused as hell on chat while I kept asking these questions and you kept answering that this target is just like |
I'm sorry you were confused. I thought we were talking about |
Yeah, no wonder we both got frustrated on chat, we were both talking about completely different things! Sorry about that! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all looking pretty good to me! I didn't review any SGX-specific code too carefully, mostly just the integration points with the rest of the standard library.
@@ -26,6 +26,7 @@ const fn done<T>() -> *mut Arc<T> { 1_usize as *mut _ } | |||
unsafe impl<T> Sync for Lazy<T> {} | |||
|
|||
impl<T> Lazy<T> { | |||
#[unstable(feature = "sys_internals", issue = "0")] // FIXME: min_const_fn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this cropping up in a few places for a few APIs, but how come this was necessary? (should be fine in any case, just curious)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ultimately comes down to the various const fn new
implementations in waitqueue.rs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm ok, that seems suspicious but it seems fine anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe someone with more experience with min_const_fn
can suggest an alternative solution? @oli-obk ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't call an unstable const fn from a stable one. You might be able to make all of the functions called from this one stable and then you won't need the unstable anymore. note that as of right now, you cannot call unsafe const fns from any const fn.
If you want, just merge the PR as is and open an issue about it and assign it to me
// Some platforms know that printing to stderr won't ever actually | ||
// print anything, and if that's the case we can skip the default | ||
// hook. | ||
Hook::Default if stderr_prints_nothing() => {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe at the time this was added this was an optimization for wasm specifically in LTO mode to ensure that lots of panicking infrastructure csan be removed because in this case the payload is never actually used.
I haven't finished reading this patch yet, but it may be worthwhile running the wasm tests as they should in theory catch this if it's still a problem!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made sure the changes should have the same functionality, and I also checked that set_payload
isn't really doing much so it's ok to do unconditionally. But yeah I suppose some form of LTO might no longer work.
The crux is that currently in this target, checking whether there is a panic output mechanism prevents future use of the same (it's an atomic “take”). I can think if there's another way to restructure all this tomorrow, but this was the best I could come up with a few months ago :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, with regards to this and #56066 (comment), I conflated two things a little bit.
In the SGX target, whether panicking can ever output anything at all is determined post-link time by setting/unsetting the DEBUG
global flag. This is because panic message may contain sensitive information, so it's opt-in. This requirement leads to the function signature: fn panic_output() -> Option<PanicOutput>
. Ideally having DEBUG = false
leads to the same fast-path as wasm without a panic implementation.
The second requirement that leads to the "writer factory" in PanicOutput::Custom
is that in this target the panic writer can only be constructed once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored this a bunch to use types instead of bare functions. I think it should be LTOable. Please take a look.
ee1a112
to
544539f
Compare
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
544539f
to
35898bd
Compare
I don't understand this failure. Is there something special that needs to be done when adding submodules or adding shim crates? |
The files src/libstd/sys/sgx/*.rs are mostly copied/adapted from the wasm target. This also updates the dlmalloc submodule to the very latest version.
d943215
to
7bea6a1
Compare
📌 Commit 7bea6a1 has been approved by |
Add SGX target to std and dependencies This PR adds tier 3 `std` support for the `x86_64-fortanix-unknown-sgx` target. ### Background Intel Software Guard Extensions (SGX) is an instruction set extension for x86 that allows executing code in fully-isolated *secure enclaves*. These enclaves reside in the address space of a regular user process, but access to the enclave's address space from outside (by e.g. the OS or a hypervisor) is blocked. From within such enclaves, there is no access to the operating system or hardware peripherals. In order to communicate with the outside world, enclaves require an untrusted “helper” program that runs as a normal user process. SGX is **not** a sandboxing technology: code inside SGX has full access to all memory belonging to the process it is running in. ### Overview The Fortanix SGX ABI (compiler target `x86_64-fortanix-unknown-sgx`) is an interface for Intel SGX enclaves. It is a small yet functional interface suitable for writing larger enclaves. In contrast to other enclave interfaces, this interface is primarly designed for running entire applications in an enclave. The interface has been under development since early 2016 and builds on Fortanix's significant experience running enclaves in production. Also unlike other enclave interfaces, this is the only implementation of an enclave interface that is nearly pure-Rust (except for the entry point code). A description of the ABI may be found at https://docs.rs/fortanix-sgx-abi/ and https://github.com/fortanix/rust-sgx/blob/master/doc/FORTANIX-SGX-ABI.md. The following parts of `std` are not supported and most operations will error when used: * `std::fs` * `std::process` * `std::net::UdpSocket` ### Future plans A separate PR (#56067) will add the SGX target to the rust compiler. In the very near future, I expect to upgrade this target to tier 2. This PR is just the initial support to make things mostly work. There will be more work coming in the future, for example to add interfaces to the native SGX primitives, implement unwinding, optimize usercalls. UDP and some form of filesystem support may be added in the future, but process support seems unlikely given the platform's constraints. ### Testing build 1. Install [Xargo](https://github.com/japaric/xargo): `cargo install xargo` 2. Create a new Cargo project, for example: `cargo new --bin sgxtest`. 3. Put the following in a file `Xargo.toml` next to your `Cargo.toml`: ```toml [target.x86_64-fortanix-unknown-sgx.dependencies.std] git = "https://github.com/jethrogb/rust" branch = "jb/sgx-target" ``` NB. This can be quite slow. Instead, you can have a local checkout of that branch and use `path = "/path/to/rust/src/libstd"` instead. Don't forget to checkout the submodules too! 4. Build: ```sh xargo build --target x86_64-fortanix-unknown-sgx ``` ### Testing execution Execution is currently only supported on x86-64 Linux, but support for Windows is planned. 1. Install pre-requisites. In order to test execution, you'll need to have a CPU with Intel SGX support. SGX support needs to be enabled in the BIOS. You'll also need to install the SGX driver and Platform Software (PSW) from [Intel](https://01.org/intel-software-guard-extensions). 2. Install toolchain, executor: ```sh cargo install sgxs-tools --version 0.6.0-rc1 cargo install fortanix-sgx-tools --version 0.1.0-rc1 ``` 3. Start the enclave: ```sh ftxsgx-elf2sgxs target/x86_64-fortanix-unknown-sgx/debug/sgxtest --heap-size 0x20000 --ssaframesize 1 --stack-size 0x20000 --threads 1 --debug sgxs-append -i target/x86_64-fortanix-unknown-sgx/debug/sgxtest.sgxs ftxsgx-runner target/x86_64-fortanix-unknown-sgx/debug/sgxtest.sgxs ```
☀️ Test successful - status-appveyor, status-travis |
📣 Toolstate changed by #56066! Tested on commit 15a2607. 💔 clippy-driver on windows: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). |
Tested on commit rust-lang/rust@15a2607. Direct link to PR: <rust-lang/rust#56066> 💔 clippy-driver on windows: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
This PR adds tier 3
std
support for thex86_64-fortanix-unknown-sgx
target.Background
Intel Software Guard Extensions (SGX) is an instruction set extension for x86 that allows executing code in fully-isolated secure enclaves. These enclaves reside in the address space of a regular user process, but access to the enclave's address space from outside (by e.g. the OS or a hypervisor) is blocked.
From within such enclaves, there is no access to the operating system or hardware peripherals. In order to communicate with the outside world, enclaves require an untrusted “helper” program that runs as a normal user process.
SGX is not a sandboxing technology: code inside SGX has full access to all memory belonging to the process it is running in.
Overview
The Fortanix SGX ABI (compiler target
x86_64-fortanix-unknown-sgx
) is an interface for Intel SGX enclaves. It is a small yet functional interface suitable for writing larger enclaves. In contrast to other enclave interfaces, this interface is primarly designed for running entire applications in an enclave. The interface has been under development since early 2016 and builds on Fortanix's significant experience running enclaves in production.Also unlike other enclave interfaces, this is the only implementation of an enclave interface that is nearly pure-Rust (except for the entry point code).
A description of the ABI may be found at https://docs.rs/fortanix-sgx-abi/ and https://github.com/fortanix/rust-sgx/blob/master/doc/FORTANIX-SGX-ABI.md.
The following parts of
std
are not supported and most operations will error when used:std::fs
std::process
std::net::UdpSocket
Future plans
A separate PR (#56067) will add the SGX target to the rust compiler. In the very near future, I expect to upgrade this target to tier 2.
This PR is just the initial support to make things mostly work. There will be more work coming in the future, for example to add interfaces to the native SGX primitives, implement unwinding, optimize usercalls.
UDP and some form of filesystem support may be added in the future, but process support seems unlikely given the platform's constraints.
Testing build
cargo install xargo
cargo new --bin sgxtest
.Xargo.toml
next to yourCargo.toml
:NB. This can be quite slow. Instead, you can have a local checkout of that branch and use
path = "/path/to/rust/src/libstd"
instead. Don't forget to checkout the submodules too!Testing execution
Execution is currently only supported on x86-64 Linux, but support for Windows is planned.
Install pre-requisites. In order to test execution, you'll need to have a CPU with Intel SGX support. SGX support needs to be enabled in the BIOS. You'll also need to install the SGX driver and Platform Software (PSW) from Intel.
Install toolchain, executor: