From 4535741d01e380c6051a50d96809a62a0fd73979 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 19 Aug 2020 11:21:44 -0700 Subject: [PATCH] refactor when we translate io::Error to an Error * a certain subset of io::Errors are expected - these we have a (platform-specific, because windows) method to translate into one of the wasi errno variants in the Error enum. * some io::Errors are unexpected - wasi-common doesnt expect them from the underlying OS. rather than preserve any fidelity in reporting those to the user (only the unix impl attempts this), lets collect those as an `Error::UnexpectedIo(#[source] std::io::Error)`. Rather than trace at the conversion site, we rely on the wiggle error conversion hooks to trace the `Error`'s `Debug` impl, and then we convert all of these unexpected into `Errno::Io` for returning to the guest. This is a different behavior from before, and I don't have any firm guarantees that nobody was depending on the old behavior, but it appears to me that none of those unexpected errnos were reasonable to expect from any of the filesystem syscalls wasi-common is making. --- crates/wasi-common/src/error.rs | 86 ++++++++++++++++++- crates/wasi-common/src/wasi.rs | 144 +------------------------------- 2 files changed, 85 insertions(+), 145 deletions(-) diff --git a/crates/wasi-common/src/error.rs b/crates/wasi-common/src/error.rs index af6d83ea3c..4a1ad031fe 100644 --- a/crates/wasi-common/src/error.rs +++ b/crates/wasi-common/src/error.rs @@ -1,3 +1,4 @@ +use cfg_if::cfg_if; use thiserror::Error; pub type Result = std::result::Result; @@ -14,8 +15,11 @@ pub enum Error { #[error("Utf8Error: {0}")] Utf8(#[from] std::str::Utf8Error), - #[error("IO: {0}")] - IoError(#[from] std::io::Error), + /// The host OS may return an io error that doesn't match one of the + /// wasi errno variants we expect. We do not expose the details of this + /// error to the user. + #[error("Unexpected IoError: {0}")] + UnexpectedIo(#[source] std::io::Error), // Below this, all variants are from the `$errno` type: /// Errno::TooBig: Argument list too long @@ -94,3 +98,81 @@ impl From for Error { unreachable!("should be impossible: From") } } + +// Turning an io::Error into an Error has platform-specific behavior +cfg_if! { + if #[cfg(windows)] { +use winapi::shared::winerror; +use std::io; +impl From for Error { + fn from(err: io::Error) -> Self { + match err.raw_os_error() { + Some(code) => match code as u32 { + winerror::ERROR_SUCCESS => Self::Success, + winerror::ERROR_BAD_ENVIRONMENT => Self::TooBig, + winerror::ERROR_FILE_NOT_FOUND => Self::Noent, + winerror::ERROR_PATH_NOT_FOUND => Self::Noent, + winerror::ERROR_TOO_MANY_OPEN_FILES => Self::Nfile, + winerror::ERROR_ACCESS_DENIED => Self::Acces, + winerror::ERROR_SHARING_VIOLATION => Self::Acces, + winerror::ERROR_PRIVILEGE_NOT_HELD => Self::Notcapable, + winerror::ERROR_INVALID_HANDLE => Self::Badf, + winerror::ERROR_INVALID_NAME => Self::Noent, + winerror::ERROR_NOT_ENOUGH_MEMORY => Self::Nomem, + winerror::ERROR_OUTOFMEMORY => Self::Nomem, + winerror::ERROR_DIR_NOT_EMPTY => Self::Notempty, + winerror::ERROR_NOT_READY => Self::Busy, + winerror::ERROR_BUSY => Self::Busy, + winerror::ERROR_NOT_SUPPORTED => Self::Notsup, + winerror::ERROR_FILE_EXISTS => Self::Exist, + winerror::ERROR_BROKEN_PIPE => Self::Pipe, + winerror::ERROR_BUFFER_OVERFLOW => Self::Nametoolong, + winerror::ERROR_NOT_A_REPARSE_POINT => Self::Inval, + winerror::ERROR_NEGATIVE_SEEK => Self::Inval, + winerror::ERROR_DIRECTORY => Self::Notdir, + winerror::ERROR_ALREADY_EXISTS => Self::Exist, + _ => Self::UnexpectedIo(err), + }, + None => Self::UnexpectedIo(err), + } + } +} + + } else { +use std::io; +impl From for Error { + fn from(err: io::Error) -> Self { + match err.raw_os_error() { + Some(code) => match code { + libc::EPERM => Self::Perm, + libc::ENOENT => Self::Noent, + libc::E2BIG => Self::TooBig, + libc::EIO => Self::Io, + libc::EBADF => Self::Badf, + libc::EACCES => Self::Acces, + libc::EFAULT => Self::Fault, + libc::ENOTDIR => Self::Notdir, + libc::EISDIR => Self::Isdir, + libc::EINVAL => Self::Inval, + libc::EEXIST => Self::Exist, + libc::EFBIG => Self::Fbig, + libc::ENOSPC => Self::Nospc, + libc::ESPIPE => Self::Spipe, + libc::EMFILE => Self::Mfile, + libc::EMLINK => Self::Mlink, + libc::ENAMETOOLONG => Self::Nametoolong, + libc::ENOTEMPTY => Self::Notempty, + libc::ELOOP => Self::Loop, + libc::EOVERFLOW => Self::Overflow, + libc::EILSEQ => Self::Ilseq, + libc::ENOTSUP => Self::Notsup, + _ => Self::UnexpectedIo(err), + }, + None => { + Self::UnexpectedIo(err) + } + } + } +} + } +} diff --git a/crates/wasi-common/src/wasi.rs b/crates/wasi-common/src/wasi.rs index 3124dff225..80f29e64ac 100644 --- a/crates/wasi-common/src/wasi.rs +++ b/crates/wasi-common/src/wasi.rs @@ -1,5 +1,4 @@ use crate::{Error, Result, WasiCtx}; -use cfg_if::cfg_if; use tracing::debug; wiggle::from_witx!({ @@ -36,7 +35,7 @@ impl From for Errno { Error::Guest(e) => e.into(), Error::TryFromInt(_) => Errno::Overflow, Error::Utf8(_) => Errno::Ilseq, - Error::IoError(e) => e.into(), + Error::UnexpectedIo(_) => Errno::Io, Error::TooBig => Errno::TooBig, Error::Acces => Errno::Acces, Error::Badf => Errno::Badf, @@ -237,144 +236,3 @@ impl crate::fdpool::Fd for types::Fd { Self::from(raw_fd) } } - -// Turning an io::Error into an Errno is different on windows. -cfg_if! { - if #[cfg(windows)] { -use winapi::shared::winerror; -use std::io; -impl From for Errno { - fn from(err: io::Error) -> Self { - match err.raw_os_error() { - Some(code) => match code as u32 { - winerror::ERROR_SUCCESS => Self::Success, - winerror::ERROR_BAD_ENVIRONMENT => Self::TooBig, - winerror::ERROR_FILE_NOT_FOUND => Self::Noent, - winerror::ERROR_PATH_NOT_FOUND => Self::Noent, - winerror::ERROR_TOO_MANY_OPEN_FILES => Self::Nfile, - winerror::ERROR_ACCESS_DENIED => Self::Acces, - winerror::ERROR_SHARING_VIOLATION => Self::Acces, - winerror::ERROR_PRIVILEGE_NOT_HELD => Self::Notcapable, - winerror::ERROR_INVALID_HANDLE => Self::Badf, - winerror::ERROR_INVALID_NAME => Self::Noent, - winerror::ERROR_NOT_ENOUGH_MEMORY => Self::Nomem, - winerror::ERROR_OUTOFMEMORY => Self::Nomem, - winerror::ERROR_DIR_NOT_EMPTY => Self::Notempty, - winerror::ERROR_NOT_READY => Self::Busy, - winerror::ERROR_BUSY => Self::Busy, - winerror::ERROR_NOT_SUPPORTED => Self::Notsup, - winerror::ERROR_FILE_EXISTS => Self::Exist, - winerror::ERROR_BROKEN_PIPE => Self::Pipe, - winerror::ERROR_BUFFER_OVERFLOW => Self::Nametoolong, - winerror::ERROR_NOT_A_REPARSE_POINT => Self::Inval, - winerror::ERROR_NEGATIVE_SEEK => Self::Inval, - winerror::ERROR_DIRECTORY => Self::Notdir, - winerror::ERROR_ALREADY_EXISTS => Self::Exist, - x => { - tracing::debug!("winerror: unknown error value: {}", x); - Self::Io - } - }, - None => { - tracing::debug!("Other I/O error: {}", err); - Self::Io - } - } - } -} - - } else { -use std::io; -impl From for Errno { - fn from(err: io::Error) -> Self { - match err.raw_os_error() { - Some(code) => match code { - libc::EPERM => Self::Perm, - libc::ENOENT => Self::Noent, - libc::ESRCH => Self::Srch, - libc::EINTR => Self::Intr, - libc::EIO => Self::Io, - libc::ENXIO => Self::Nxio, - libc::E2BIG => Self::TooBig, - libc::ENOEXEC => Self::Noexec, - libc::EBADF => Self::Badf, - libc::ECHILD => Self::Child, - libc::EAGAIN => Self::Again, - libc::ENOMEM => Self::Nomem, - libc::EACCES => Self::Acces, - libc::EFAULT => Self::Fault, - libc::EBUSY => Self::Busy, - libc::EEXIST => Self::Exist, - libc::EXDEV => Self::Xdev, - libc::ENODEV => Self::Nodev, - libc::ENOTDIR => Self::Notdir, - libc::EISDIR => Self::Isdir, - libc::EINVAL => Self::Inval, - libc::ENFILE => Self::Nfile, - libc::EMFILE => Self::Mfile, - libc::ENOTTY => Self::Notty, - libc::ETXTBSY => Self::Txtbsy, - libc::EFBIG => Self::Fbig, - libc::ENOSPC => Self::Nospc, - libc::ESPIPE => Self::Spipe, - libc::EROFS => Self::Rofs, - libc::EMLINK => Self::Mlink, - libc::EPIPE => Self::Pipe, - libc::EDOM => Self::Dom, - libc::ERANGE => Self::Range, - libc::EDEADLK => Self::Deadlk, - libc::ENAMETOOLONG => Self::Nametoolong, - libc::ENOLCK => Self::Nolck, - libc::ENOSYS => Self::Nosys, - libc::ENOTEMPTY => Self::Notempty, - libc::ELOOP => Self::Loop, - libc::ENOMSG => Self::Nomsg, - libc::EIDRM => Self::Idrm, - libc::ENOLINK => Self::Nolink, - libc::EPROTO => Self::Proto, - libc::EMULTIHOP => Self::Multihop, - libc::EBADMSG => Self::Badmsg, - libc::EOVERFLOW => Self::Overflow, - libc::EILSEQ => Self::Ilseq, - libc::ENOTSOCK => Self::Notsock, - libc::EDESTADDRREQ => Self::Destaddrreq, - libc::EMSGSIZE => Self::Msgsize, - libc::EPROTOTYPE => Self::Prototype, - libc::ENOPROTOOPT => Self::Noprotoopt, - libc::EPROTONOSUPPORT => Self::Protonosupport, - libc::EAFNOSUPPORT => Self::Afnosupport, - libc::EADDRINUSE => Self::Addrinuse, - libc::EADDRNOTAVAIL => Self::Addrnotavail, - libc::ENETDOWN => Self::Netdown, - libc::ENETUNREACH => Self::Netunreach, - libc::ENETRESET => Self::Netreset, - libc::ECONNABORTED => Self::Connaborted, - libc::ECONNRESET => Self::Connreset, - libc::ENOBUFS => Self::Nobufs, - libc::EISCONN => Self::Isconn, - libc::ENOTCONN => Self::Notconn, - libc::ETIMEDOUT => Self::Timedout, - libc::ECONNREFUSED => Self::Connrefused, - libc::EHOSTUNREACH => Self::Hostunreach, - libc::EALREADY => Self::Already, - libc::EINPROGRESS => Self::Inprogress, - libc::ESTALE => Self::Stale, - libc::EDQUOT => Self::Dquot, - libc::ECANCELED => Self::Canceled, - libc::EOWNERDEAD => Self::Ownerdead, - libc::ENOTRECOVERABLE => Self::Notrecoverable, - libc::ENOTSUP => Self::Notsup, - x => { - tracing::debug!("Unknown errno value: {}", x); - Self::Io - } - }, - None => { - tracing::debug!("Other I/O error: {}", err); - Self::Io - } - } - } -} - } -}