Vardhan/rust sgx unwind support #1

VardhanThigle · 2018-12-17T11:37:42Z

No description provided.

jethrogb

Approved with minor changes

jethrogb · 2018-12-17T11:38:21Z

src/librustc_target/spec/x86_64_fortanix_unknown_sgx.rs

@@ -11,7 +11,8 @@
 use std::iter;

 use super::{LinkerFlavor, Target, TargetOptions, PanicStrategy};
-
+// TODO-[unwind support] add post link args to libunwind.a
+// How do we bundle it?


Remove comment

jethrogb · 2018-12-17T11:38:28Z

src/librustc_target/spec/x86_64_fortanix_unknown_sgx.rs

@@ -41,13 +42,17 @@ pub fn target() -> Result<Target, String> {
        "ENCLAVE_SIZE",
        "CFGDATA_BASE",
        "DEBUG",
+	    "EH_FRM_HDR_BASE",


jethrogb · 2018-12-17T11:38:56Z

src/librustc_target/spec/x86_64_fortanix_unknown_sgx.rs

+	    "EH_FRM_HDR_BASE",
+        "EH_FRM_HDR_SIZE",
+        "ENCLAVE_TEXT_BASE",
+        "ENCLAVE_TEXT_SIZE",


fortanix/rust-sgx#41 (comment)

jethrogb · 2018-12-17T11:39:23Z

src/libstd/Cargo.toml

@@ -38,7 +38,8 @@ rustc_tsan = { path = "../librustc_tsan" }
 [target.'cfg(any(all(target_arch = "wasm32", not(target_os = "emscripten")), target_env = "sgx"))'.dependencies]
 dlmalloc = { version = "0.1", features = ['rustc-dep-of-std'] }

-[target.x86_64-fortanix-unknown-sgx.dependencies]
+# TODO-[unwind support] Remve this once we test default strategy unwind in rust_c for sgx target 
+[target.x86_64-fortanix-unknown-sgx-unwind.dependencies]


Undo this change

jethrogb · 2018-12-17T11:39:49Z

src/libstd/sys/sgx/rwlock.rs


 pub struct RWLock {
    readers: SpinMutex<WaitVariable<Option<NonZeroUsize>>>,
    writer: SpinMutex<WaitVariable<bool>>,
 }

+
+// Below is to check at compile time, that RWLock has size of 128 bytes.
+#[allow(unreachable_code)]


I don't think you need the lint and the underscore prefix?

jethrogb · 2018-12-17T11:40:28Z

src/libstd/sys/sgx/rwlock.rs

+    }
+
+    #[inline]
+    pub unsafe fn unlock(&self) {


Should not be pub

jethrogb · 2018-12-17T11:40:49Z

src/libstd/sys/sgx/rwlock.rs

    #[inline]
    pub unsafe fn destroy(&self) {}
 }
+
+const EINVAL:i32 = 22;


Nit: add space after :

jethrogb · 2018-12-17T11:41:04Z

src/libstd/sys/sgx/rwlock.rs

+
+
+#[no_mangle]
+pub unsafe extern "C"  fn __rust_rwlock_wrlock(p : *mut RWLock) -> i32 {


Why no NULL check here?

jethrogb · 2018-12-17T11:50:06Z

src/libstd/sys/sgx/rwlock.rs

+}
+
+#[no_mangle]
+pub unsafe extern "C"  fn __rust_print_msg(m : *mut u8, s : i32) -> i32 {


Reduce unsafety. E.g.:

if s < 0 { return; } let buf = slice::from_raw_parts(m as *const u8, s as _); if let Ok(s) = str::from_utf8(&buf[buf.iter().position(|&b| b==0).map_or(.., |p| ..p)]) { eprint!("{}", s); }

Also, it should be eprint! right?

jethrogb · 2018-12-17T11:50:28Z

src/libstd/sys/sgx/rwlock.rs

+const EINVAL:i32 = 22;
+
+
+// TODO-[unwind support] move these to another file maybe? Link issue due to different CUGs.


Remove comment

jethrogb · 2018-12-18T07:25:45Z

src/libstd/sys/sgx/abi/mod.rs

@@ -13,7 +13,7 @@ use io::Write;

 // runtime features
 mod reloc;
-mod mem;
+pub(super) mod mem;


This change is no longer necessary?

jethrogb

Once done, please squash all changes into a single commit. Please run the PR message by me before filing a PR upstream

jethrogb · 2018-12-19T09:10:44Z

src/librustc_target/spec/x86_64_fortanix_unknown_sgx.rs

+            PRE_LINK_ARGS.iter().cloned().map(String::from).collect(),
+        ))
+        .collect(),
+        post_link_objects: iter::once(POST_LINK_OBJS.iter().cloned().map(String::from).collect())


