From bfb8c3e9025f4e31409626925a2ecf5e17ca5157 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Fri, 6 Sep 2024 10:27:26 -0700 Subject: [PATCH] Add PDZ indexing to symbol store (#4907) Change debug asserts to release argument exceptions for better parameter validation --------- Co-authored-by: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> --- documentation/symbols/SSQP_Key_Conventions.md | 19 +++++++++ .../StreamAddressSpace.cs | 5 ++- .../KeyGenerators/ELFFileKeyGenerator.cs | 4 +- .../KeyGenerators/KeyGenerator.cs | 41 +++++++++++++++++-- .../KeyGenerators/MachOKeyGenerator.cs | 5 ++- .../KeyGenerators/PDBFileKeyGenerator.cs | 37 ++++++++++++----- .../KeyGenerators/PEFileKeyGenerator.cs | 4 +- .../KeyGenerators/PerfMapFileKeyGenerator.cs | 3 +- .../PortablePDBFileKeyGenerator.cs | 4 +- .../KeyGenerators/SourceFileKeyGenerator.cs | 3 +- src/Microsoft.SymbolStore/SymbolStoreFile.cs | 12 ++++-- src/Microsoft.SymbolStore/SymbolStoreKey.cs | 22 ++++++---- .../Microsoft.FileFormats.UnitTests.csproj | 8 ++-- .../KeyGeneratorTests.cs | 25 +++++++---- .../Microsoft.SymbolStore.UnitTests.csproj | 3 ++ 15 files changed, 144 insertions(+), 51 deletions(-) diff --git a/documentation/symbols/SSQP_Key_Conventions.md b/documentation/symbols/SSQP_Key_Conventions.md index 34d5e4f753..e5a7205474 100644 --- a/documentation/symbols/SSQP_Key_Conventions.md +++ b/documentation/symbols/SSQP_Key_Conventions.md @@ -49,6 +49,25 @@ Example: **Lookup key**: `foo.pdb/497b72f6390a44fc878e5a2d63b6cc4b1/foo.pdb` +### PDZ-Signature-Age + +This applies to the Microsoft C++ Symbol Format with compressed streams, known as PDZ or msfz, also commonly saved with the pdb extension. + +Like regular C++ PDBs, the key also uses values extracted from the GUID stream which is uncompressed. Additionally, the index contains a marker for the type ('msfz') and version (currently only '0'): + +`//msfz/` + +Example: + +**File name:** `Foo.pdb` + +**Signature field:** `{ 0x497B72F6, 0x390A, 0x44FC, { 0x87, 0x8E, 0x5A, 0x2D, 0x63, 0xB6, 0xCC, 0x4B } }` + +**Age field:** `0x1` + +**Format version:** `0` + +**Lookup key**: `foo.pdb/497b72f6390a44fc878e5a2d63b6cc4b1/msfz0/foo.pdb` ### Portable-Pdb-Signature diff --git a/src/Microsoft.FileFormats/StreamAddressSpace.cs b/src/Microsoft.FileFormats/StreamAddressSpace.cs index 773c411dd3..278a2115e4 100644 --- a/src/Microsoft.FileFormats/StreamAddressSpace.cs +++ b/src/Microsoft.FileFormats/StreamAddressSpace.cs @@ -18,7 +18,10 @@ public sealed class StreamAddressSpace : IAddressSpace, IDisposable public StreamAddressSpace(Stream stream) { - System.Diagnostics.Debug.Assert(stream.CanSeek); + if (stream is null || !stream.CanSeek) + { + throw new ArgumentException("Stream null or not seekable", nameof(stream)); + } _stream = stream; Length = (ulong)stream.Length; } diff --git a/src/Microsoft.SymbolStore/KeyGenerators/ELFFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/ELFFileKeyGenerator.cs index 4dd81a3f21..c7de8cc2c3 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/ELFFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/ELFFileKeyGenerator.cs @@ -38,8 +38,8 @@ public class ELFFileKeyGenerator : KeyGenerator public ELFFileKeyGenerator(ITracer tracer, ELFFile elfFile, string path) : base(tracer) { - _elfFile = elfFile; - _path = path; + _elfFile = elfFile ?? throw new ArgumentNullException(nameof(elfFile)); + _path = path ?? throw new ArgumentNullException(nameof(path)); } public ELFFileKeyGenerator(ITracer tracer, SymbolStoreFile file) diff --git a/src/Microsoft.SymbolStore/KeyGenerators/KeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/KeyGenerator.cs index e2f51f2a9b..03307b909a 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/KeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/KeyGenerator.cs @@ -118,7 +118,7 @@ public virtual bool IsDump() protected static SymbolStoreKey BuildKey(string path, string id, bool clrSpecialFile = false, IEnumerable pdbChecksums = null) { string file = GetFileName(path).ToLowerInvariant(); - return BuildKey(path, null, id, file, clrSpecialFile, pdbChecksums); + return BuildKey(path, prefix: null, id, type: null, file, clrSpecialFile, pdbChecksums); } /// @@ -148,7 +148,7 @@ protected static SymbolStoreKey BuildKey(string path, string prefix, byte[] id, /// key protected static SymbolStoreKey BuildKey(string path, string prefix, byte[] id, string file, bool clrSpecialFile = false, IEnumerable pdbChecksums = null) { - return BuildKey(path, prefix, ToHexString(id), file, clrSpecialFile, pdbChecksums); + return BuildKey(path, prefix, ToHexString(id), type: null, file, clrSpecialFile, pdbChecksums); } /// @@ -163,15 +163,44 @@ protected static SymbolStoreKey BuildKey(string path, string prefix, byte[] id, /// key protected static SymbolStoreKey BuildKey(string path, string prefix, string id, string file, bool clrSpecialFile = false, IEnumerable pdbChecksums = null) { + return BuildKey(path, prefix, id, type: null, file, clrSpecialFile, pdbChecksums); + } + + /// + /// Base key building helper for "file/{prefix}-id/{type}/file" indexes. + /// + /// full path of file or binary + /// optional id prefix + /// build id or uuid + /// optional type or format piece + /// file name only + /// if true, the file is one the clr special files + /// Checksums of pdb file. May be null. + /// key + internal static SymbolStoreKey BuildKey(string path, string prefix, string id, string type, string file, bool clrSpecialFile, IEnumerable pdbChecksums) + { + if (id is null or "") + { + throw new ArgumentNullException(nameof(id)); + } + if (file is null or "") + { + throw new ArgumentNullException(nameof(file)); + } StringBuilder key = new(); key.Append(file); key.Append('/'); - if (prefix != null) + if (prefix is not null) { key.Append(prefix); key.Append('-'); } key.Append(id); + if (type is not null) + { + key.Append('/'); + key.Append(type); + } key.Append('/'); key.Append(file); return new SymbolStoreKey(key.ToString(), path, clrSpecialFile, pdbChecksums); @@ -181,7 +210,7 @@ protected static SymbolStoreKey BuildKey(string path, string prefix, string id, /// hex string public static string ToHexString(byte[] bytes) { - if (bytes == null) + if (bytes is null) { throw new ArgumentNullException(nameof(bytes)); } @@ -196,6 +225,10 @@ public static string ToHexString(byte[] bytes) /// just the file name internal static string GetFileName(string path) { + if (path is null or "") + { + throw new ArgumentNullException(nameof(path)); + } return Path.GetFileName(path.Replace('\\', '/')); } } diff --git a/src/Microsoft.SymbolStore/KeyGenerators/MachOKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/MachOKeyGenerator.cs index 02a89b9538..ae4f9106d1 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/MachOKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/MachOKeyGenerator.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -33,8 +34,8 @@ public class MachOFileKeyGenerator : KeyGenerator public MachOFileKeyGenerator(ITracer tracer, MachOFile machoFile, string path) : base(tracer) { - _machoFile = machoFile; - _path = path; + _machoFile = machoFile ?? throw new ArgumentNullException(nameof(machoFile)); + _path = path ?? throw new ArgumentNullException(nameof(path)); } public MachOFileKeyGenerator(ITracer tracer, SymbolStoreFile file) diff --git a/src/Microsoft.SymbolStore/KeyGenerators/PDBFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/PDBFileKeyGenerator.cs index 79aa08ce8b..1e0e627650 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/PDBFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/PDBFileKeyGenerator.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.IO; using Microsoft.FileFormats; using Microsoft.FileFormats.PDB; using Microsoft.FileFormats.PE; @@ -18,6 +19,10 @@ public class PDBFileKeyGenerator : KeyGenerator public PDBFileKeyGenerator(ITracer tracer, SymbolStoreFile file) : base(tracer) { + if (file is null) + { + throw new ArgumentNullException(nameof(file)); + } StreamAddressSpace dataSource = new(file.Stream); _pdbFile = new PDBFile(dataSource); _path = file.FileName; @@ -34,14 +39,10 @@ public override IEnumerable GetKeys(KeyTypeFlags flags) { if ((flags & KeyTypeFlags.IdentityKey) != 0) { - if (_pdbFile.DbiStream.IsValid()) - { - yield return GetKey(_path, _pdbFile.Signature, unchecked((int)_pdbFile.DbiAge)); - } - else - { - yield return GetKey(_path, _pdbFile.Signature, unchecked((int)_pdbFile.Age)); - } + uint age = _pdbFile.DbiStream.IsValid() ? _pdbFile.DbiAge : _pdbFile.Age; + // No format type if legacy Windows PDB (MSF), otherwise, pass container type string (i.e. msfz0) + string type = _pdbFile.ContainerKind == PDBContainerKind.MSF ? null : _pdbFile.ContainerKindSpecString; + yield return GetKey(_path, _pdbFile.Signature, unchecked((int)age), type, pdbChecksums: null); } } } @@ -52,12 +53,26 @@ public override IEnumerable GetKeys(KeyTypeFlags flags) /// file name and path /// mvid guid /// pdb age + /// Checksums of pdb file. May be null. /// symbol store key public static SymbolStoreKey GetKey(string path, Guid signature, int age, IEnumerable pdbChecksums = null) { - Debug.Assert(path != null); - Debug.Assert(signature != null); - return BuildKey(path, string.Format("{0}{1:x}", signature.ToString("N"), age)); + return GetKey(path, signature, age, type: null, pdbChecksums); + } + + /// + /// Create a symbol store key for a Windows PDB or PDZ. + /// + /// file name and path + /// mvid guid + /// pdb age + /// PDB format type like msfz0 or null + /// Checksums of pdb file. May be null. + /// symbol store key + public static SymbolStoreKey GetKey(string path, Guid signature, int age, string type, IEnumerable pdbChecksums = null) + { + string file = GetFileName(path).ToLowerInvariant(); + return BuildKey(path, prefix: null, string.Format("{0}{1:x}", signature.ToString("N"), age), type, file, clrSpecialFile: false, pdbChecksums); } } } diff --git a/src/Microsoft.SymbolStore/KeyGenerators/PEFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/PEFileKeyGenerator.cs index ed8b2190e6..5ccfd355a3 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/PEFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/PEFileKeyGenerator.cs @@ -29,8 +29,8 @@ public class PEFileKeyGenerator : KeyGenerator public PEFileKeyGenerator(ITracer tracer, PEFile peFile, string path) : base(tracer) { - _peFile = peFile; - _path = path; + _peFile = peFile ?? throw new ArgumentNullException(nameof(peFile)); + _path = path ?? throw new ArgumentNullException(nameof(path)); } public PEFileKeyGenerator(ITracer tracer, SymbolStoreFile file) diff --git a/src/Microsoft.SymbolStore/KeyGenerators/PerfMapFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/PerfMapFileKeyGenerator.cs index 9ecc355fbf..f7efd4fab6 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/PerfMapFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/PerfMapFileKeyGenerator.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -16,7 +17,7 @@ public class PerfMapFileKeyGenerator : KeyGenerator public PerfMapFileKeyGenerator(ITracer tracer, SymbolStoreFile file) : base(tracer) { - _file = file; + _file = file ?? throw new ArgumentNullException(nameof(file)); _perfmapFile = new PerfMapFile(_file.Stream); } diff --git a/src/Microsoft.SymbolStore/KeyGenerators/PortablePDBFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/PortablePDBFileKeyGenerator.cs index 7bc3536ed5..c11fb42b4e 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/PortablePDBFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/PortablePDBFileKeyGenerator.cs @@ -16,7 +16,7 @@ public class PortablePDBFileKeyGenerator : KeyGenerator public PortablePDBFileKeyGenerator(ITracer tracer, SymbolStoreFile file) : base(tracer) { - _file = file; + _file = file ?? throw new ArgumentNullException(nameof(file)); } public override bool IsValid() @@ -56,7 +56,7 @@ public override IEnumerable GetKeys(KeyTypeFlags flags) else { // Force the Windows PDB index - key = PDBFileKeyGenerator.GetKey(_file.FileName, blob.Guid, 1); + key = PDBFileKeyGenerator.GetKey(_file.FileName, blob.Guid, age: 1); } } } diff --git a/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs index c9095b8ace..cb0631149e 100644 --- a/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs +++ b/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; using System.Diagnostics; using System.Security.Cryptography; @@ -14,7 +15,7 @@ public class SourceFileKeyGenerator : KeyGenerator public SourceFileKeyGenerator(ITracer tracer, SymbolStoreFile file) : base(tracer) { - _file = file; + _file = file ?? throw new ArgumentNullException(nameof(file)); } public override bool IsValid() diff --git a/src/Microsoft.SymbolStore/SymbolStoreFile.cs b/src/Microsoft.SymbolStore/SymbolStoreFile.cs index 3c6493d098..2e4f01e518 100644 --- a/src/Microsoft.SymbolStore/SymbolStoreFile.cs +++ b/src/Microsoft.SymbolStore/SymbolStoreFile.cs @@ -35,10 +35,14 @@ public sealed class SymbolStoreFile : IDisposable /// name of the file public SymbolStoreFile(Stream stream, string fileName) { - Debug.Assert(stream != null); - Debug.Assert(stream.CanSeek); - Debug.Assert(fileName != null); - + if (stream is null || !stream.CanSeek) + { + throw new ArgumentException("Stream null or not seekable", nameof(stream)); + } + if (fileName is null or "") + { + throw new ArgumentNullException(nameof(fileName)); + } Stream = stream; FileName = fileName; } diff --git a/src/Microsoft.SymbolStore/SymbolStoreKey.cs b/src/Microsoft.SymbolStore/SymbolStoreKey.cs index 8cb60f71ff..a7f57ff564 100644 --- a/src/Microsoft.SymbolStore/SymbolStoreKey.cs +++ b/src/Microsoft.SymbolStore/SymbolStoreKey.cs @@ -50,17 +50,17 @@ public sealed class SymbolStoreKey /// if true, the file is one the clr special files public SymbolStoreKey(string index, string fullPathName, bool clrSpecialFile = false, IEnumerable pdbChecksums = null) { - Debug.Assert(index != null && fullPathName != null); - Index = index; - FullPathName = fullPathName; + Index = index ?? throw new ArgumentNullException(nameof(index)); + FullPathName = fullPathName ?? throw new ArgumentNullException(nameof(fullPathName)); IsClrSpecialFile = clrSpecialFile; PdbChecksums = pdbChecksums ?? Enumerable.Empty(); } /// - /// Returns the first two parts of the index tuple. Allows a different file name + /// Returns the first two or three parts of the index. Allows a different file name /// to be appended to this symbol key. Includes the trailing "/". /// + [Obsolete] public string IndexPrefix { get { return Index.Substring(0, Index.LastIndexOf("/") + 1); } @@ -98,24 +98,28 @@ public override bool Equals(object obj) public static bool IsKeyValid(string index) { string[] parts = index.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries); - if (parts.Length != 3) { + if (parts.Length < 3 || parts.Length > 4) + { return false; } - for (int i = 0; i < 3; i++) + for (int i = 0; i < parts.Length; i++) { foreach (char c in parts[i]) { - if (char.IsLetterOrDigit(c)) { + if (char.IsLetterOrDigit(c)) + { continue; } - if (!s_invalidChars.Contains(c)) { + if (!s_invalidChars.Contains(c)) + { continue; } return false; } // We need to support files with . in the name, but we don't want identifiers that // are meaningful to the filesystem - if (parts[i] == "." || parts[i] == "..") { + if (parts[i] == "." || parts[i] == "..") + { return false; } } diff --git a/src/tests/Microsoft.FileFormats.UnitTests/Microsoft.FileFormats.UnitTests.csproj b/src/tests/Microsoft.FileFormats.UnitTests/Microsoft.FileFormats.UnitTests.csproj index f46b5d9310..59a73891df 100644 --- a/src/tests/Microsoft.FileFormats.UnitTests/Microsoft.FileFormats.UnitTests.csproj +++ b/src/tests/Microsoft.FileFormats.UnitTests/Microsoft.FileFormats.UnitTests.csproj @@ -23,6 +23,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest @@ -60,9 +63,4 @@ - - - Always - - diff --git a/src/tests/Microsoft.SymbolStore.UnitTests/KeyGeneratorTests.cs b/src/tests/Microsoft.SymbolStore.UnitTests/KeyGeneratorTests.cs index 9da934c6b7..85509b5b96 100644 --- a/src/tests/Microsoft.SymbolStore.UnitTests/KeyGeneratorTests.cs +++ b/src/tests/Microsoft.SymbolStore.UnitTests/KeyGeneratorTests.cs @@ -28,7 +28,8 @@ public void FileKeyGenerator() //MachCoreKeyGeneratorInternal(fileGenerator: true); MachOFileKeyGeneratorInternal(fileGenerator: true); MinidumpKeyGeneratorInternal(fileGenerator: true); - PDBFileKeyGeneratorInternal(fileGenerator: true); + PDBFileKeyGeneratorInternal(fileGenerator: true, pdz: false); + PDBFileKeyGeneratorInternal(fileGenerator: true, pdz: true); PEFileKeyGeneratorInternal(fileGenerator: true); PortablePDBFileKeyGeneratorInternal(fileGenerator: true); PerfMapFileKeyGeneratorInternal(fileGenerator: true); @@ -348,20 +349,30 @@ private void MinidumpKeyGeneratorInternal(bool fileGenerator) [Fact] public void PDBFileKeyGenerator() { - PDBFileKeyGeneratorInternal(fileGenerator: false); + PDBFileKeyGeneratorInternal(fileGenerator: false, pdz: false); } - private void PDBFileKeyGeneratorInternal(bool fileGenerator) + [Fact] + public void PDZFileKeyGenerator() { - const string TestBinary = "TestBinaries/HelloWorld.pdb"; - using (Stream pdb = File.OpenRead(TestBinary)) + PDBFileKeyGeneratorInternal(fileGenerator: false, pdz: true); + } + + private void PDBFileKeyGeneratorInternal(bool fileGenerator, bool pdz) + { + using (Stream pdb = File.OpenRead(pdz ? "TestBinaries/HelloWorld.pdz" : "TestBinaries/HelloWorld.pdb")) { - var file = new SymbolStoreFile(pdb, TestBinary); + var file = new SymbolStoreFile(pdb, "TestBinaries/HelloWorld.pdb"); KeyGenerator generator = fileGenerator ? (KeyGenerator)new FileKeyGenerator(_tracer, file) : new PDBFileKeyGenerator(_tracer, file); IEnumerable identityKey = generator.GetKeys(KeyTypeFlags.IdentityKey); Assert.True(identityKey.Count() == 1); - Assert.True(identityKey.First().Index == "helloworld.pdb/99891b3ed7ae4c3babff8a2b4a9b0c431/helloworld.pdb"); + + string index = pdz ? + "helloworld.pdb/99891b3ed7ae4c3babff8a2b4a9b0c431/msfz0/helloworld.pdb" : + "helloworld.pdb/99891b3ed7ae4c3babff8a2b4a9b0c431/helloworld.pdb"; + + Assert.True(identityKey.First().Index == index); IEnumerable symbolKey = generator.GetKeys(KeyTypeFlags.SymbolKey); Assert.True(symbolKey.Count() == 0); diff --git a/src/tests/Microsoft.SymbolStore.UnitTests/Microsoft.SymbolStore.UnitTests.csproj b/src/tests/Microsoft.SymbolStore.UnitTests/Microsoft.SymbolStore.UnitTests.csproj index a603d622ca..04396b193c 100644 --- a/src/tests/Microsoft.SymbolStore.UnitTests/Microsoft.SymbolStore.UnitTests.csproj +++ b/src/tests/Microsoft.SymbolStore.UnitTests/Microsoft.SymbolStore.UnitTests.csproj @@ -55,6 +55,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest