From ddf0ba22e163112ab032e04d2a93e2f0a73bab36 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 6 Jun 2025 16:11:25 +0200 Subject: [PATCH 1/2] Use `Command::exec` to run `rustc`/`cargo` commands to ensure that if they exit because of a signal, it will be displayed at the top level --- build_system/src/rust_tools.rs | 34 +++++++++++++++++++++------------- build_system/src/utils.rs | 13 ++----------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/build_system/src/rust_tools.rs b/build_system/src/rust_tools.rs index 105f5eebe24..7a0c5962919 100644 --- a/build_system/src/rust_tools.rs +++ b/build_system/src/rust_tools.rs @@ -1,12 +1,11 @@ use std::collections::HashMap; use std::ffi::OsStr; +#[cfg(unix)] +use std::os::unix::process::CommandExt; use std::path::PathBuf; use crate::config::ConfigInfo; -use crate::utils::{ - get_toolchain, run_command_with_output_and_env_no_err, rustc_toolchain_version_info, - rustc_version_info, -}; +use crate::utils::{get_toolchain, rustc_toolchain_version_info, rustc_version_info}; fn args(command: &str) -> Result>, String> { // We skip the binary and the "cargo"/"rustc" option. @@ -97,6 +96,22 @@ impl RustcTools { } } +fn exec(input: &[&dyn AsRef], env: &HashMap) -> Result<(), String> { + #[cfg(unix)] + { + let error = crate::utils::get_command_inner(input, None, Some(env)).exec(); + eprintln!("execvp syscall failed: {error:?}"); + std::process::exit(1); + } + #[cfg(not(unix))] + { + if crate::utils::run_command_with_output_and_env_no_err(input, None, Some(env)).is_err() { + std::process::exit(1); + } + Ok(()) + } +} + pub fn run_cargo() -> Result<(), String> { let Some(mut tools) = RustcTools::new("cargo")? else { return Ok(()) }; let rustflags = tools.env.get("RUSTFLAGS").cloned().unwrap_or_default(); @@ -105,11 +120,7 @@ pub fn run_cargo() -> Result<(), String> { for arg in &tools.args { command.push(arg); } - if run_command_with_output_and_env_no_err(&command, None, Some(&tools.env)).is_err() { - std::process::exit(1); - } - - Ok(()) + exec(&command, &tools.env) } pub fn run_rustc() -> Result<(), String> { @@ -118,8 +129,5 @@ pub fn run_rustc() -> Result<(), String> { for arg in &tools.args { command.push(arg); } - if run_command_with_output_and_env_no_err(&command, None, Some(&tools.env)).is_err() { - std::process::exit(1); - } - Ok(()) + exec(&command, &tools.env) } diff --git a/build_system/src/utils.rs b/build_system/src/utils.rs index ca177a5feb8..da91b3a8c29 100644 --- a/build_system/src/utils.rs +++ b/build_system/src/utils.rs @@ -1,7 +1,5 @@ use std::collections::HashMap; use std::ffi::OsStr; -#[cfg(unix)] -use std::ffi::c_int; use std::fmt::Debug; use std::fs; #[cfg(unix)] @@ -9,11 +7,6 @@ use std::os::unix::process::ExitStatusExt; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus, Output}; -#[cfg(unix)] -unsafe extern "C" { - fn raise(signal: c_int) -> c_int; -} - fn exec_command( input: &[&dyn AsRef], cwd: Option<&Path>, @@ -27,9 +20,6 @@ fn exec_command( #[cfg(unix)] { if let Some(signal) = status.signal() { - unsafe { - raise(signal as _); - } // In case the signal didn't kill the current process. return Err(command_error(input, &cwd, format!("Process received signal {}", signal))); } @@ -37,7 +27,7 @@ fn exec_command( Ok(status) } -fn get_command_inner( +pub(crate) fn get_command_inner( input: &[&dyn AsRef], cwd: Option<&Path>, env: Option<&HashMap>, @@ -136,6 +126,7 @@ pub fn run_command_with_output_and_env( Ok(()) } +#[cfg(not(unix))] pub fn run_command_with_output_and_env_no_err( input: &[&dyn AsRef], cwd: Option<&Path>, From d14a49d89c21a2f1dfccbefae22f45efc832c79e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 8 Jun 2025 11:48:01 +0200 Subject: [PATCH 2/2] Add documentation about why we use `exec` --- build_system/src/rust_tools.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build_system/src/rust_tools.rs b/build_system/src/rust_tools.rs index 7a0c5962919..7e3e37a30a8 100644 --- a/build_system/src/rust_tools.rs +++ b/build_system/src/rust_tools.rs @@ -99,6 +99,10 @@ impl RustcTools { fn exec(input: &[&dyn AsRef], env: &HashMap) -> Result<(), String> { #[cfg(unix)] { + // We use `exec` to call the `execvp` syscall instead of creating a new process where the + // command will be executed because very few signals can actually kill a current process, + // so if segmentation fault (SIGSEGV signal) happens and we raise to the current process, + // it will simply do nothing and we won't have the nice error message for the shell. let error = crate::utils::get_command_inner(input, None, Some(env)).exec(); eprintln!("execvp syscall failed: {error:?}"); std::process::exit(1);