From: Mike McLaughlin Date: Sat, 6 Nov 2021 04:01:22 +0000 (-0700) Subject: SOS crash fixes (#2733) X-Git-Tag: submit/tizen/20220302.040122~18^2^2~20 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4b6be80f369ce3d696fb9e32ab3aaa82adb38924;p=platform%2Fcore%2Fdotnet%2Fdiagnostics.git SOS crash fixes (#2733) * Fix issue https://github.com/dotnet/diagnostics/issues/2695 The exception in the service provider is caused by the recursion of the ImageMappingMemoryService memory service. Remove the recursion by passing the "raw" memory service (the one without any "mapping" memory service wrappers) to the module service to use to get the build id/PE reader, etc. * Fix Windows debugger SOS termination exception * Code review feedback --- diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/Module.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/Module.cs index ab0e2ff38..b614a2fd5 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/Module.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/Module.cs @@ -90,8 +90,16 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation { get { - GetPEInfo(); - return (_flags & Flags.IsPEImage) != 0; + // For Windows targets, we can always assume that all the modules are PEs. + if (Target.OperatingSystem == OSPlatform.Windows) + { + return true; + } + else + { + GetPEInfo(); + return (_flags & Flags.IsPEImage) != 0; + } } } @@ -172,7 +180,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation { if (Target.OperatingSystem == OSPlatform.Windows) { - Stream stream = Target.Services.GetService().CreateMemoryStream(ImageBase, ImageSize); + Stream stream = ModuleService.MemoryService.CreateMemoryStream(ImageBase, ImageSize); PEFile image = new(new StreamAddressSpace(stream), isDataSourceVirtualAddressSpace: true); if (image.IsValid()) { @@ -189,7 +197,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation { try { - Stream stream = Target.Services.GetService().CreateMemoryStream(ImageBase, ImageSize); + Stream stream = ModuleService.MemoryService.CreateMemoryStream(ImageBase, ImageSize); ElfFile elfFile = new(stream, position: ImageBase, leaveOpen: false, isVirtual: true); if (elfFile.Header.IsValid) { diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleService.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleService.cs index 1d85c0deb..253a2be6e 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleService.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleService.cs @@ -39,6 +39,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation const uint VmProtWrite = 0x02; internal protected readonly ITarget Target; + internal protected IMemoryService RawMemoryService; private IMemoryService _memoryService; private ISymbolService _symbolService; private ReadVirtualCache _versionCache; @@ -48,10 +49,11 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation private static readonly byte[] s_versionString = Encoding.ASCII.GetBytes("@(#)Version "); private static readonly int s_versionLength = s_versionString.Length; - public ModuleService(ITarget target) + public ModuleService(ITarget target, IMemoryService rawMemoryService) { Debug.Assert(target != null); Target = target; + RawMemoryService = rawMemoryService; target.OnFlushEvent.Register(() => { _versionCache?.Clear(); @@ -118,7 +120,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation /// module or null IModule IModuleService.GetModuleFromAddress(ulong address) { - Debug.Assert((address & ~MemoryService.SignExtensionMask()) == 0); + Debug.Assert((address & ~RawMemoryService.SignExtensionMask()) == 0); IModule[] modules = GetSortedModules(); int min = 0, max = modules.Length - 1; @@ -129,7 +131,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation IModule module = modules[mid]; ulong start = module.ImageBase; - Debug.Assert((start & ~MemoryService.SignExtensionMask()) == 0); + Debug.Assert((start & ~RawMemoryService.SignExtensionMask()) == 0); ulong end = start + module.ImageSize; if (address >= start && address < end) { @@ -238,7 +240,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation /// PEImage instance or null private PEImage GetPEInfo(bool isVirtual, ulong address, ulong size, ref PdbFileInfo pdbFileInfo, ref Module.Flags flags) { - Stream stream = MemoryService.CreateMemoryStream(address, size); + Stream stream = RawMemoryService.CreateMemoryStream(address, size); try { stream.Position = 0; @@ -521,7 +523,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation /// build id or null internal byte[] GetBuildId(ulong address) { - Stream stream = MemoryService.CreateMemoryStream(); + Stream stream = RawMemoryService.CreateMemoryStream(); byte[] buildId = null; try { @@ -556,7 +558,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation /// version string or null protected string GetVersionString(ulong address) { - Stream stream = MemoryService.CreateMemoryStream(); + Stream stream = RawMemoryService.CreateMemoryStream(); try { if (Target.OperatingSystem == OSPlatform.Linux) @@ -566,7 +568,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation { foreach (ELFProgramHeader programHeader in elfFile.Segments.Select((segment) => segment.Header)) { - uint flags = MemoryService.PointerSize == 8 ? programHeader.Flags : programHeader.Flags32; + uint flags = RawMemoryService.PointerSize == 8 ? programHeader.Flags : programHeader.Flags32; if (programHeader.Type == ELFProgramHeaderType.Load && (flags & (uint)ELFProgramHeaderAttributes.Writable) != 0) { @@ -625,7 +627,8 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation byte[] buffer = new byte[s_versionString.Length]; if (_versionCache == null) { - _versionCache = new ReadVirtualCache(MemoryService); + // We use the possibly mapped memory service to find the version string in case it isn't in the dump. + _versionCache = new ReadVirtualCache(Target.Services.GetService()); } _versionCache.Clear(); @@ -687,11 +690,11 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation else { return string.Equals(Path.GetFileName(module.FileName), moduleName); } - } + } - protected IMemoryService MemoryService => _memoryService ??= Target.Services.GetService(); + internal protected IMemoryService MemoryService => _memoryService ??= Target.Services.GetService(); - protected ISymbolService SymbolService => _symbolService ??= Target.Services.GetService(); + internal protected ISymbolService SymbolService => _symbolService ??= Target.Services.GetService(); /// /// Search memory helper class diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleServiceFromDataReader.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleServiceFromDataReader.cs index 8480ca4e6..7fd2e230f 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleServiceFromDataReader.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/ModuleServiceFromDataReader.cs @@ -62,7 +62,7 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation { ImmutableArray buildId = _moduleService._dataReader.GetBuildId(ImageBase); // If the data reader can't get the build id, it returns a empty (instead of default) immutable array. - _buildId = buildId.IsEmpty ? base.BuildId : buildId; + _buildId = buildId.IsDefaultOrEmpty ? base.BuildId : buildId; } return _buildId; } @@ -122,8 +122,8 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation private readonly IDataReader _dataReader; - public ModuleServiceFromDataReader(ITarget target, IDataReader dataReader) - : base(target) + public ModuleServiceFromDataReader(ITarget target, IMemoryService rawMemoryService, IDataReader dataReader) + : base(target, rawMemoryService) { _dataReader = dataReader; } diff --git a/src/Microsoft.Diagnostics.DebugServices.Implementation/TargetFromDataReader.cs b/src/Microsoft.Diagnostics.DebugServices.Implementation/TargetFromDataReader.cs index 3679c460a..55dbe636d 100644 --- a/src/Microsoft.Diagnostics.DebugServices.Implementation/TargetFromDataReader.cs +++ b/src/Microsoft.Diagnostics.DebugServices.Implementation/TargetFromDataReader.cs @@ -47,10 +47,11 @@ namespace Microsoft.Diagnostics.DebugServices.Implementation } // Add the thread, memory, and module services + IMemoryService rawMemoryService = new MemoryServiceFromDataReader(_dataReader); ServiceProvider.AddServiceFactory(() => new ThreadServiceFromDataReader(this, _dataReader)); - ServiceProvider.AddServiceFactory(() => new ModuleServiceFromDataReader(this, _dataReader)); + ServiceProvider.AddServiceFactory(() => new ModuleServiceFromDataReader(this, rawMemoryService, _dataReader)); ServiceProvider.AddServiceFactory(() => { - IMemoryService memoryService = new MemoryServiceFromDataReader(_dataReader); + IMemoryService memoryService = rawMemoryService; if (IsDump) { memoryService = new ImageMappingMemoryService(this, memoryService); diff --git a/src/SOS/SOS.Extensions/HostServices.cs b/src/SOS/SOS.Extensions/HostServices.cs index 8ca01b708..29572e36b 100644 --- a/src/SOS/SOS.Extensions/HostServices.cs +++ b/src/SOS/SOS.Extensions/HostServices.cs @@ -302,9 +302,16 @@ namespace SOS.Extensions IntPtr self) { Trace.TraceInformation("HostServices.DestroyTarget #{0}", _target != null ? _target.Id : ""); - if (_target != null) + try + { + if (_target != null) + { + DestroyTarget(_target); + } + } + catch (Exception ex) { - DestroyTarget(_target); + Trace.TraceError(ex.ToString()); } } @@ -350,22 +357,29 @@ namespace SOS.Extensions IntPtr self) { Trace.TraceInformation("HostServices.Uninitialize"); - DestroyTarget(self); - - if (DebuggerServices != null) + try { - DebuggerServices.Release(); - DebuggerServices = null; - } + DestroyTarget(self); + + if (DebuggerServices != null) + { + DebuggerServices.Release(); + DebuggerServices = null; + } - // Send shutdown event on exit - OnShutdownEvent.Fire(); + // Send shutdown event on exit + OnShutdownEvent.Fire(); - // Release the host services wrapper - Release(); + // Release the host services wrapper + Release(); - // Clear HostService instance - Instance = null; + // Clear HostService instance + Instance = null; + } + catch (Exception ex) + { + Trace.TraceError(ex.ToString()); + } } #endregion diff --git a/src/SOS/SOS.Extensions/ModuleServiceFromDebuggerServices.cs b/src/SOS/SOS.Extensions/ModuleServiceFromDebuggerServices.cs index 0e91804e1..cbd533e41 100644 --- a/src/SOS/SOS.Extensions/ModuleServiceFromDebuggerServices.cs +++ b/src/SOS/SOS.Extensions/ModuleServiceFromDebuggerServices.cs @@ -133,8 +133,8 @@ namespace SOS.Extensions private readonly DebuggerServices _debuggerServices; - internal ModuleServiceFromDebuggerServices(ITarget target, DebuggerServices debuggerServices) - : base(target) + internal ModuleServiceFromDebuggerServices(ITarget target, IMemoryService rawMemoryService, DebuggerServices debuggerServices) + : base(target, rawMemoryService) { Debug.Assert(debuggerServices != null); _debuggerServices = debuggerServices; diff --git a/src/SOS/SOS.Extensions/TargetFromFromDebuggerServices.cs b/src/SOS/SOS.Extensions/TargetFromFromDebuggerServices.cs index 3a44ba2f8..3ce6225da 100644 --- a/src/SOS/SOS.Extensions/TargetFromFromDebuggerServices.cs +++ b/src/SOS/SOS.Extensions/TargetFromFromDebuggerServices.cs @@ -72,18 +72,19 @@ namespace SOS.Extensions } // Add the thread, memory, and module services - ServiceProvider.AddServiceFactory(() => new ModuleServiceFromDebuggerServices(this, debuggerServices)); + IMemoryService rawMemoryService = new MemoryServiceFromDebuggerServices(this, debuggerServices); + ServiceProvider.AddServiceFactory(() => new ModuleServiceFromDebuggerServices(this, rawMemoryService, debuggerServices)); ServiceProvider.AddServiceFactory(() => new ThreadServiceFromDebuggerServices(this, debuggerServices)); ServiceProvider.AddServiceFactory(() => { - IMemoryService memoryService = new MemoryServiceFromDebuggerServices(this, debuggerServices); Debug.Assert(Host.HostType != HostType.DotnetDump); + IMemoryService memoryService = rawMemoryService; if (IsDump && Host.HostType == HostType.Lldb) { // This is a special memory service that maps the managed assemblies' metadata into the address // space. The lldb debugger returns zero's (instead of failing the memory read) for missing pages // in core dumps that older (< 5.0) createdumps generate so it needs this special metadata mapping // memory service. dotnet-dump needs this logic for clrstack -i (uses ICorDebug data targets). - memoryService = new MetadataMappingMemoryService(this, memoryService); + return new MetadataMappingMemoryService(this, memoryService); } return memoryService; });