From 6a10d544326b089503bc48956ed25a7a8ad4e5a3 Mon Sep 17 00:00:00 2001 From: whitequark Date: Mon, 14 May 2018 16:52:14 +0000 Subject: [PATCH] firmware: migrate rpc_proto to new libio. This closes an old and horrible issue in that some code in ksupport may implicitly try to allocate and we'll never know until it crashes at runtime inside liballoc_stub. This also removes liballoc_stub. This also removes ReadExt and WriteExt from libproto. Hooray! --- artiq/firmware/Cargo.lock | 9 +-- artiq/firmware/ksupport/Cargo.toml | 4 +- artiq/firmware/ksupport/lib.rs | 18 ++--- artiq/firmware/liballoc_stub/Cargo.toml | 8 --- artiq/firmware/liballoc_stub/lib.rs | 18 ----- artiq/firmware/libio/lib.rs | 17 +++++ artiq/firmware/libio/proto.rs | 6 ++ artiq/firmware/libproto/Cargo.toml | 6 +- artiq/firmware/libproto/lib.rs | 92 ++----------------------- artiq/firmware/libproto/rpc_proto.rs | 33 ++++++--- artiq/firmware/runtime/Cargo.toml | 4 +- artiq/firmware/runtime/main.rs | 1 + artiq/firmware/runtime/mgmt.rs | 5 +- artiq/firmware/runtime/session.rs | 4 +- 14 files changed, 73 insertions(+), 152 deletions(-) delete mode 100644 artiq/firmware/liballoc_stub/Cargo.toml delete mode 100644 artiq/firmware/liballoc_stub/lib.rs diff --git a/artiq/firmware/Cargo.lock b/artiq/firmware/Cargo.lock index ef7c33ae5..5686e6124 100644 --- a/artiq/firmware/Cargo.lock +++ b/artiq/firmware/Cargo.lock @@ -2,10 +2,6 @@ name = "alloc_list" version = "0.0.0" -[[package]] -name = "alloc_stub" -version = "0.0.0" - [[package]] name = "amp" version = "0.0.0" @@ -146,15 +142,13 @@ dependencies = [ name = "ksupport" version = "0.0.0" dependencies = [ - "alloc_stub 0.0.0", "amp 0.0.0", "board 0.0.0", "build_misoc 0.0.0", - "byteorder 1.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "cslice 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "dyld 0.0.0", + "io 0.0.0", "proto 0.0.0", - "std_artiq 0.0.0", ] [[package]] @@ -211,7 +205,6 @@ dependencies = [ "dyld 0.0.0", "io 0.0.0", "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", - "std_artiq 0.0.0", ] [[package]] diff --git a/artiq/firmware/ksupport/Cargo.toml b/artiq/firmware/ksupport/Cargo.toml index eafb7c7ae..53995f3e4 100644 --- a/artiq/firmware/ksupport/Cargo.toml +++ b/artiq/firmware/ksupport/Cargo.toml @@ -13,10 +13,8 @@ crate-type = ["staticlib"] build_misoc = { path = "../libbuild_misoc" } [dependencies] -byteorder = { version = "1.0", default-features = false } cslice = { version = "0.3" } -alloc_stub = { path = "../liballoc_stub" } -std_artiq = { path = "../libstd_artiq" } +io = { path = "../libio", features = ["byteorder"] } dyld = { path = "../libdyld" } board = { path = "../libboard" } proto = { path = "../libproto" } diff --git a/artiq/firmware/ksupport/lib.rs b/artiq/firmware/ksupport/lib.rs index f4f7c8afb..e46d4e244 100644 --- a/artiq/firmware/ksupport/lib.rs +++ b/artiq/firmware/ksupport/lib.rs @@ -1,31 +1,27 @@ -#![feature(lang_items, asm, libc, panic_unwind, unwind_attributes, global_allocator)] +#![feature(lang_items, asm, libc, panic_unwind, unwind_attributes, global_allocator, + needs_panic_runtime)] #![no_std] +#![needs_panic_runtime] -extern crate byteorder; extern crate cslice; extern crate unwind; extern crate libc; -extern crate alloc_stub; -extern crate std_artiq as std; +extern crate io; extern crate board; extern crate dyld; extern crate proto; extern crate amp; use core::{mem, ptr, slice, str}; -use std::io::Cursor; use cslice::{CSlice, AsCSlice}; -use alloc_stub::StubAlloc; +use io::Cursor; use board::csr; use dyld::Library; use proto::{kernel_proto, rpc_proto}; use proto::kernel_proto::*; use amp::{mailbox, rpc_queue}; -#[global_allocator] -static mut ALLOC: StubAlloc = StubAlloc; - fn send(request: &Message) { unsafe { mailbox::send(request as *const _ as usize) } while !mailbox::acknowledged() {} @@ -131,9 +127,9 @@ extern fn rpc_send_async(service: u32, tag: CSlice, data: *const *const ()) rpc_proto::send_args(&mut writer, service, tag.as_ref(), data)?; writer.position() }; - proto::WriteExt::write_u32(&mut slice, length as u32) + io::proto::ProtoWrite::write_u32(&mut slice, length as u32) }).unwrap_or_else(|err| { - assert!(err.kind() == std::io::ErrorKind::WriteZero); + assert!(err == io::Error::UnexpectedEof); while !rpc_queue::empty() {} send(&RpcSend { diff --git a/artiq/firmware/liballoc_stub/Cargo.toml b/artiq/firmware/liballoc_stub/Cargo.toml deleted file mode 100644 index 1503c9005..000000000 --- a/artiq/firmware/liballoc_stub/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -authors = ["M-Labs"] -name = "alloc_stub" -version = "0.0.0" - -[lib] -name = "alloc_stub" -path = "lib.rs" diff --git a/artiq/firmware/liballoc_stub/lib.rs b/artiq/firmware/liballoc_stub/lib.rs deleted file mode 100644 index 941b5e16f..000000000 --- a/artiq/firmware/liballoc_stub/lib.rs +++ /dev/null @@ -1,18 +0,0 @@ -#![feature(alloc, allocator_api)] -#![no_std] - -extern crate alloc; - -use alloc::allocator::{Layout, AllocErr, Alloc}; - -pub struct StubAlloc; - -unsafe impl<'a> Alloc for &'a StubAlloc { - unsafe fn alloc(&mut self, _layout: Layout) -> Result<*mut u8, AllocErr> { - unimplemented!() - } - - unsafe fn dealloc(&mut self, _ptr: *mut u8, _layout: Layout) { - unimplemented!() - } -} diff --git a/artiq/firmware/libio/lib.rs b/artiq/firmware/libio/lib.rs index 0524e1c6f..8c0b501af 100644 --- a/artiq/firmware/libio/lib.rs +++ b/artiq/firmware/libio/lib.rs @@ -99,6 +99,23 @@ pub trait Write { fn size_hint(&mut self, _min: usize, _max: Option) {} } +#[cfg(not(feature = "std_artiq"))] +impl<'a> Write for &'a mut [u8] { + type WriteError = !; + type FlushError = !; + + fn write(&mut self, buf: &[u8]) -> result::Result { + let len = buf.len().min(self.len()); + self[..len].copy_from_slice(&buf[..len]); + Ok(len) + } + + #[inline] + fn flush(&mut self) -> result::Result<(), Self::FlushError> { + Ok(()) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CursorError { EndOfBuffer diff --git a/artiq/firmware/libio/proto.rs b/artiq/firmware/libio/proto.rs index 143e8fd8f..bb36ffa40 100644 --- a/artiq/firmware/libio/proto.rs +++ b/artiq/firmware/libio/proto.rs @@ -1,15 +1,19 @@ +#[cfg(feature = "alloc")] use core::fmt; +#[cfg(feature = "alloc")] use alloc::string; use byteorder::{ByteOrder, NetworkEndian}; use ::{Read, Write, Error as IoError}; +#[cfg(feature = "alloc")] #[derive(Debug)] pub enum ReadStringError { Utf8Error(string::FromUtf8Error), Other(T) } +#[cfg(feature = "alloc")] impl fmt::Display for ReadStringError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { @@ -21,6 +25,7 @@ impl fmt::Display for ReadStringError { } } +#[cfg(feature = "alloc")] impl From>> for IoError { fn from(value: ReadStringError>) -> IoError { @@ -31,6 +36,7 @@ impl From>> for IoError } } +#[cfg(feature = "alloc")] #[cfg(feature = "std_artiq")] impl From> for ::std_artiq::io::Error where T: Into<::std_artiq::io::Error> diff --git a/artiq/firmware/libproto/Cargo.toml b/artiq/firmware/libproto/Cargo.toml index c1ff3a4c3..c1e980915 100644 --- a/artiq/firmware/libproto/Cargo.toml +++ b/artiq/firmware/libproto/Cargo.toml @@ -11,6 +11,8 @@ path = "lib.rs" byteorder = { version = "1.0", default-features = false } cslice = { version = "0.3" } log = { version = "0.4", default-features = false, optional = true } -io = { path = "../libio", features = ["byteorder", "alloc"] } -std_artiq = { path = "../libstd_artiq", features = ["alloc"] } +io = { path = "../libio", features = ["byteorder"] } dyld = { path = "../libdyld" } + +[features] +alloc = ["io/alloc"] diff --git a/artiq/firmware/libproto/lib.rs b/artiq/firmware/libproto/lib.rs index 4f4e27018..949fabb35 100644 --- a/artiq/firmware/libproto/lib.rs +++ b/artiq/firmware/libproto/lib.rs @@ -1,8 +1,8 @@ #![no_std] -#![feature(alloc)] +#![cfg_attr(feature = "alloc", feature(alloc))] +#[cfg(feature = "alloc")] extern crate alloc; -extern crate byteorder; extern crate cslice; #[cfg(feature = "log")] #[macro_use] @@ -10,97 +10,17 @@ extern crate log; extern crate io; extern crate dyld; -extern crate std_artiq as std; // Internal protocols. pub mod kernel_proto; // External protocols. +#[cfg(feature = "alloc")] pub mod mgmt_proto; +#[cfg(feature = "alloc")] pub mod analyzer_proto; +#[cfg(feature = "alloc")] pub mod moninj_proto; +#[cfg(feature = "alloc")] pub mod session_proto; pub mod rpc_proto; - -use std::io::{Read, Write, Result, Error, ErrorKind}; -use std::vec::Vec; -use std::string::String; -use byteorder::{ByteOrder, NetworkEndian}; - -pub trait ReadExt: Read { - fn read_u8(&mut self) -> Result { - let mut bytes = [0; 1]; - self.read_exact(&mut bytes)?; - Ok(bytes[0]) - } - - fn read_u16(&mut self) -> Result { - let mut bytes = [0; 2]; - self.read_exact(&mut bytes)?; - Ok(NetworkEndian::read_u16(&bytes)) - } - - fn read_u32(&mut self) -> Result { - let mut bytes = [0; 4]; - self.read_exact(&mut bytes)?; - Ok(NetworkEndian::read_u32(&bytes)) - } - - fn read_u64(&mut self) -> Result { - let mut bytes = [0; 8]; - self.read_exact(&mut bytes)?; - Ok(NetworkEndian::read_u64(&bytes)) - } - - fn read_bytes(&mut self) -> Result> { - let length = self.read_u32()?; - let mut value = Vec::new(); - value.resize(length as usize, 0); - self.read_exact(&mut value)?; - Ok(value) - } - - fn read_string(&mut self) -> Result { - let bytes = self.read_bytes()?; - String::from_utf8(bytes) - .map_err(|_| Error::new(ErrorKind::InvalidData, "invalid UTF-8")) - } -} - -impl ReadExt for R {} - -pub trait WriteExt: Write { - fn write_u8(&mut self, value: u8) -> Result<()> { - let bytes = [value; 1]; - self.write_all(&bytes) - } - - fn write_u16(&mut self, value: u16) -> Result<()> { - let mut bytes = [0; 2]; - NetworkEndian::write_u16(&mut bytes, value); - self.write_all(&bytes) - } - - fn write_u32(&mut self, value: u32) -> Result<()> { - let mut bytes = [0; 4]; - NetworkEndian::write_u32(&mut bytes, value); - self.write_all(&bytes) - } - - fn write_u64(&mut self, value: u64) -> Result<()> { - let mut bytes = [0; 8]; - NetworkEndian::write_u64(&mut bytes, value); - self.write_all(&bytes) - } - - fn write_bytes(&mut self, value: &[u8]) -> Result<()> { - self.write_u32(value.len() as u32)?; - self.write_all(value) - } - - fn write_string(&mut self, value: &str) -> Result<()> { - self.write_bytes(value.as_bytes()) - } -} - -impl WriteExt for W {} diff --git a/artiq/firmware/libproto/rpc_proto.rs b/artiq/firmware/libproto/rpc_proto.rs index 8ff7693a9..226cb47b6 100644 --- a/artiq/firmware/libproto/rpc_proto.rs +++ b/artiq/firmware/libproto/rpc_proto.rs @@ -1,11 +1,16 @@ -use std::io::{self, Read, Write}; -use std::str; +use core::str; use cslice::{CSlice, CMutSlice}; -use {ReadExt, WriteExt}; + +use io::{Read, Write, Result}; +use io::proto::{ProtoRead, ProtoWrite}; + use self::tag::{Tag, TagIterator, split_tag}; -unsafe fn recv_value(reader: &mut Read, tag: Tag, data: &mut *mut (), - alloc: &Fn(usize) -> io::Result<*mut ()>) -> io::Result<()> { +unsafe fn recv_value(reader: &mut T, tag: Tag, data: &mut *mut (), + alloc: &Fn(usize) -> Result<*mut (), T::ReadError>) + -> Result<(), T::ReadError> + where T: Read + ?Sized +{ macro_rules! consume_value { ($ty:ty, |$ptr:ident| $map:expr) => ({ let $ptr = (*data) as *mut $ty; @@ -71,8 +76,11 @@ unsafe fn recv_value(reader: &mut Read, tag: Tag, data: &mut *mut (), } } -pub fn recv_return(reader: &mut Read, tag_bytes: &[u8], data: *mut (), - alloc: &Fn(usize) -> io::Result<*mut ()>) -> io::Result<()> { +pub fn recv_return(reader: &mut T, tag_bytes: &[u8], data: *mut (), + alloc: &Fn(usize) -> Result<*mut (), T::ReadError>) + -> Result<(), T::ReadError> + where T: Read + ?Sized +{ let mut it = TagIterator::new(tag_bytes); #[cfg(feature = "log")] debug!("recv ...->{}", it); @@ -84,7 +92,10 @@ pub fn recv_return(reader: &mut Read, tag_bytes: &[u8], data: *mut (), Ok(()) } -unsafe fn send_value(writer: &mut Write, tag: Tag, data: &mut *const ()) -> io::Result<()> { +unsafe fn send_value(writer: &mut T, tag: Tag, data: &mut *const ()) + -> Result<(), T::WriteError> + where T: Write + ?Sized +{ macro_rules! consume_value { ($ty:ty, |$ptr:ident| $map:expr) => ({ let $ptr = (*data) as *const $ty; @@ -158,8 +169,10 @@ unsafe fn send_value(writer: &mut Write, tag: Tag, data: &mut *const ()) -> io:: } } -pub fn send_args(writer: &mut Write, service: u32, tag_bytes: &[u8], - data: *const *const ()) -> io::Result<()> { +pub fn send_args(writer: &mut T, service: u32, tag_bytes: &[u8], data: *const *const ()) + -> Result<(), T::WriteError> + where T: Write + ?Sized +{ let (arg_tags_bytes, return_tag_bytes) = split_tag(tag_bytes); let mut args_it = TagIterator::new(arg_tags_bytes); diff --git a/artiq/firmware/runtime/Cargo.toml b/artiq/firmware/runtime/Cargo.toml index ddca4d8a6..a264157f4 100644 --- a/artiq/firmware/runtime/Cargo.toml +++ b/artiq/firmware/runtime/Cargo.toml @@ -19,13 +19,13 @@ cslice = { version = "0.3" } log = { version = "0.4", default-features = false } managed = { version = "0.6", default-features = false, features = ["alloc", "map"] } unwind_backtrace = { path = "../libunwind_backtrace" } -io = { path = "../libio", features = ["std_artiq"] } +io = { path = "../libio", features = ["byteorder", "std_artiq"] } board = { path = "../libboard", features = ["uart_console", "smoltcp"] } alloc_list = { path = "../liballoc_list" } std_artiq = { path = "../libstd_artiq", features = ["alloc", "io_error_alloc"] } logger_artiq = { path = "../liblogger_artiq" } board_artiq = { path = "../libboard_artiq" } -proto = { path = "../libproto", features = ["log"] } +proto = { path = "../libproto", features = ["log", "alloc"] } amp = { path = "../libamp" } drtioaux = { path = "../libdrtioaux" } diff --git a/artiq/firmware/runtime/main.rs b/artiq/firmware/runtime/main.rs index 7472acc51..212e7f7de 100644 --- a/artiq/firmware/runtime/main.rs +++ b/artiq/firmware/runtime/main.rs @@ -12,6 +12,7 @@ extern crate smoltcp; extern crate alloc_list; extern crate unwind_backtrace; +extern crate io; #[macro_use] extern crate std_artiq as std; extern crate logger_artiq; diff --git a/artiq/firmware/runtime/mgmt.rs b/artiq/firmware/runtime/mgmt.rs index 3c14e776e..70fb2ea07 100644 --- a/artiq/firmware/runtime/mgmt.rs +++ b/artiq/firmware/runtime/mgmt.rs @@ -1,10 +1,11 @@ -use board::boot; use std::io::{self, Read, Write}; use log::{self, LevelFilter}; + +use io::proto::ProtoWrite; +use board::boot; use logger_artiq::BufferLogger; use sched::Io; use sched::{TcpListener, TcpStream}; -use proto::WriteExt; use mgmt_proto::*; use profiler; diff --git a/artiq/firmware/runtime/session.rs b/artiq/firmware/runtime/session.rs index 2d617ef78..d60a48430 100644 --- a/artiq/firmware/runtime/session.rs +++ b/artiq/firmware/runtime/session.rs @@ -313,12 +313,12 @@ fn process_host_message(io: &Io, })?; rpc::recv_return(stream, &tag, slot, &|size| { kern_send(io, &kern::RpcRecvReply(Ok(size)))?; - kern_recv(io, |reply| { + Ok(kern_recv(io, |reply| { match reply { &kern::RpcRecvRequest(slot) => Ok(slot), other => unexpected!("unexpected reply from kernel CPU: {:?}", other) } - }) + })?) })?; kern_send(io, &kern::RpcRecvReply(Ok(0)))?;