From 97c8fffc39439180ef971cd9e3a7627fb927cc86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Jul 2025 23:13:22 +0000 Subject: [PATCH 1/6] Initial plan From af3bc1d7d293f785b69a667cbd77f0731f69b801 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Jul 2025 23:23:18 +0000 Subject: [PATCH 2/6] Fix universal symbol conversion overlapping mappings issue Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- src/TraceEvent/TraceLog.cs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/TraceEvent/TraceLog.cs b/src/TraceEvent/TraceLog.cs index 8d5a97a28..c263becf5 100644 --- a/src/TraceEvent/TraceLog.cs +++ b/src/TraceEvent/TraceLog.cs @@ -7111,7 +7111,13 @@ internal TraceModuleFile UniversalMapping(ProcessMappingTraceData data, ProcessM // A loaded and managed modules depend on a module file, so get or create one. // TODO: We'll need to store FileOffset as well to handle elf images. TraceModuleFile moduleFile = process.Log.ModuleFiles.GetOrCreateModuleFile(data.FileName, data.StartAddress); - moduleFile.imageSize = (long)(data.EndAddress - data.StartAddress); + long newImageSize = (long)(data.EndAddress - data.StartAddress); + + // If we found an existing module file, check if we need to expand its size to handle overlapping mappings + if (moduleFile.imageSize < newImageSize) + { + moduleFile.imageSize = newImageSize; + } // Get or create the loaded module. TraceLoadedModule loadedModule = FindModuleAndIndexContainingAddress(data.StartAddress, data.TimeStampQPC, out index); @@ -7125,6 +7131,14 @@ internal TraceModuleFile UniversalMapping(ProcessMappingTraceData data, ProcessM InsertAndSetOverlap(index + 1, loadedModule); } + else + { + // Handle overlapping mappings by updating the loaded module's image size if needed + if (loadedModule.ModuleFile.imageSize < newImageSize) + { + loadedModule.ModuleFile.imageSize = newImageSize; + } + } // Get or create a managed module. This module is the container for dynamic symbols. TraceManagedModule managedModule = FindManagedModuleAndIndex((long)data.StartAddress, data.TimeStampQPC, out index); @@ -8525,15 +8539,21 @@ internal void AddUniversalDynamicSymbol(ProcessSymbolTraceData data, TraceProces { int index; TraceLoadedModule loadedModule = process.LoadedModules.FindModuleAndIndexContainingAddress(data.StartAddress, data.TimeStampQPC, out index); - module = process.LoadedModules.GetOrCreateManagedModule(loadedModule.ModuleID, data.TimeStampQPC); - moduleFileIndex = module.ModuleFile.ModuleFileIndex; - methodIndex = methods.NewMethod(data.Name, moduleFileIndex, (int)data.Id); + if (loadedModule != null) + { + module = process.LoadedModules.GetOrCreateManagedModule(loadedModule.ModuleID, data.TimeStampQPC); + moduleFileIndex = module.ModuleFile.ModuleFileIndex; + methodIndex = methods.NewMethod(data.Name, moduleFileIndex, (int)data.Id); + } // When universal traces support re-use of address space, we'll need to support it here. } - // Set the info - info.SetMethodIndex(this, methodIndex); + // Set the info (only if we found a module) + if (module != null) + { + info.SetMethodIndex(this, methodIndex); + } } }); } From 6caab6b8039bf82da83ca9eb4586ed266759a632 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 6 Jul 2025 00:22:31 +0000 Subject: [PATCH 3/6] Fix overlapping mappings logic and timestamp handling Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- src/TraceEvent/TraceLog.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/TraceEvent/TraceLog.cs b/src/TraceEvent/TraceLog.cs index c263becf5..827360e56 100644 --- a/src/TraceEvent/TraceLog.cs +++ b/src/TraceEvent/TraceLog.cs @@ -7121,13 +7121,13 @@ internal TraceModuleFile UniversalMapping(ProcessMappingTraceData data, ProcessM // Get or create the loaded module. TraceLoadedModule loadedModule = FindModuleAndIndexContainingAddress(data.StartAddress, data.TimeStampQPC, out index); - if (loadedModule == null) + if (loadedModule == null || loadedModule.ImageBase != data.StartAddress) { // The module file is what is used when looking up the module for an arbitrary address, so it must save both the start address and image size. loadedModule = new TraceLoadedModule(process, moduleFile, data.StartAddress); - // All mappings are enumerated at the beginning of the trace. - loadedModule.loadTimeQPC = process.Log.sessionStartTimeQPC; + // Set the timestamp from the mapping data + loadedModule.loadTimeQPC = data.TimeStampQPC; InsertAndSetOverlap(index + 1, loadedModule); } @@ -7138,6 +7138,8 @@ internal TraceModuleFile UniversalMapping(ProcessMappingTraceData data, ProcessM { loadedModule.ModuleFile.imageSize = newImageSize; } + // Update the timestamp to match the new mapping + loadedModule.loadTimeQPC = data.TimeStampQPC; } // Get or create a managed module. This module is the container for dynamic symbols. From 0469cd221e051ecbc30e4e11eb78209fdafb3f6a Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Sat, 5 Jul 2025 18:44:01 -0700 Subject: [PATCH 4/6] Simplify code and add comments. --- src/TraceEvent/TraceLog.cs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/TraceEvent/TraceLog.cs b/src/TraceEvent/TraceLog.cs index 827360e56..e481068e9 100644 --- a/src/TraceEvent/TraceLog.cs +++ b/src/TraceEvent/TraceLog.cs @@ -7109,44 +7109,39 @@ internal TraceModuleFile UniversalMapping(ProcessMappingTraceData data, ProcessM int index; // A loaded and managed modules depend on a module file, so get or create one. + // The key is the file name. For jitted code on Linux, this will be a memfd with a static name, which is OK + // because this path will use the StartAddress to ensure that we get the right one. // TODO: We'll need to store FileOffset as well to handle elf images. TraceModuleFile moduleFile = process.Log.ModuleFiles.GetOrCreateModuleFile(data.FileName, data.StartAddress); long newImageSize = (long)(data.EndAddress - data.StartAddress); - // If we found an existing module file, check if we need to expand its size to handle overlapping mappings + // New mappings will have an imageSize of 0 and will get set. + // Existing mappings that have the same StartAddress but increase in length will get updated here. if (moduleFile.imageSize < newImageSize) { moduleFile.imageSize = newImageSize; } - // Get or create the loaded module. + // The loaded module is looked up by StartAddress and time to ensure that we don't use a module that hasn't been loaded yet. + // If the StartAddress or size don't match, then create a new one. This handles overlapping cases. TraceLoadedModule loadedModule = FindModuleAndIndexContainingAddress(data.StartAddress, data.TimeStampQPC, out index); - if (loadedModule == null || loadedModule.ImageBase != data.StartAddress) - { + if (loadedModule == null || loadedModule.ImageBase != data.StartAddress || loadedModule.ModuleFile.imageSize != newImageSize) + { // The module file is what is used when looking up the module for an arbitrary address, so it must save both the start address and image size. loadedModule = new TraceLoadedModule(process, moduleFile, data.StartAddress); - + // Set the timestamp from the mapping data loadedModule.loadTimeQPC = data.TimeStampQPC; - + InsertAndSetOverlap(index + 1, loadedModule); } - else - { - // Handle overlapping mappings by updating the loaded module's image size if needed - if (loadedModule.ModuleFile.imageSize < newImageSize) - { - loadedModule.ModuleFile.imageSize = newImageSize; - } - // Update the timestamp to match the new mapping - loadedModule.loadTimeQPC = data.TimeStampQPC; - } // Get or create a managed module. This module is the container for dynamic symbols. TraceManagedModule managedModule = FindManagedModuleAndIndex((long)data.StartAddress, data.TimeStampQPC, out index); if (managedModule == null) { managedModule = new TraceManagedModule(process, moduleFile, (long)data.StartAddress); + managedModule.loadTimeQPC = data.TimeStampQPC; modules.Insert(index + 1, managedModule); } From 5a0696b2c699e91750726f99bc8a5f5086388868 Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Sat, 5 Jul 2025 19:04:26 -0700 Subject: [PATCH 5/6] Add assert when LoadedModule isn't found for a symbol. --- src/TraceEvent/TraceLog.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/TraceEvent/TraceLog.cs b/src/TraceEvent/TraceLog.cs index e481068e9..36488bf76 100644 --- a/src/TraceEvent/TraceLog.cs +++ b/src/TraceEvent/TraceLog.cs @@ -8542,6 +8542,12 @@ internal void AddUniversalDynamicSymbol(ProcessSymbolTraceData data, TraceProces moduleFileIndex = module.ModuleFile.ModuleFileIndex; methodIndex = methods.NewMethod(data.Name, moduleFileIndex, (int)data.Id); } + else + { + // We should always get a loadedModule here because if we have a symbol, then we should have a module that contains it. + // Assert so that we can detect bugs here during development. + Debug.Assert(false, "loadedModule is missing for symbol"); + } // When universal traces support re-use of address space, we'll need to support it here. } From 19b8f376bfb764103586a81838d0d1d251e31ba6 Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Thu, 10 Jul 2025 14:07:29 -0700 Subject: [PATCH 6/6] Code review feedback. --- src/TraceEvent/TraceLog.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/TraceEvent/TraceLog.cs b/src/TraceEvent/TraceLog.cs index 36488bf76..1354e6b97 100644 --- a/src/TraceEvent/TraceLog.cs +++ b/src/TraceEvent/TraceLog.cs @@ -8536,18 +8536,17 @@ internal void AddUniversalDynamicSymbol(ProcessSymbolTraceData data, TraceProces { int index; TraceLoadedModule loadedModule = process.LoadedModules.FindModuleAndIndexContainingAddress(data.StartAddress, data.TimeStampQPC, out index); + + // We should always get a loadedModule here because if we have a symbol, then we should have a module that contains it. + // Assert so that we can detect bugs here during development. + Debug.Assert(loadedModule != null, "loadedModule is missing for symbol"); + if (loadedModule != null) { module = process.LoadedModules.GetOrCreateManagedModule(loadedModule.ModuleID, data.TimeStampQPC); moduleFileIndex = module.ModuleFile.ModuleFileIndex; methodIndex = methods.NewMethod(data.Name, moduleFileIndex, (int)data.Id); } - else - { - // We should always get a loadedModule here because if we have a symbol, then we should have a module that contains it. - // Assert so that we can detect bugs here during development. - Debug.Assert(false, "loadedModule is missing for symbol"); - } // When universal traces support re-use of address space, we'll need to support it here. }