Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update #81

Merged
merged 10 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions .github/workflows/rust-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
name: Lint
strategy:
matrix:
os: [ubuntu-22.04, windows-2022, macos-11]
os: [ubuntu-22.04, windows-2022, macos-12]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
Expand All @@ -39,7 +39,7 @@ jobs:
name: Test
strategy:
matrix:
os: [ubuntu-22.04, windows-2022, macos-11]
os: [ubuntu-22.04, windows-2022, macos-12]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
Expand All @@ -59,8 +59,7 @@ jobs:
job:
#- { target: aarch64-unknown-linux-gnu, toolchain: stable }
- { target: aarch64-unknown-linux-musl, toolchain: stable }
# workaround for https://github.com/cross-rs/cross/issues/1222
- { target: aarch64-linux-android, toolchain: "1.67.0" }
- { target: aarch64-linux-android, toolchain: stable }
steps:
- uses: actions/checkout@v3
- name: Install Rust
Expand All @@ -70,11 +69,13 @@ jobs:
target: ${{ matrix.job.target }}
- uses: Swatinem/rust-cache@v2
- name: Build
uses: actions-rs/cargo@v1
with:
command: build
use-cross: true
args: --target ${{ matrix.job.target }} --verbose --all-targets
# We need to install cross from git, because https://github.com/cross-rs/cross/issues/1222
# is still unreleased (it's been almost a year since the last release)
# and we can't use 1.67.0 any longer because some dependencies (sigh,
# home, really?) have a higher MSRV...so...
run: |
cargo install cross --git https://github.com/cross-rs/cross --rev 185398b
cross build --target ${{ matrix.job.target }} --verbose --all-targets

