Skip to content

Commit 5ae3492

Browse files
committed
fix the early init
1 parent ef5b4c7 commit 5ae3492

File tree

4 files changed

+38
-42
lines changed

4 files changed

+38
-42
lines changed

src/bin/miri.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,14 +795,12 @@ fn main() {
795795

796796
debug!("rustc arguments: {:?}", rustc_args);
797797
debug!("crate arguments: {:?}", miri_config.args);
798-
799798
#[cfg(target_os = "linux")]
800799
if !miri_config.native_lib.is_empty() && !miri_config.force_old_native_lib {
801800
// FIXME: This should display a diagnostic / warning on error
802801
// SAFETY: No other threads have spawned yet
803802
let _ = unsafe { miri::init_sv() };
804803
}
805-
806804
run_compiler_and_exit(
807805
&rustc_args,
808806
&mut MiriCompilerCalls::new(miri_config, many_seeds, genmc_config),

src/shims/trace/child.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub struct Supervisor {
2525
}
2626

2727
/// Marker representing that an error occurred during creation of the supervisor.
28+
#[derive(Debug)]
2829
pub struct SvInitError;
2930

3031
impl Supervisor {
@@ -185,12 +186,8 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> {
185186
// The child process is free to unwind, so we won't to avoid doubly freeing
186187
// system resources
187188
let init = std::panic::catch_unwind(|| {
188-
let listener = ChildListener {
189-
message_rx,
190-
pid: child,
191-
attached: false,
192-
override_retcode: None,
193-
};
189+
let listener =
190+
ChildListener { message_rx, attached: false, override_retcode: None };
194191
// Trace as many things as possible, to be able to handle them as needed
195192
let options = ptrace::Options::PTRACE_O_TRACESYSGOOD
196193
| ptrace::Options::PTRACE_O_TRACECLONE
@@ -199,8 +196,8 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> {
199196
match ptrace::seize(child, options) {
200197
// Ptrace works :D
201198
Ok(_) => {
202-
let code =
203-
sv_loop(listener, event_tx, confirm_tx, page_size).unwrap_err();
199+
let code = sv_loop(listener, child, event_tx, confirm_tx, page_size)
200+
.unwrap_err();
204201
// If a return code of 0 is not explicitly given, assume something went
205202
// wrong and return 1
206203
std::process::exit(code.unwrap_or(1))
@@ -236,7 +233,7 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> {
236233
/// Instruct the supervisor process to return a particular code. Useful if for
237234
/// whatever reason this code fails to be intercepted normally. In the case of
238235
/// `abort_if_errors()` used in `bin/miri.rs`, the return code is erroneously
239-
/// given as a if this is not used.
236+
/// given as a 0 if this is not used.
240237
pub fn register_retcode_sv(code: i32) {
241238
let mut sv_guard = SUPERVISOR.lock().unwrap();
242239
if let Some(sv) = sv_guard.take() {

src/shims/trace/messages.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//! ```
1313
//! `TraceRequest::OverrideRetcode` can be sent at any point in the above, including
1414
//! before or after all of them.
15-
//!
15+
//!
1616
//! NB: sending these events out of order, skipping steps, etc. will result in
1717
//! unspecified behaviour from the supervisor process, so use the abstractions
1818
//! in `super::child` (namely `start_ffi()` and `end_ffi()`) to handle this. It is

src/shims/trace/parent.rs

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,6 @@ pub enum ExecEvent {
125125
pub struct ChildListener {
126126
/// The matching channel for the child's `Supervisor` struct.
127127
pub message_rx: ipc::IpcReceiver<TraceRequest>,
128-
/// The main child process' pid.
129-
pub pid: unistd::Pid,
130128
/// Whether an FFI call is currently ongoing.
131129
pub attached: bool,
132130
/// If `Some`, overrides the return code with the given value.
@@ -179,11 +177,8 @@ impl Iterator for ChildListener {
179177
return Some(ExecEvent::Status(pid, signal));
180178
}
181179
} else {
182-
// Log that this happened and pass along the signal.
183-
// If we don't do the kill, the child will instead
184-
// act as if it never received this signal!
185-
eprintln!("Ignoring PtraceEvent {signal:?}");
186-
signal::kill(pid, signal).unwrap();
180+
// Just pass along the signal
181+
ptrace::cont(pid, signal).unwrap();
187182
},
188183
// Child was stopped at the given signal. Same logic as for
189184
// WaitStatus::PtraceEvent
@@ -196,8 +191,7 @@ impl Iterator for ChildListener {
196191
return Some(ExecEvent::Status(pid, signal));
197192
}
198193
} else {
199-
eprintln!("Ignoring Stopped {signal:?}");
200-
signal::kill(pid, signal).unwrap();
194+
ptrace::cont(pid, signal).unwrap();
201195
},
202196
_ => (),
203197
},
@@ -242,6 +236,7 @@ enum ExecError {
242236
/// created before the fork - like statics - are the same).
243237
pub fn sv_loop(
244238
listener: ChildListener,
239+
init_pid: unistd::Pid,
245240
event_tx: ipc::IpcSender<MemEvents>,
246241
confirm_tx: ipc::IpcSender<Confirmation>,
247242
page_size: usize,
@@ -256,18 +251,18 @@ pub fn sv_loop(
256251
// An instance of the Capstone disassembler, so we don't spawn one on every access
257252
let cs = get_disasm();
258253

259-
// The pid of the process that we forked from, used by default if we don't
260-
// have a reason to use another one.
261-
let main_pid = listener.pid;
254+
// The pid of the last process we interacted with, used by default if we don't have a
255+
// reason to use a different one
256+
let mut curr_pid = init_pid;
262257

263258
// There's an initial sigstop we need to deal with
264-
wait_for_signal(main_pid, signal::SIGSTOP, false).map_err(|e| {
259+
wait_for_signal(Some(curr_pid), signal::SIGSTOP, false).map_err(|e| {
265260
match e {
266261
ExecError::Died(code) => code,
267262
ExecError::Shrug => None,
268263
}
269264
})?;
270-
ptrace::cont(main_pid, None).unwrap();
265+
ptrace::cont(curr_pid, None).unwrap();
271266

272267
for evt in listener {
273268
match evt {
@@ -283,9 +278,11 @@ pub fn sv_loop(
283278
// raise a SIGSTOP. We need it to be signal-stopped *and waited for* in
284279
// order to do most ptrace operations!
285280
confirm_tx.send(Confirmation).unwrap();
286-
wait_for_signal(main_pid, signal::SIGSTOP, false).unwrap();
281+
// We can't trust simply calling `Pid::this()` in the child process to give the right
282+
// PID for us, so we get it this way
283+
curr_pid = wait_for_signal(None, signal::SIGSTOP, false).unwrap();
287284

288-
ptrace::syscall(main_pid, None).unwrap();
285+
ptrace::syscall(curr_pid, None).unwrap();
289286
}
290287
// end_ffi was called by the child
291288
ExecEvent::End => {
@@ -296,7 +293,7 @@ pub fn sv_loop(
296293
ch_stack = None;
297294

298295
// No need to monitor syscalls anymore, they'd just be ignored
299-
ptrace::cont(main_pid, None).unwrap();
296+
ptrace::cont(curr_pid, None).unwrap();
300297
}
301298
// Child process was stopped by a signal
302299
ExecEvent::Status(pid, signal) =>
@@ -370,41 +367,45 @@ fn get_disasm() -> capstone::Capstone {
370367

371368
/// Waits for `wait_signal`. If `init_cont`, it will first do a `ptrace::cont`.
372369
/// We want to avoid that in some cases, like at the beginning of FFI.
370+
///
371+
/// If `pid` is `None`, only one wait will be done and `init_cont` should be false.
373372
fn wait_for_signal(
374-
pid: unistd::Pid,
373+
pid: Option<unistd::Pid>,
375374
wait_signal: signal::Signal,
376375
init_cont: bool,
377-
) -> Result<(), ExecError> {
376+
) -> Result<unistd::Pid, ExecError> {
378377
if init_cont {
379-
ptrace::cont(pid, None).unwrap();
378+
ptrace::cont(pid.unwrap(), None).unwrap();
380379
}
381380
// Repeatedly call `waitid` until we get the signal we want, or the process dies
382381
loop {
383-
let stat =
384-
wait::waitid(wait::Id::Pid(pid), WAIT_FLAGS).map_err(|_| ExecError::Died(None))?;
385-
let signal = match stat {
382+
let wait_id = match pid {
383+
Some(pid) => wait::Id::Pid(pid),
384+
None => wait::Id::All,
385+
};
386+
let stat = wait::waitid(wait_id, WAIT_FLAGS).map_err(|_| ExecError::Died(None))?;
387+
let (signal, pid) = match stat {
386388
// Report the cause of death, if we know it
387389
wait::WaitStatus::Exited(_, code) => {
388390
//eprintln!("Exited sig1 {code}");
389391
return Err(ExecError::Died(Some(code)));
390392
}
391393
wait::WaitStatus::Signaled(_, _, _) => return Err(ExecError::Died(None)),
392-
wait::WaitStatus::Stopped(_, signal) => signal,
393-
wait::WaitStatus::PtraceEvent(_, signal, _) => signal,
394+
wait::WaitStatus::Stopped(pid, signal) => (signal, pid),
395+
wait::WaitStatus::PtraceEvent(pid, signal, _) => (signal, pid),
394396
// This covers PtraceSyscall and variants that are impossible with
395397
// the flags set (e.g. WaitStatus::StillAlive)
396398
_ => {
397-
ptrace::cont(pid, None).unwrap();
399+
ptrace::cont(pid.unwrap(), None).unwrap();
398400
continue;
399401
}
400402
};
401403
if signal == wait_signal {
402-
break;
404+
return Ok(pid);
403405
} else {
404406
ptrace::cont(pid, None).map_err(|_| ExecError::Died(None))?;
405407
}
406408
}
407-
Ok(())
408409
}
409410

410411
/// Grabs the access that caused a segfault and logs it down if it's to our memory,
@@ -582,7 +583,7 @@ fn handle_segfault(
582583
ptrace::setregs(pid, new_regs).unwrap();
583584

584585
// Our mempr_* functions end with a raise(SIGSTOP)
585-
wait_for_signal(pid, signal::SIGSTOP, true)?;
586+
wait_for_signal(Some(pid), signal::SIGSTOP, true)?;
586587

587588
// Step 1 instruction
588589
ptrace::setregs(pid, regs_bak).unwrap();
@@ -625,7 +626,7 @@ fn handle_segfault(
625626
new_regs.set_ip(mempr_on as usize);
626627
new_regs.set_sp(stack_ptr);
627628
ptrace::setregs(pid, new_regs).unwrap();
628-
wait_for_signal(pid, signal::SIGSTOP, true)?;
629+
wait_for_signal(Some(pid), signal::SIGSTOP, true)?;
629630

630631
ptrace::setregs(pid, regs_bak).unwrap();
631632
ptrace::syscall(pid, None).unwrap();

0 commit comments

Comments
 (0)