Skip to content

Commit

Permalink
Improve handling of unmerged ETL files.
Browse files Browse the repository at this point in the history
We still merge during recording. But this is one step on the way
to a world where we no longer need to do it. We could probably
stop merging right now for the case where we only have a kernel
session and no user session.

This commit tweaks how we buffer the ImageID information before we
process the MSNT_SystemTrace/Image information.
And it adds handling to get the CodeId + DebugId from the binary,
if we're importing an unmerged ETL file.

I've removed a fair amount of code that we had kept around for
adding kernel driver handling. It doesn't seem to be needed - I get
MSNT_SystemTrace/Image/DCStart events for all the same kernel modules
that I also see in the merged ETL.
  • Loading branch information
mstange committed Jul 13, 2024
1 parent a2cde52 commit 17a9b85
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 230 deletions.
64 changes: 48 additions & 16 deletions samply/src/windows/etw_gecko.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use uuid::Uuid;

use super::profile_context::ProfileContext;
use crate::windows::coreclr;
use crate::windows::profile_context::KnownCategory;
use crate::windows::profile_context::{ImageInfoFromMergedEtl, KnownCategory};

pub fn profile_pid_from_etl_file(context: &mut ProfileContext, etl_file: &Path) {
let is_arm64 = context.is_arm64();
Expand All @@ -26,6 +26,7 @@ pub fn profile_pid_from_etl_file(context: &mut ProfileContext, etl_file: &Path)
let demand_zero_faults = false; //pargs.contains("--demand-zero-faults");

let mut core_clr_context = coreclr::CoreClrContext::new(context.creation_props());
let mut pending_image_info: Option<((u32, u64), ImageInfoFromMergedEtl)> = None;

let result = open_trace(etl_file, |e| {
let Ok(s) = schema_locator.event_schema(e) else {
Expand Down Expand Up @@ -198,19 +199,25 @@ pub fn profile_pid_from_etl_file(context: &mut ProfileContext, etl_file: &Path)
text,
);
}
// KernelTraceControl/ImageID/ and KernelTraceControl/ImageID/DbgID_RSDS are synthesized by xperf during
// `xperf -stop -d` from MSNT_SystemTrace/Image/DCStart and MSNT_SystemTrace/Image/Load; they are inserted
// right before the original events.
//
// These KernelTraceControl events are not available in unmerged ETL files.
//
// We can get the following information out of them:
// - KernelTraceControl/ImageID/ has the image_timestamp (needed for the CodeId)
// - KernelTraceControl/ImageID/DbgID_RSDS has the guid+age and the PDB path (needed for DebugId + debug_path).
"KernelTraceControl/ImageID/" => {
// KernelTraceControl/ImageID/ and KernelTraceControl/ImageID/DbgID_RSDS are synthesized during merge from
// from MSNT_SystemTrace/Image/Load but don't contain the full path of the binary, or the full debug info in one go.
// We go through "ImageID/" to capture pid/address + the original filename, then expect to see a "DbgID_RSDS" event
// with pdb and debug info. We link those up through the "libs" hash, and then finally add them to the process
// in Image/Load.

let pid = s.process_id(); // there isn't a ProcessId field here
let image_base: u64 = parser.try_parse("ImageBase").unwrap();
let image_timestamp: u32 = parser.try_parse("TimeDateStamp").unwrap();
let image_size: u32 = parser.try_parse("ImageSize").unwrap();
let image_path: String = parser.try_parse("OriginalFileName").unwrap();
context.handle_image_id(pid, image_base, image_timestamp, image_size, image_path);
let mut pending_info = match pending_image_info.take() {
Some((pid_and_base, info)) if pid_and_base == (pid, image_base) => info,
_ => ImageInfoFromMergedEtl::default(),
};
pending_info.image_timestamp = Some(image_timestamp);
pending_image_info = Some(((pid, image_base), pending_info));
}
"KernelTraceControl/ImageID/DbgID_RSDS" => {
let pid = parser.try_parse("ProcessId").unwrap();
Expand All @@ -222,19 +229,44 @@ pub fn profile_pid_from_etl_file(context: &mut ProfileContext, etl_file: &Path)
age,
);
let pdb_path: String = parser.try_parse("PdbFileName").unwrap();
context.handle_image_debug_id(pid, image_base, debug_id, pdb_path);
let mut pending_info = match pending_image_info.take() {
Some((pid_and_base, info)) if pid_and_base == (pid, image_base) => info,
_ => ImageInfoFromMergedEtl::default(),
};
pending_info.debug_id = Some(debug_id);
pending_info.pdb_path = Some(pdb_path);
pending_image_info = Some(((pid, image_base), pending_info));
}
// These events are generated by the kernel logger.
// They are available in unmerged ETL files.
"MSNT_SystemTrace/Image/Load" | "MSNT_SystemTrace/Image/DCStart" => {
// KernelTraceControl/ImageID/ and KernelTraceControl/ImageID/DbgID_RSDS are synthesized from MSNT_SystemTrace/Image/Load
// but don't contain the full path of the binary. We go through a bit of a dance to store the information from those events
// in pending_libraries and deal with it here. We assume that the KernelTraceControl events come before the Image/Load event.

// the ProcessId field doesn't necessarily match s.process_id();
let pid = parser.try_parse("ProcessId").unwrap();
let image_base: u64 = parser.try_parse("ImageBase").unwrap();
let image_size: u64 = parser.try_parse("ImageSize").unwrap();
let image_timestamp_maybe_zero: u32 = parser.try_parse("TimeDateStamp").unwrap(); // zero for MSNT_SystemTrace/Image/DCStart
let image_checksum: u32 = parser.try_parse("ImageChecksum").unwrap();
let path: String = parser.try_parse("FileName").unwrap();
context.handle_image_load(timestamp_raw, pid, image_base, image_size, path);

// Supplement the information from this event with the information from
// KernelTraceControl/ImageID events, if available. Those events come right
// before this one (they were inserted by xperf during merging, if this is
// a merged file).
let info_from_merged_etl = match pending_image_info.take() {
Some((pid_and_base, info)) if pid_and_base == (pid, image_base) => info,
_ => ImageInfoFromMergedEtl::default(),
};

context.handle_image_load(
timestamp_raw,
pid,
image_base,
image_size as u32,
image_timestamp_maybe_zero,
image_checksum,
path,
info_from_merged_etl,
);
}
"MSNT_SystemTrace/Image/UnLoad" => {
// nothing, but we don't want a marker for it
Expand Down
Loading

0 comments on commit 17a9b85

Please sign in to comment.