Add fast trace collection & use it for instrumenting guest memory operations #103
Conversation
syntactically
force-pushed
the
lm/tracing
branch
from
e37690f to
cbe1370
Compare
syntactically
force-pushed
the
lm/tracing
branch
from
cbe1370 to
88a6083
Compare
In the future, the outb handler will need to take a Hypervisor instance in order to be able to access register and memory state of the VM, so it doesn't make sense for these interfaces to be more public than the `Hypervisor` trait. Nobody outside of Hyperlight seems to use these at the moment, so it's probably simplest to restrict these to `pub(crate)`. Signed-off-by: Lucy Menon <[email protected]>
syntactically
force-pushed
the
lm/tracing
branch
2 times, most recently
from
538b2d8 to
becb995
Compare
This adds (unused) support for creating trace files for sandboxes and passing them around to relevant sandbox event handler code. This will be used for collecting debug trace and profiling information. Signed-off-by: Lucy Menon <[email protected]>
This will be useful in the near future, when it will allow transforming the exe_info into unwind information without an extra copy. Signed-off-by: Lucy Menon <[email protected]>
This adds a new interface which tracing code can use to request the values of registers from the hypervisor supervising a sandbox. Signed-off-by: Lucy Menon <[email protected]>
Signed-off-by: Lucy Menon <[email protected]>
This allows for producing profiles of memory usage. Signed-off-by: Lucy Menon <[email protected]>
Signed-off-by: Lucy Menon <[email protected]>
syntactically
force-pushed
the
lm/tracing
branch
from
becb995 to
482ce23
Compare
There was a problem hiding this comment.
This looks great to me. Stack unwinding will definitely be very helpful. Btw, it's not totally clear to me how to use the two added features, nor the new trace_dump crate. Would it be possible to add a simple example or minimal docs somewhere? Lastly, should we try to compile some feature matrix of this in CI to make sure we don't break it in the future? Great job 👍
There was a problem hiding this comment.
This is really great @syntactically , I added a few review comments.
I agree with @ludfjig it would be nice to have some docs that explain how to use these features and how they interact (it isn't immediately obvious to me at least if the trace_guest feature is useful on its own).
I also think it would be useful to perhaps have a README for the trace_dump tool that specifies how to use it , last I think we should maybe include the trace_dump binary in the GH release?
| pub(crate) struct TraceInfo { | ||
| /// The epoch against which trace events are timed; at least as | ||
| /// early as the creation of the sandbox being traced. | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
Curious why these need allow(dead_code)?
There was a problem hiding this comment.
At least right now, if you build with trace_guest and on other features, there is no code that emits trace events compiled in so the trace sink structure is unused.
| impl super::exe::UnwindInfo for UnwindInfo { | ||
| fn as_module(&self) -> framehop::Module<Vec<u8>> { | ||
| framehop::Module::new( | ||
| // TODO: plumb through a name from from_file if this |
There was a problem hiding this comment.
Is this todo still relevant? Do we need a follow up to make the file details available?
There was a problem hiding this comment.
I couldn't find much this was actually being used for, so I don't think it actually matters for now; at the same time just coming up with a random name like this is a bit unpleasant, so I think it justifies a comment but not doing more work unless we find a use for having a better name here.
| } | ||
| fn section_data(&mut self, name: &[u8]) -> Option<Vec<u8>> { | ||
| if name == b".eh_frame" && self.resolved_section_header(b".debug_frame").is_some() { | ||
| /* Rustc does not always emit enough information for stack |
There was a problem hiding this comment.
Does this imply that we need debug builds of the guest to make this work?
There was a problem hiding this comment.
I think so, at least for now. Because we have panic="abort" in the guest, release builds don't need to support unwinding the stack at all, since panic is the only native stack unwinding mechanism in rust. There is a rust codegen flag -C force-unwind-tables that can probably force it on dev builds, but I don't know an easy way to convince Cargo to apply that flag just for feature = unwind_guest builds.
| @@ -160,20 +164,53 @@ pub(crate) struct TraceInfo { | |||
| /// The file to which the trace is being written | |||
| #[allow(dead_code)] | |||
| pub file: Arc<Mutex<std::fs::File>>, | |||
There was a problem hiding this comment.
should this be behind the unwind_guest feature flag? Also as before not clear why we need allow(dead_code) here?
There was a problem hiding this comment.
I think it makes sense to have the basic trace sink infrastructure, which could be used by other trace sources in the future (e.g. trace every page table change, trace every write to a register, etc), factored out from them the specific unwinding source. The dead_code is there for the same reason, which is that building with trace_guest to get the common trace sink infrastructure, but no actual trace sources, makes this dead.
| #[cfg(feature = "mem_profile")] | ||
| unsafe impl<const ORDER: usize> alloc::alloc::GlobalAlloc for ProfiledLockedHeap<ORDER> { | ||
| unsafe fn alloc(&self, layout: core::alloc::Layout) -> *mut u8 { | ||
| let addr = self.0.alloc(layout); |
There was a problem hiding this comment.
Heap provides some statistics (alloc, user, total) about allocations , it might be useful to emit those (maybe total at least so we can see how big the heap got?)
There was a problem hiding this comment.
The max size of the user heap is easy to calculate from the trace file. The overhead from the buddy system allocator is probably worth getting those kind of statistics for at least occasionally, but I also don't know if it's worth defining a trace packet format for them when we're planning on replacing the allocator in the near future?
This adds a framework for quickly tracing events of interest in a guest, including getting stack traces out of the guest based on dwarf unwinding information. This capability is used to allow collecting stack traces of memory allocations/frees, providing various statistics on what causes the maximum memory consumption for a sandbox.