deny-check:
name: cargo-deny
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[workspace]
resolver = "2"
members = [
"crash-context",
"crash-handler",
Expand Down
6 changes: 6 additions & 0 deletions crash-handler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

<!-- next-header -->
## [Unreleased] - ReleaseDate
### Changed
- [PR#81](https://github.com/EmbarkStudios/crash-handling/pull/81) resolved [#80](https://github.com/EmbarkStudios/crash-handling/issues/80) by updating `polling` to 0.3.

### Added
- [PR#81](https://github.com/EmbarkStudios/crash-handling/pull/81) resolved [#79](https://github.com/EmbarkStudios/crash-handling/issues/79) by adding `make_single_crash_event`.

## [0.6.0] - 2023-04-03
### Changed
- [PR#70](https://github.com/EmbarkStudios/crash-handling/pull/70) removed `winapi` in favor of embedded bindings.
Expand Down
50 changes: 48 additions & 2 deletions crash-handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,19 @@ impl From<bool> for CrashEventResult {
/// exception handler which makes them slightly safer to handle than UNIX signals,
/// but it is again recommended to do as little work as possible.
pub unsafe trait CrashEvent: Send + Sync {
/// Method invoked when a crash occurs. Returning true indicates your handler
/// has processed the crash and that no further handlers should run.
/// Method invoked when a crash occurs.
///
/// Returning true indicates your handler has processed the crash and that
/// no further handlers should run.
fn on_crash(&self, context: &CrashContext) -> CrashEventResult;
}

/// Creates a [`CrashEvent`] using the supplied closure as the implementation.
///
/// The supplied closure will be called for both real crash events as well as
/// those simulated by calling `simulate_signal/exception`, which is why it is
/// not `FnOnce`
///
/// # Safety
///
/// See the [`CrashEvent`] Safety section for information on why this is `unsafe`.
Expand All @@ -135,6 +141,46 @@ where
Box::new(Wrapper { inner: closure })
}

/// Creates a [`CrashEvent`] using the supplied closure as the implementation.
///
/// This uses an `FnOnce` closure instead of `Fn` like `[make_crash_event]`, but
/// means this closure can only be used for the first crash, and cannot be used
/// in a situation where user-triggered crashes via the `simulate_signal/exception`
/// methods are used.
///
/// # Safety
///
/// See the [`CrashEvent`] Safety section for information on why this is `unsafe`.
#[inline]
pub unsafe fn make_single_crash_event<F>(closure: F) -> Box<dyn CrashEvent>
where
F: Send + Sync + FnOnce(&CrashContext) -> CrashEventResult + 'static,
{
struct Wrapper<F> {
// technically mutexes are not async signal safe on linux, but this is
// an internal-only detail that will be safe _unless_ the callback invoked
// by the user also crashes, but if that occurs...that's on them
inner: parking_lot::Mutex<Option<F>>,
}

unsafe impl<F> CrashEvent for Wrapper<F>
where
F: Send + Sync + FnOnce(&CrashContext) -> CrashEventResult,
{
fn on_crash(&self, context: &CrashContext) -> CrashEventResult {
if let Some(inner) = self.inner.lock().take() {
(inner)(context)
} else {
false.into()
}
}
}

Box::new(Wrapper {
inner: parking_lot::Mutex::new(Some(closure)),
})
}

cfg_if::cfg_if! {
if #[cfg(any(target_os = "linux", target_os = "android"))] {
mod linux;
Expand Down
6 changes: 3 additions & 3 deletions crash-handler/src/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl CrashHandler {
/// method is not called, `PR_SET_PTRACER_ANY` is used to allow any process
/// to dump the current process.
///
/// Note that this is only needed of `/proc/sys/kernel/yama/ptrace_scope`
/// Note that this is only needed if `/proc/sys/kernel/yama/ptrace_scope`
/// is 1 "restricted ptrace", but there is no harm in setting this if it is
/// in another mode.
///
Expand All @@ -82,13 +82,14 @@ impl CrashHandler {
}

/// Sends the specified user signal.
pub fn simulate_signal(&self, signal: Signal) -> crate::CrashEventResult {
pub fn simulate_signal(&self, signal: u32) -> crate::CrashEventResult {
// Normally this would be an unsafe function, since this unsafe encompasses
// the entirety of the body, however the user is really not required to
// uphold any guarantees on their end, so no real need to declare the
// function itself unsafe.
unsafe {
let mut siginfo: libc::signalfd_siginfo = std::mem::zeroed();
siginfo.ssi_signo = signal;
siginfo.ssi_code = state::SI_USER;
siginfo.ssi_pid = std::process::id();

Expand All @@ -98,7 +99,6 @@ impl CrashHandler {
let lock = state::HANDLER.lock();
if let Some(handler) = &*lock {
handler.handle_signal(
signal as i32,
&mut *(&mut siginfo as *mut libc::signalfd_siginfo).cast::<libc::siginfo_t>(),
&mut *(&mut context as *mut crash_context::ucontext_t).cast::<libc::c_void>(),
)
Expand Down
5 changes: 2 additions & 3 deletions crash-handler/src/linux/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ unsafe extern "C" fn signal_handler(
let handler = HANDLER.lock();

if let Some(handler) = &*handler {
match handler.handle_signal(sig as i32, info, uc) {
match handler.handle_signal(info, uc) {
crate::CrashEventResult::Handled(true) => Action::RestoreDefault,
crate::CrashEventResult::Handled(false) => Action::RestorePrevious,
crate::CrashEventResult::Jump { jmp_buf, value } => Action::Jump((jmp_buf, value)),
Expand Down Expand Up @@ -406,7 +406,6 @@ impl HandlerInner {

pub(super) unsafe fn handle_signal(
&self,
_sig: libc::c_int,
info: &mut libc::siginfo_t,
uc: &mut libc::c_void,
) -> crate::CrashEventResult {
Expand All @@ -421,7 +420,7 @@ impl HandlerInner {

{
*crash_ctx = mem::MaybeUninit::zeroed();
let mut cc = &mut *crash_ctx.as_mut_ptr();
let cc = &mut *crash_ctx.as_mut_ptr();

ptr::copy_nonoverlapping(nix_info, &mut cc.siginfo, 1);

Expand Down
1 change: 0 additions & 1 deletion crash-handler/tests/shared.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![allow(unsafe_code)]

pub use ch::debug_print;
use crash_handler as ch;

pub use sadness_generator::SadnessFlavor;
Expand Down
42 changes: 18 additions & 24 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,19 @@ targets = [
{ triple = "x86_64-unknown-linux-musl" },
{ triple = "x86_64-pc-windows-msvc" },
]
# This crate is not published, exclude it since it triggers various warnings
# downstream users wouldn't see
exclude = ["minidumper-test"]

[advisories]
vulnerability = "deny"
unmaintained = "deny"
yanked = "deny"
ignore = [
# PR https://github.com/rust-minidump/rust-minidump/pull/826 already merged, just needs release
"RUSTSEC-2021-0153",
# Irrelevant, dev-only
"RUSTSEC-2021-0145",
]
ignore = []

[licenses]
unlicensed = "deny"
allow = [
"MIT",
"Apache-2.0",
# Unfortunately, encoding brings in a lot of crates with this license
"CC0-1.0",
]
allow = ["MIT", "Apache-2.0"]
copyleft = "deny"
allow-osi-fsf-free = "neither"
default = "deny"
Expand All @@ -31,27 +24,28 @@ exceptions = [{ allow = ["Unicode-DFS-2016"], name = "unicode-ident" }]

[bans]
multiple-versions = "deny"
deny = []
deny = [
# Incredibly heavyweight, we should never have a dependency on this
{ name = "windows" },
# We should never have a dependency on openssl
{ name = "openssl-sys" },
]
skip = [
# The crate is in the repo, so we have the path, but it's also a crates.io
# dependency
{ name = "crash-context" },

# minidump-writer uses old versions
{ name = "memoffset", version = "=0.7.1" },
{ name = "minidump-common", version = "=0.14.0" },
{ name = "range-map", version = "=0.1.5" },
{ name = "num-traits", version = "=0.1.43" },

{ name = "syn", version = "1.0" },
{ name = "bitflags", version = "=1.3.2" },
]
skip-tree = []
skip-tree = [
# parking_lot uses an old version https://github.com/Amanieu/parking_lot/pull/425,
# my original PR didn't have windows-targets since it's not needed at all,
# but it is what it is
{ name = "windows-targets" },
]

[sources]
unknown-registry = "deny"
unknown-git = "deny"
allow-git = []

[sources.allow-org]
github = ["rust-minidump"]
#github = ["rust-minidump"]
10 changes: 5 additions & 5 deletions minidumper/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,23 @@ log = "0.4"
# Minidump writing
minidump-writer = "0.8"
# Event loop
polling = "2.2"
polling = "3.2"
# Nicer locking primitives
parking_lot = "0.12"
# Nicer error creation
thiserror = "1.0"

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
# Improved Unix domain socket support, includes features that are not available in std
uds = "0.2.6"
uds = "0.4"

[target.'cfg(any(target_os = "windows", target_os = "macos"))'.dependencies]
# Nicer binary interop
scroll = "0.11"
# Nicer binary interop, keep aligned with minidump-writer
scroll = { version = "0.11", features = ["derive"] }

[dev-dependencies]
# Diskwrite example
crash-handler = { path = "../crash-handler" }
pretty_env_logger = "0.4.0"
pretty_env_logger = "0.5.0"
# uuid generation
uuid = { version = "1.0", features = ["v4"] }
2 changes: 1 addition & 1 deletion minidumper/examples/diskwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn main() {

cfg_if::cfg_if! {
if #[cfg(any(target_os = "linux", target_os = "android"))] {
handler.simulate_signal(crash_handler::Signal::Segv);
handler.simulate_signal(libc::SIGALRM as _);
} else if #[cfg(windows)] {
handler.simulate_exception(None);
} else if #[cfg(target_os = "macos")] {
Expand Down
71 changes: 69 additions & 2 deletions minidumper/src/ipc.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,76 @@
cfg_if::cfg_if! {
if #[cfg(any(target_os = "linux", target_os = "android"))] {
use std::os::{
unix::{prelude::{RawFd, BorrowedFd}, io::AsRawFd},
fd::AsFd,
};

type Stream = uds::UnixSeqpacketConn;

type Listener = uds::nonblocking::UnixSeqpacketListener;
type Connection = uds::nonblocking::UnixSeqpacketConn;
struct Connection(uds::nonblocking::UnixSeqpacketConn);

impl polling::AsRawSource for Connection {
fn raw(&self) -> RawFd {
self.0.as_raw_fd()
}
}

impl AsRawFd for Connection {
fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd()
}
}

impl AsFd for Connection {
fn as_fd(&self) -> BorrowedFd<'_> {
#[allow(unsafe_code)]
unsafe { BorrowedFd::borrow_raw(self.as_raw_fd()) }
}
}

impl Connection {
#[inline]
fn send(&self, buf: &[u8]) -> Result<usize, std::io::Error> {
self.0.send(buf)
}

#[inline]
fn recv(&self, buf: &mut [u8]) -> Result<usize, std::io::Error> {
self.0.recv(buf)
}

#[inline]
fn recv_vectored(&self, buf: &mut [std::io::IoSliceMut<'_>]) -> Result<(usize, bool), std::io::Error> {
self.0.recv_vectored(buf)
}
}

struct Listener(uds::nonblocking::UnixSeqpacketListener);

impl polling::AsRawSource for Listener {
fn raw(&self) -> RawFd {
self.0.as_raw_fd()
}
}

impl AsRawFd for Listener {
fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd()
}
}

impl AsFd for Listener {
fn as_fd(&self) -> BorrowedFd<'_> {
#[allow(unsafe_code)]
unsafe { BorrowedFd::borrow_raw(self.as_raw_fd()) }
}
}

impl Listener {
fn accept_unix_addr(&self) -> Result<(Connection, uds::UnixSocketAddr), std::io::Error> {
self.0.accept_unix_addr().map(|(conn, addr)| (Connection(conn), addr))
}
}
} else if #[cfg(target_os = "windows")] {
mod windows;

Expand Down
Loading
Loading