Skip to content

Commit 72683db

Browse files
committed
Move the signal handler from ironfish-rust to ironfish-rust-nodejs
This removes the `libc` dependency from `ironfish-rust`, which is not supported in generic WASM environments. In addition to that: * Removed the `triggerSegfault` function, which does not appear to be used anywhere. * Replaced the call to `libc::exit()` with `std::process::abort()`. The two are not exactly equivalent, but the latter is better suited for the task. If we want equivalence, we can use `std::process::exit()` instead. * Corrected the definition of `display_trace`: the argument to signal is `void (*sighandler_t)(int)`, but our `display_trace` was not accepting an int argument. This could lead to stack corruption on some platforms (on x86-64, the first argument is passed on a register, so no stack corruption can occur there, but other platforms may be affected). * Changed the `STACK_TRACE` variable from being a `static mut` to a stack-local variable. This simplifies the implementation and also avoids potential issues with multi-threading. * Protected the `backtrace_symbols_fd` symbol from being defined on platforms where it's not supported.
1 parent 652eae5 commit 72683db

File tree

6 files changed

+31
-66
lines changed

6 files changed

+31
-66
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ironfish-rust-nodejs/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,12 @@ base64 = "0.13.0"
3131
fish_hash = "0.3.0"
3232
ironfish = { path = "../ironfish-rust" }
3333
ironfish-frost = { version = "0.1.0" }
34+
ironfish-jubjub = { version = "0.1.0", features = ["multiply-many"] }
35+
libc = "0.2.150"
3436
napi = { version = "2.14.4", features = ["napi6"] }
3537
napi-derive = "2.14.6"
36-
ironfish-jubjub = { version = "0.1.0", features = ["multiply-many"] }
37-
rand = "0.8.5"
3838
num_cpus = "1.16.0"
39+
rand = "0.8.5"
3940
signal-hook = { version = "0.3.17", optional = true, default-features = false, features = ["iterator"] }
4041

4142
[build-dependencies]

ironfish-rust-nodejs/src/signal_catcher.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,35 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
44

5-
use ironfish::signal_catcher::{init_signal_handler, trigger_segfault};
65
use napi_derive::napi;
76

8-
/// # Safety
9-
/// This is unsafe, it calls libc functions
10-
#[napi(js_name = "initSignalHandler")]
11-
pub unsafe fn native_init_signal_handler() {
12-
init_signal_handler()
7+
extern "C" {
8+
#[cfg(all(unix, not(target_env = "musl"), not(target_os = "android")))]
9+
fn backtrace_symbols_fd(buffer: *const *mut libc::c_void, size: libc::c_int, fd: libc::c_int);
1310
}
1411

15-
/// # Safety
16-
/// This is unsafe, it intentionally crashes
17-
#[napi(js_name = "triggerSegfault")]
18-
pub unsafe fn native_trigger_segfault() {
19-
trigger_segfault()
12+
fn display_trace(_signal: libc::c_int) {
13+
#[cfg(all(unix, not(target_env = "musl"), not(target_os = "android")))]
14+
unsafe {
15+
const MAX_FRAMES: usize = 256;
16+
let mut stack_trace = [std::ptr::null_mut(); MAX_FRAMES];
17+
let depth = libc::backtrace(stack_trace.as_mut_ptr(), MAX_FRAMES as i32);
18+
backtrace_symbols_fd(stack_trace.as_ptr(), depth, libc::STDERR_FILENO);
19+
}
20+
21+
std::process::abort();
22+
}
23+
24+
#[napi(js_name = "initSignalHandler")]
25+
pub fn init_signal_handler() {
26+
#[cfg(unix)]
27+
unsafe {
28+
libc::signal(libc::SIGSEGV, display_trace as usize);
29+
// Rust may throw one of these in place of a SIGSEGV when the platform does not have a
30+
// native implementation for `abort()`, or sometimes when encountering internal errors,
31+
// see: https://doc.rust-lang.org/std/intrinsics/fn.abort.html
32+
libc::signal(libc::SIGBUS, display_trace as usize);
33+
libc::signal(libc::SIGILL, display_trace as usize);
34+
libc::signal(libc::SIGTRAP, display_trace as usize);
35+
}
2036
}

ironfish-rust/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ fish_hash = "0.3.0"
5151
ironfish_zkp = { version = "0.2.0", path = "../ironfish-zkp" }
5252
ironfish-jubjub = { version = "0.1.0", features = ["multiply-many"] }
5353
lazy_static = "1.4.0"
54-
libc = "0.2.126" # sub-dependency that needs a pinned version until a new release of cpufeatures: https://github.com/RustCrypto/utils/pull/789
5554
rand = "0.8.5"
5655
tiny-bip39 = "1.0"
5756
xxhash-rust = { version = "0.8.5", features = ["xxh3"] }

ironfish-rust/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ pub mod nacl;
2424
pub mod note;
2525
pub mod rolling_filter;
2626
pub mod serializing;
27-
pub mod signal_catcher;
2827
pub mod transaction;
2928
pub mod util;
3029
pub mod witness;

ironfish-rust/src/signal_catcher.rs

Lines changed: 0 additions & 50 deletions
This file was deleted.

0 commit comments

Comments
 (0)