Code review (async-usercalls)

Address code review comments

Author: Vasili Novikov

Diff for: .travis.yml

@@ -26,6 +26,7 @@ matrix:
             - clang-11
             - musl-tools
       rust:
+        - nightly
         - nightly-2023-05-07
       env:
         - RUST_BACKTRACE=1

Diff for: intel-sgx/async-usercalls/src/callback.rs

@@ -62,4 +62,4 @@ macro_rules! define_callback {
     };
 }
 
-invoke_with_usercalls!(define_callback);
+invoke_with_usercalls!(define_callback);

Diff for: intel-sgx/async-usercalls/src/lib.rs

@@ -436,4 +436,4 @@ mod tests {
             }
         }
     }
-}
+}

Diff for: intel-sgx/enclave-runner/src/usercalls/abi.rs

@@ -19,7 +19,7 @@ use futures::future::Future;
 
 type Register = u64;
 
-pub(super) trait RegisterArgument {
+trait RegisterArgument {
     fn from_register(_: Register) -> Self;
     fn into_register(self) -> Register;
 }

Diff for: intel-sgx/enclave-runner/src/usercalls/mod.rs

@@ -38,7 +38,9 @@ use sgxs::loader::Tcs as SgxsTcs;
 use crate::loader::{EnclavePanic, ErasedTcs};
 use crate::tcs::{self, CoResult, ThreadResult};
 use self::abi::dispatch;
+use self::abi::ReturnValue;
 use self::abi::UsercallList;
+use self::interface::ToSgxResult;
 use self::interface::{Handler, OutputBuffer};
 
 pub(crate) mod abi;
@@ -793,9 +795,6 @@ impl EnclaveState {
         let mut input = IOHandlerInput { enclave: enclave.clone(), tcs, work_sender: &work_sender };
         let handler = Handler(&mut input);
         let result = {
-            use self::interface::ToSgxResult;
-            use self::abi::ReturnValue;
-
             let (p1, p2, p3, p4, p5) = parameters;
             match notifier_rx {
                 None => dispatch(handler, p1, p2, p3, p4, p5).await.1,

Diff for: ipc-queue/src/position.rs

@@ -27,21 +27,26 @@ impl<T> PositionMonitor<T> {
     pub fn read_position(&self) -> ReadPosition {
         let current = self.fifo.current_offsets(Ordering::Relaxed);
         let read_epoch = self.read_epoch.load(Ordering::Relaxed);
-        let read_epoch_shifted = read_epoch
+        let read_epoch_shifted = (read_epoch as u64)
             .checked_shl(32)
-            .expect("Reading from position of over 2^32 (2 to the power of 32). This is unsupported.");
+            .expect("Read epoch is >= 2^32 (2 to the power of 32). This is unsupported.");
         ReadPosition(read_epoch_shifted | (current.read_offset() as u64))
     }
 
     pub fn write_position(&self) -> WritePosition {
         let current = self.fifo.current_offsets(Ordering::Relaxed);
         let mut write_epoch = self.read_epoch.load(Ordering::Relaxed);
+        // Write epoch keeps track of how many times the write offset wrapped around
+        // the ring buffer. Write epochs are not tracked separately, only read epoch are.
+        // We know, however, that objects are always written to the buffer first.
+        // So, the high bit used in the write and read offsets tell us whether writes
+        // already wrapped around the ring buffer, while reads have not yet.
         if current.read_high_bit() != current.write_high_bit() {
             write_epoch += 1;
         }
         let write_epoch_shifted = write_epoch
             .checked_shl(32)
-            .expect("Writing to position of over 2^32 (2 to the power of 32). This is unsupported.");
+            .expect("Write epoch is >= 2^32 (2 to the power of 32). This is unsupported.");
         WritePosition(write_epoch_shifted | (current.write_offset() as u64))
     }
 }
@@ -57,8 +62,7 @@ impl<T> Clone for PositionMonitor<T> {
 
 impl ReadPosition {
     /// A `WritePosition` can be compared to a `ReadPosition` **correctly** if
-    /// at most 2³¹ (2 to the power of 31) writes
-    /// have occurred since the write position was recorded.
+    /// the ring buffer wrapped around at most 2³¹ (2 to the power of 31) times.
     pub fn is_past(&self, write: &WritePosition) -> bool {
         let (read, write) = (self.0, write.0);
         let hr = read & (1 << 63);
@@ -68,4 +72,4 @@ impl ReadPosition {
         }
         true
     }
-}
+}