Skip to content

Commit 3ea29a5

Browse files
committed
error rework
1 parent 017cdc7 commit 3ea29a5

File tree

2 files changed

+17
-33
lines changed

2 files changed

+17
-33
lines changed

src/shims/trace/child.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> {
204204
let code = sv_loop(listener, child, event_tx, confirm_tx).unwrap_err();
205205
// If a return code of 0 is not explicitly given, assume something went
206206
// wrong and return 1.
207-
std::process::exit(code.unwrap_or(1))
207+
std::process::exit(code.0.unwrap_or(1))
208208
}
209209
// Ptrace does not work and we failed to catch that.
210210
Err(_) => {

src/shims/trace/parent.rs

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,9 @@ impl Iterator for ChildListener {
222222
}
223223

224224
/// An error came up while waiting on the child process to do something.
225+
/// It likely died, with this return code if we have one.
225226
#[derive(Debug)]
226-
enum ExecError {
227-
/// The child process died with this return code, if we have one.
228-
Died(Option<i32>),
229-
/// Something errored, but we should ignore it and proceed.
230-
Shrug,
231-
}
227+
pub struct ExecEnd(pub Option<i32>);
232228

233229
/// This is the main loop of the supervisor process. It runs in a separate
234230
/// process from the rest of Miri (but because we fork, addresses for anything
@@ -238,7 +234,7 @@ pub fn sv_loop(
238234
init_pid: unistd::Pid,
239235
event_tx: ipc::IpcSender<MemEvents>,
240236
confirm_tx: ipc::IpcSender<Confirmation>,
241-
) -> Result<!, Option<i32>> {
237+
) -> Result<!, ExecEnd> {
242238
// Get the pagesize set and make sure it isn't still on the zero sentinel value!
243239
let page_size = PAGE_SIZE.load(std::sync::atomic::Ordering::Relaxed);
244240
assert_ne!(page_size, 0);
@@ -258,12 +254,7 @@ pub fn sv_loop(
258254
let mut curr_pid = init_pid;
259255

260256
// There's an initial sigstop we need to deal with.
261-
wait_for_signal(Some(curr_pid), signal::SIGSTOP, false).map_err(|e| {
262-
match e {
263-
ExecError::Died(code) => code,
264-
ExecError::Shrug => None,
265-
}
266-
})?;
257+
wait_for_signal(Some(curr_pid), signal::SIGSTOP, false)?;
267258
ptrace::cont(curr_pid, None).unwrap();
268259

269260
for evt in listener {
@@ -303,21 +294,14 @@ pub fn sv_loop(
303294
// If it was a segfault, check if it was an artificial one
304295
// caused by it trying to access the MiriMachine memory.
305296
signal::SIGSEGV =>
306-
match handle_segfault(
297+
handle_segfault(
307298
pid,
308299
&ch_pages,
309300
ch_stack.unwrap(),
310301
page_size,
311302
&cs,
312303
&mut acc_events,
313-
) {
314-
Err(e) =>
315-
match e {
316-
ExecError::Died(code) => return Err(code),
317-
ExecError::Shrug => continue,
318-
},
319-
_ => (),
320-
},
304+
)?,
321305
// Something weird happened.
322306
_ => {
323307
eprintln!("Process unexpectedly got {signal}; continuing...");
@@ -335,7 +319,7 @@ pub fn sv_loop(
335319
ptrace::syscall(pid, None).unwrap();
336320
}
337321
ExecEvent::Died(code) => {
338-
return Err(code);
322+
return Err(ExecEnd(code));
339323
}
340324
}
341325
}
@@ -375,7 +359,7 @@ fn wait_for_signal(
375359
pid: Option<unistd::Pid>,
376360
wait_signal: signal::Signal,
377361
init_cont: bool,
378-
) -> Result<unistd::Pid, ExecError> {
362+
) -> Result<unistd::Pid, ExecEnd> {
379363
if init_cont {
380364
ptrace::cont(pid.unwrap(), None).unwrap();
381365
}
@@ -385,13 +369,13 @@ fn wait_for_signal(
385369
Some(pid) => wait::Id::Pid(pid),
386370
None => wait::Id::All,
387371
};
388-
let stat = wait::waitid(wait_id, WAIT_FLAGS).map_err(|_| ExecError::Died(None))?;
372+
let stat = wait::waitid(wait_id, WAIT_FLAGS).map_err(|_| ExecEnd(None))?;
389373
let (signal, pid) = match stat {
390374
// Report the cause of death, if we know it.
391375
wait::WaitStatus::Exited(_, code) => {
392-
return Err(ExecError::Died(Some(code)));
376+
return Err(ExecEnd(Some(code)));
393377
}
394-
wait::WaitStatus::Signaled(_, _, _) => return Err(ExecError::Died(None)),
378+
wait::WaitStatus::Signaled(_, _, _) => return Err(ExecEnd(None)),
395379
wait::WaitStatus::Stopped(pid, signal) => (signal, pid),
396380
wait::WaitStatus::PtraceEvent(pid, signal, _) => (signal, pid),
397381
// This covers PtraceSyscall and variants that are impossible with
@@ -404,7 +388,7 @@ fn wait_for_signal(
404388
if signal == wait_signal {
405389
return Ok(pid);
406390
} else {
407-
ptrace::cont(pid, signal).map_err(|_| ExecError::Died(None))?;
391+
ptrace::cont(pid, signal).map_err(|_| ExecEnd(None))?;
408392
}
409393
}
410394
}
@@ -418,7 +402,7 @@ fn handle_segfault(
418402
page_size: usize,
419403
cs: &capstone::Capstone,
420404
acc_events: &mut Vec<AccessEvent>,
421-
) -> Result<(), ExecError> {
405+
) -> Result<(), ExecEnd> {
422406
/// This is just here to not pollute the main namespace with `capstone::prelude::*`.
423407
#[inline]
424408
fn capstone_disassemble(
@@ -538,7 +522,7 @@ fn handle_segfault(
538522

539523
// Get information on what caused the segfault. This contains the address
540524
// that triggered it.
541-
let siginfo = ptrace::getsiginfo(pid).map_err(|_| ExecError::Shrug)?;
525+
let siginfo = ptrace::getsiginfo(pid).unwrap();
542526
// All x86, ARM, etc. instructions only have at most one memory operand
543527
// (thankfully!)
544528
// SAFETY: si_addr is safe to call.
@@ -599,7 +583,7 @@ fn handle_segfault(
599583
// Don't use wait_for_signal here since 1 instruction doesn't give room
600584
// for any uncertainty + we don't want it `cont()`ing randomly by accident
601585
// Also, don't let it continue with unprotected memory if something errors!
602-
let _ = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecError::Died(None))?;
586+
let _ = wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecEnd(None))?;
603587

604588
// Zero out again to be safe
605589
for a in (ch_stack..ch_stack.strict_add(FAKE_STACK_SIZE)).step_by(ARCH_WORD_SIZE) {
@@ -651,7 +635,7 @@ fn handle_segfault(
651635
eprintln!("Expected access on pages: {ch_pages:#018x?}");
652636
eprintln!("Register dump: {regs:#x?}");
653637
ptrace::kill(pid).unwrap();
654-
Err(ExecError::Died(None))
638+
Err(ExecEnd(None))
655639
}
656640
}
657641

0 commit comments

Comments
 (0)