Skip to content

Commit

Permalink
Check that tracing doesn't finish in a different stack frame.
Browse files Browse the repository at this point in the history
An interpreter could do something like:

```
fn f(loc: &Location) {
  for i in 0..1000 {
    control_point(loc);
    if i == hot_threshold {
      return;
    }
  }
}
```

In other words: we start tracing, then `return` before we hit the
control point in the current stack frame again.
  • Loading branch information
ltratt committed Nov 29, 2024
1 parent 2508fbc commit e258828
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ pub(crate) extern "C" fn __yk_deopt(

// The `clone` should really be `Arc::clone(&ctr)` but that doesn't play well with type
// inference in this (unusual) case.
ctr.mt.guard_failure(ctr.clone(), gidx);
ctr.mt.guard_failure(ctr.clone(), gidx, frameaddr);

// Since we won't return from this function, drop `ctr` manually.
drop(ctr);
Expand Down
28 changes: 25 additions & 3 deletions ykrt/src/mt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ impl MT {
hl,
thread_tracer: tt,
promotions: Vec::new(),
frameaddr,
};
}),
Err(e) => todo!("{e:?}"),
Expand All @@ -453,7 +454,11 @@ impl MT {
hl,
thread_tracer,
promotions,
} => (hl, thread_tracer, promotions),
frameaddr: tracing_frameaddr,
} => {
assert_eq!(frameaddr as *const c_void, tracing_frameaddr);
(hl, thread_tracer, promotions)
}
_ => unreachable!(),
},
);
Expand Down Expand Up @@ -485,7 +490,11 @@ impl MT {
hl,
thread_tracer,
promotions,
} => (hl, thread_tracer, promotions),
frameaddr: tracing_frameaddr,
} => {
assert_eq!(frameaddr as *const c_void, tracing_frameaddr);
(hl, thread_tracer, promotions)
}
_ => unreachable!(),
},
);
Expand Down Expand Up @@ -733,7 +742,12 @@ impl MT {
// FIXME: Don't side trace the last guard of a side-trace as this guard always fails.
// FIXME: Don't side-trace after switch instructions: not every guard failure is equal
// and a trace compiled for case A won't work for case B.
pub(crate) fn guard_failure(self: &Arc<Self>, parent: Arc<dyn CompiledTrace>, gidx: GuardIdx) {
pub(crate) fn guard_failure(
self: &Arc<Self>,
parent: Arc<dyn CompiledTrace>,
gidx: GuardIdx,
frameaddr: *const c_void,
) {
match self.transition_guard_failure(parent, gidx) {
TransitionGuardFailure::NoAction => (),
TransitionGuardFailure::StartSideTracing(hl) => {
Expand All @@ -748,6 +762,7 @@ impl MT {
hl,
thread_tracer: tt,
promotions: Vec::new(),
frameaddr,
};
}),
Err(e) => todo!("{e:?}"),
Expand Down Expand Up @@ -809,6 +824,9 @@ enum MTThreadState {
thread_tracer: Box<dyn TraceRecorder>,
/// Records the content of data recorded via `yk_promote`.
promotions: Vec<u8>,
/// The `frameaddr` when tracing started. This allows us to tell if we're finishing tracing
/// at the same point that we started.
frameaddr: *const c_void,
},
/// This thread is executing a trace. Note that the `dyn CompiledTrace` serves two different purposes:
///
Expand Down Expand Up @@ -1023,6 +1041,7 @@ mod tests {
hl,
thread_tracer: Box::new(DummyTraceRecorder),
promotions: Vec::new(),
frameaddr: 0 as *const c_void,
};
});
}
Expand All @@ -1047,6 +1066,7 @@ mod tests {
hl,
thread_tracer: Box::new(DummyTraceRecorder),
promotions: Vec::new(),
frameaddr: 0 as *const c_void,
};
});
}
Expand Down Expand Up @@ -1168,6 +1188,7 @@ mod tests {
hl,
thread_tracer: Box::new(DummyTraceRecorder),
promotions: Vec::new(),
frameaddr: 0 as *const c_void,
};
});
break;
Expand Down Expand Up @@ -1360,6 +1381,7 @@ mod tests {
hl,
thread_tracer: Box::new(DummyTraceRecorder),
promotions: Vec::new(),
frameaddr: 0 as *const c_void,
};
});
assert!(matches!(
Expand Down

0 comments on commit e258828

Please sign in to comment.