Skip to content

Commit

Permalink
Get CI testing working again and prevent future bit-rot (#165)
Browse files Browse the repository at this point in the history
* Apply cargo fmt.

* Drop use of unaligned_references feature and clean up warnings.

* Drop "packed" annotation from Process.

It doesn't look like it's required, and it's causing unaligned
references that newer rust hates.

* Fix pasteos for syscall/interrupt stack alloc errors.

* Give the KERNEL/USER bit fields actual bit field values.

Future rust clippy errors about them doing "& 0" operations.  We can
decide if they're useful flags later if we actually want.

* Add a note about the apparently-dead kernel_stack.

Quiets clippy warnings in the strict-lint target.

* Drop slice_internals feature -- causes a warning, doesn't seem necessary.

* Drop redundant pub(self) annotations.

Fixes future clippy lints.

* Clean up some derefs that the compiler performs anyway.

Fixes future clippy lints.

* Drop redundant casts.

Fixes future clippy lints.

* Suppress or work around the new clippy 0-bitmask lints.

* Bump the nightly toolchain a bit so CI can have some hope again.

Fixes the new clippy complaints in the process.

* Pin the cargo-binutils and rustfilt versions in CI so it's more reproducible.

* Work around testing image build failure.

The RUN was throwing errors with current docker, but we can easily
enough just make it another ADD.
  • Loading branch information
anholt authored Sep 28, 2024
1 parent ffead99 commit 9816b67
Show file tree
Hide file tree
Showing 28 changed files with 137 additions and 132 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jobs:
cargo --version
- name: Install cargo crates
run: cargo install cargo-binutils rustfilt
run: cargo install cargo-binutils@0.3.6 rustfilt@0.2.1

- name: make check
run: make check
Expand Down
18 changes: 10 additions & 8 deletions kernel/arch/x64/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ use kerla_runtime::{
};
use x86::current::segmentation::wrfsbase;

#[repr(C, packed)]
pub struct Process {
rsp: UnsafeCell<u64>,
pub(super) fsbase: AtomicCell<u64>,
pub(super) xsave_area: Option<OwnedPages>,
// This appears dead, but really we're keeping the pages referenced from the
// rsp from being dropped until the Process is dropped.
#[allow(dead_code)]
kernel_stack: OwnedPages,
// FIXME: Do we really need these stacks?
interrupt_stack: OwnedPages,
Expand Down Expand Up @@ -46,17 +48,17 @@ impl Process {
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed to allocate kernel stack");
.expect("failed to allocate interrupt stack");
let syscall_stack = alloc_pages_owned(
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed to allocate kernel stack");
.expect("failed to allocate syscall stack");
let kernel_stack = alloc_pages_owned(
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed to allocat kernel stack");
.expect("failed to allocate kernel stack");

let rsp = unsafe {
let mut rsp: *mut u64 = sp.as_mut_ptr();
Expand Down Expand Up @@ -92,7 +94,7 @@ impl Process {
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed to allocat kernel stack");
.expect("failed to allocate kernel stack");
let interrupt_stack = alloc_pages_owned(
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
Expand Down Expand Up @@ -174,7 +176,7 @@ impl Process {
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed to allocat kernel stack");
.expect("failed to allocate kernel stack");
let rsp = unsafe {
let kernel_sp = kernel_stack.as_vaddr().add(KERNEL_STACK_SIZE);
let mut rsp: *mut u64 = kernel_sp.as_mut_ptr();
Expand Down Expand Up @@ -213,12 +215,12 @@ impl Process {
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed allocate kernel stack");
.expect("failed allocate interrupt stack");
let syscall_stack = alloc_pages_owned(
KERNEL_STACK_SIZE / PAGE_SIZE,
AllocPageFlags::KERNEL | AllocPageFlags::DIRTY_OK,
)
.expect("failed allocate kernel stack");
.expect("failed allocate syscall stack");

Ok(Process {
rsp: UnsafeCell::new(rsp as u64),
Expand Down
3 changes: 2 additions & 1 deletion kernel/fs/mount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ impl RootFs {

fn lookup_mount_point(&self, dir: &Arc<dyn Directory>) -> Result<Option<&MountPoint>> {
let stat = dir.stat()?;
Ok(self.mount_points.get(&stat.inode_no))
let inode_no = stat.inode_no; // Move out of unaligned
Ok(self.mount_points.get(&inode_no))
}

/// Resolves a path into `PathComponent`. If `follow_symlink` is `true`,
Expand Down
3 changes: 3 additions & 0 deletions kernel/fs/opened_file.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Allow the bad bit mask of O_RDONLY
#![allow(clippy::bad_bit_mask)]

use super::{
inode::{DirEntry, Directory, FileLike, INode},
path::PathBuf,
Expand Down
2 changes: 1 addition & 1 deletion kernel/fs/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl AsRef<Path> for PathBuf {
}
}

impl<'a> From<&Path> for PathBuf {
impl From<&Path> for PathBuf {
fn from(path: &Path) -> PathBuf {
PathBuf {
path: path.path.to_owned(),
Expand Down
3 changes: 0 additions & 3 deletions kernel/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
#![no_main]
#![feature(custom_test_frameworks)]
#![feature(alloc_error_handler)]
#![feature(const_btree_new)]
#![feature(array_methods)]
#![test_runner(crate::test_runner::run_tests)]
#![reexport_test_harness_main = "test_main"]
// FIXME:
#![allow(unaligned_references)]
#![feature(trait_alias)]

#[macro_use]
Expand Down
4 changes: 2 additions & 2 deletions kernel/net/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ impl From<MonotonicClock> for Instant {
}
}

pub(self) static SOCKETS: Once<SpinLock<SocketSet>> = Once::new();
static SOCKETS: Once<SpinLock<SocketSet>> = Once::new();
static INTERFACE: Once<SpinLock<EthernetInterface<OurDevice>>> = Once::new();
static DHCP_CLIENT: Once<SpinLock<Dhcpv4Client>> = Once::new();
static DHCP_ENABLED: Once<bool> = Once::new();
pub(self) static SOCKET_WAIT_QUEUE: Once<WaitQueue> = Once::new();
static SOCKET_WAIT_QUEUE: Once<WaitQueue> = Once::new();

pub fn process_packets() {
let mut sockets = SOCKETS.lock();
Expand Down
5 changes: 2 additions & 3 deletions kernel/net/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ use smoltcp::wire::{IpAddress, IpEndpoint, Ipv4Address};
bitflags! {
pub struct RecvFromFlags: i32 {
// TODO:
const _NOT_IMPLEMENTED = 0;
const _NOT_IMPLEMENTED = 0x1;
}
}

bitflags! {
pub struct SendToFlags: i32 {
// TODO:
const _NOT_IMPLEMENTED = 0;
// TODO: remaining flags
const MSG_NOSIGNAL = 0x4000;
}
}
Expand Down
4 changes: 2 additions & 2 deletions kernel/net/tcp_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl FileLike for TcpSocket {
SOCKET_WAIT_QUEUE.sleep_signalable_until(|| {
let mut sockets = SOCKETS.lock();
let mut backlogs = self.backlogs.lock();
match get_ready_backlog_index(&mut *sockets, &*backlogs) {
match get_ready_backlog_index(&mut sockets, &backlogs) {
Some(index) => {
// Pop the client socket and add a new socket into the backlog.
let socket = backlogs.remove(index);
Expand Down Expand Up @@ -313,7 +313,7 @@ impl FileLike for TcpSocket {
fn poll(&self) -> Result<PollStatus> {
let mut status = PollStatus::empty();
let mut sockets = SOCKETS.lock();
if get_ready_backlog_index(&mut *sockets, &*self.backlogs.lock()).is_some() {
if get_ready_backlog_index(&mut sockets, &self.backlogs.lock()).is_some() {
status |= PollStatus::POLLIN;
}

Expand Down
2 changes: 1 addition & 1 deletion kernel/process/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use kerla_runtime::{
page_allocator::{alloc_pages, AllocPageFlags},
spinlock::{SpinLock, SpinLockGuard},
};
use kerla_utils::{alignment::align_up};
use kerla_utils::alignment::align_up;

type ProcessTable = BTreeMap<PId, Arc<Process>>;

Expand Down
4 changes: 2 additions & 2 deletions kernel/process/signal.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{ctypes::c_int, prelude::*};
use kerla_runtime::address::UserVAddr;
use bitvec::prelude::*;
use kerla_runtime::address::UserVAddr;

pub type Signal = c_int;
#[allow(unused)]
Expand Down Expand Up @@ -160,7 +160,7 @@ impl SignalDelivery {
}

//pub type SigSet = BitMap<128 /* 1024 / 8 */>;
pub type SigSet = BitArray<[u8; 1024 / 8 /* quark no_std? */], LocalBits>;
pub type SigSet = BitArray<[u8; 1024 / 8], LocalBits>;
pub enum SignalMask {
Block,
Unblock,
Expand Down
4 changes: 2 additions & 2 deletions kernel/syscalls/linkat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ impl<'a> SyscallHandler<'a> {
let root_fs = current.root_fs().lock();
let opened_files = current.opened_files().lock();
let src = root_fs.lookup_path_at(
&*opened_files,
&opened_files,
&src_dir,
src_path,
flags.contains(AtFlags::AT_SYMLINK_FOLLOW),
)?;
let (parent_dir, dst_name) =
root_fs.lookup_parent_path_at(&*opened_files, &dst_dir, dst_path, true)?;
root_fs.lookup_parent_path_at(&opened_files, &dst_dir, dst_path, true)?;
parent_dir.inode.as_dir()?.link(dst_name, &src.inode)?;
Ok(0)
}
Expand Down
136 changes: 66 additions & 70 deletions kernel/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,64 +16,64 @@ use crate::{
use bitflags::bitflags;
use kerla_runtime::{address::UserVAddr, arch::PtRegs};

pub(self) mod accept;
pub(self) mod arch_prctl;
pub(self) mod bind;
pub(self) mod brk;
pub(self) mod chdir;
pub(self) mod chmod;
pub(self) mod clock_gettime;
pub(self) mod close;
pub(self) mod connect;
pub(self) mod dup2;
pub(self) mod execve;
pub(self) mod exit;
pub(self) mod exit_group;
pub(self) mod fcntl;
pub(self) mod fork;
pub(self) mod fstat;
pub(self) mod fsync;
pub(self) mod getcwd;
pub(self) mod getdents64;
pub(self) mod getpeername;
pub(self) mod getpgid;
pub(self) mod getpid;
pub(self) mod getppid;
pub(self) mod getrandom;
pub(self) mod getsockname;
pub(self) mod getsockopt;
pub(self) mod gettid;
pub(self) mod ioctl;
pub(self) mod kill;
pub(self) mod link;
pub(self) mod linkat;
pub(self) mod listen;
pub(self) mod lstat;
pub(self) mod mkdir;
pub(self) mod mmap;
pub(self) mod open;
pub(self) mod pipe;
pub(self) mod poll;
pub(self) mod read;
pub(self) mod readlink;
pub(self) mod reboot;
pub(self) mod recvfrom;
pub(self) mod rt_sigaction;
pub(self) mod rt_sigprocmask;
pub(self) mod rt_sigreturn;
pub(self) mod select;
pub(self) mod sendto;
pub(self) mod set_tid_address;
pub(self) mod setpgid;
pub(self) mod shutdown;
pub(self) mod socket;
pub(self) mod stat;
pub(self) mod syslog;
pub(self) mod uname;
pub(self) mod utimes;
pub(self) mod wait4;
pub(self) mod write;
pub(self) mod writev;
mod accept;
mod arch_prctl;
mod bind;
mod brk;
mod chdir;
mod chmod;
mod clock_gettime;
mod close;
mod connect;
mod dup2;
mod execve;
mod exit;
mod exit_group;
mod fcntl;
mod fork;
mod fstat;
mod fsync;
mod getcwd;
mod getdents64;
mod getpeername;
mod getpgid;
mod getpid;
mod getppid;
mod getrandom;
mod getsockname;
mod getsockopt;
mod gettid;
mod ioctl;
mod kill;
mod link;
mod linkat;
mod listen;
mod lstat;
mod mkdir;
mod mmap;
mod open;
mod pipe;
mod poll;
mod read;
mod readlink;
mod reboot;
mod recvfrom;
mod rt_sigaction;
mod rt_sigprocmask;
mod rt_sigreturn;
mod select;
mod sendto;
mod set_tid_address;
mod setpgid;
mod shutdown;
mod socket;
mod stat;
mod syslog;
mod uname;
mod utimes;
mod wait4;
mod write;
mod writev;

pub enum CwdOrFd {
/// `AT_FDCWD`
Expand All @@ -96,11 +96,11 @@ bitflags! {
}
}

pub(self) const MAX_READ_WRITE_LEN: usize = core::isize::MAX as usize;
pub(self) const IOV_MAX: usize = 1024;
const MAX_READ_WRITE_LEN: usize = core::isize::MAX as usize;
const IOV_MAX: usize = 1024;

#[repr(C)]
pub(self) struct IoVec {
struct IoVec {
base: UserVAddr,
len: usize,
}
Expand Down Expand Up @@ -269,9 +269,7 @@ impl<'a> SyscallHandler<'a> {
&resolve_path(a4)?,
bitflags_from_user!(AtFlags, a5 as c_int)?,
),
SYS_READLINK => {
self.sys_readlink(&resolve_path(a1)?, UserVAddr::new_nonnull(a2)?, a3 as usize)
}
SYS_READLINK => self.sys_readlink(&resolve_path(a1)?, UserVAddr::new_nonnull(a2)?, a3),
SYS_CHMOD => self.sys_chmod(&resolve_path(a1)?, FileMode::new(a2 as u32)),
SYS_CHOWN => Ok(0), // TODO:
SYS_FSYNC => self.sys_fsync(Fd::new(a1 as i32)),
Expand Down Expand Up @@ -325,11 +323,9 @@ impl<'a> SyscallHandler<'a> {
SYS_EXIT => self.sys_exit(a1 as i32),
SYS_EXIT_GROUP => self.sys_exit_group(a1 as i32),
SYS_SOCKET => self.sys_socket(a1 as i32, a2 as i32, a3 as i32),
SYS_BIND => self.sys_bind(Fd::new(a1 as i32), UserVAddr::new_nonnull(a2)?, a3 as usize),
SYS_BIND => self.sys_bind(Fd::new(a1 as i32), UserVAddr::new_nonnull(a2)?, a3),
SYS_SHUTDOWN => self.sys_shutdown(Fd::new(a1 as i32), a2 as i32),
SYS_CONNECT => {
self.sys_connect(Fd::new(a1 as i32), UserVAddr::new_nonnull(a2)?, a3 as usize)
}
SYS_CONNECT => self.sys_connect(Fd::new(a1 as i32), UserVAddr::new_nonnull(a2)?, a3),
SYS_LISTEN => self.sys_listen(Fd::new(a1 as i32), a2 as c_int),
SYS_GETSOCKNAME => self.sys_getsockname(
Fd::new(a1 as i32),
Expand All @@ -354,15 +350,15 @@ impl<'a> SyscallHandler<'a> {
SYS_SENDTO => self.sys_sendto(
Fd::new(a1 as i32),
UserVAddr::new_nonnull(a2)?,
a3 as usize,
a3,
bitflags_from_user!(SendToFlags, a4 as i32)?,
UserVAddr::new(a5),
a6,
),
SYS_RECVFROM => self.sys_recvfrom(
Fd::new(a1 as i32),
UserVAddr::new_nonnull(a2)?,
a3 as usize,
a3,
bitflags_from_user!(RecvFromFlags, a4 as i32)?,
UserVAddr::new(a5),
UserVAddr::new(a6),
Expand Down
2 changes: 1 addition & 1 deletion kernel/syscalls/open.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'a> SyscallHandler<'a> {
let root_fs = current.root_fs().lock();
let mut opened_files = current.opened_files().lock();

let path_comp = root_fs.lookup_path_at(&*opened_files, &CwdOrFd::AtCwd, path, true)?;
let path_comp = root_fs.lookup_path_at(&opened_files, &CwdOrFd::AtCwd, path, true)?;
if flags.contains(OpenFlags::O_DIRECTORY) && !path_comp.inode.is_dir() {
return Err(Error::new(Errno::ENOTDIR));
}
Expand Down
Loading

0 comments on commit 9816b67

Please sign in to comment.