Since this is just a vec with a single element, this should be sufficient:

post_link_objects: vec!["libunwind.a".into()],

Only use the more complicated construction if the simple one becomes to hard to read

jethrogb · 2018-12-19T09:18:48Z

src/libstd/sys/sgx/rwlock.rs

+#[no_mangle]
+pub unsafe extern "C" fn __rust_abort() {
+    ::sys::abort_internal();
+}


Add the following. We don't currently have a way to run tests in SGX but we will in the future.

#[cfg(test)] mod tests { use super::*; use {mem, ptr}; #[test] fn test_c_rwlock_initializer() { unsafe { const RWLOCK_INIT: [u8; 128] = [...]; // hopefully the following trick fills padding/uninitialized bytes // with zeroes let mut init = MaybeUninit::<RWLock>::zeroed(); init.set(RWLock::new()); assert_eq!(mem::transmute::<_, [u8; 128]>(init.into_inner()), RWLOCK_INIT); } } }

Explain safety for `vec.set_len(0)`

jethrogb suggested changes Dec 17, 2018

View reviewed changes

VardhanThigle force-pushed the Vardhan/rust-sgx-unwind-support branch 2 times, most recently from c36454e to e5b12ef Compare December 18, 2018 07:23

jethrogb reviewed Dec 18, 2018

View reviewed changes

VardhanThigle force-pushed the Vardhan/rust-sgx-unwind-support branch 2 times, most recently from 2cd43fa to d7cafe4 Compare December 19, 2018 09:06

jethrogb suggested changes Dec 19, 2018

View reviewed changes

VardhanThigle force-pushed the Vardhan/rust-sgx-unwind-support branch 4 times, most recently from 897ee8a to b9b9f72 Compare December 19, 2018 11:37

Adding unwinding support for x86_64_fortanix_unknown_sgx target.

885cf2a

VardhanThigle force-pushed the Vardhan/rust-sgx-unwind-support branch from b9b9f72 to 885cf2a Compare December 19, 2018 13:02

VardhanThigle merged this pull request into master Dec 21, 2018

VardhanThigle mentioned this pull request Jan 8, 2019

Supporting backtrace for x86_64-fortanix-unknown-sgx. #3

Merged

VardhanThigle pushed a commit that referenced this pull request Jan 13, 2019

Merge pull request #1 from Centril/redo-vec-set_len-docs-adjust

986e49d

Explain safety for `vec.set_len(0)`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vardhan/rust sgx unwind support #1

Vardhan/rust sgx unwind support #1

VardhanThigle commented Dec 17, 2018

jethrogb left a comment

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 17, 2018

jethrogb Dec 18, 2018

jethrogb left a comment

jethrogb Dec 19, 2018

jethrogb Dec 19, 2018



		#[no_mangle]
		pub unsafe extern "C" fn __rust_rwlock_wrlock(p : *mut RWLock) -> i32 {

		const EINVAL:i32 = 22;


		// TODO-[unwind support] move these to another file maybe? Link issue due to different CUGs.

Vardhan/rust sgx unwind support #1

Vardhan/rust sgx unwind support #1

Conversation

VardhanThigle commented Dec 17, 2018

jethrogb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jethrogb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment