-
Notifications
You must be signed in to change notification settings - Fork 264
Description
There is a code path in the Minidump constructor that passes displayName to the CachedMemoryReader's dumpPath parameter:
| memoryReader = new CachedMemoryReader(segments, displayName, fs, cacheSize, technology, PointerSize, leaveOpen); |
The CachedMemoryReader constructor does:
clrmd/src/Microsoft.Diagnostics.Runtime/Windows/CachedMemoryReader.cs
Lines 37 to 40 in bfbcda6
| public CachedMemoryReader(ImmutableArray<MinidumpSegment> segments, string dumpPath, FileStream stream, long maxCacheSize, CacheTechnology cacheTechnology, int pointerSize, bool leaveOpen) | |
| { | |
| PointerSize = pointerSize; | |
| DumpPath = dumpPath; |
and importantly, has this method, which expects DumpPath to actually be a valid file path:
clrmd/src/Microsoft.Diagnostics.Runtime/Windows/CachedMemoryReader.cs
Lines 93 to 101 in bfbcda6
| public override int ReadFromRva(ulong rva, Span<byte> buffer) | |
| { | |
| lock (_rvaLock) | |
| { | |
| _rvaStream ??= File.OpenRead(DumpPath); | |
| _rvaStream.Position = (long)rva; | |
| return _rvaStream.Read(buffer); | |
| } | |
| } |
However, this breaks the data contract from DataTarget.LoadDump:
clrmd/src/Microsoft.Diagnostics.Runtime/DataTarget.cs
Lines 261 to 269 in bfbcda6
| /// <summary> | |
| /// Loads a dump stream. Currently supported formats are Mach-O coredump, ELF coredump, and Windows Minidump formats. | |
| /// </summary> | |
| /// <param name="displayName">The name of this DataTarget, might be used in exceptions.</param> | |
| /// <param name="stream">The stream that should be used.</param> | |
| /// <param name="leaveOpen">True whenever the given stream should be leaved open when the DataTarget is disposed.</param> | |
| /// <param name="options">The options to use when loading the dump.</param> | |
| /// <returns>A <see cref="DataTarget"/> for the given dump.</returns> | |
| public static DataTarget LoadDump(string displayName, Stream stream, bool leaveOpen = false, DataTargetOptions? options = null) |
where the caller is allowed to pass in a Stream and a display name of their own choice (which, importantly, is not specified to have to be a file path).
The Minidump constructor should maybe pass fs.Name instead to the CachedMemoryReader? Or, the API contract must be changed.