Merge pull request #291 from mstange/improve-unmerged

Improve handling of unmerged ETL files.

samply/src/windows/etw_gecko.rs

@@ -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();
@@ -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 {
@@ -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();
@@ -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