diff --git a/src/Compilers/Core/Portable/Compilation/Compilation.cs b/src/Compilers/Core/Portable/Compilation/Compilation.cs index 7dba509dcc2b7..9d5d592c2f0e4 100644 --- a/src/Compilers/Core/Portable/Compilation/Compilation.cs +++ b/src/Compilers/Core/Portable/Compilation/Compilation.cs @@ -3351,8 +3351,6 @@ internal static bool SerializePeToStream( RSAParameters? privateKeyOpt, CancellationToken cancellationToken) { - bool emitSecondaryAssembly = getMetadataPeStreamOpt != null; - bool includePrivateMembersOnPrimaryOutput = metadataOnly ? includePrivateMembers : true; bool deterministicPrimaryOutput = (metadataOnly && !includePrivateMembers) || isDeterministic; if (!Cci.PeWriter.WritePeToStream( @@ -3372,7 +3370,7 @@ internal static bool SerializePeToStream( } // produce the secondary output (ref assembly) if needed - if (emitSecondaryAssembly) + if (getMetadataPeStreamOpt is not null) { Debug.Assert(!metadataOnly); Debug.Assert(!includePrivateMembers); diff --git a/src/Compilers/Core/Portable/Emit/DebugDocumentsBuilder.cs b/src/Compilers/Core/Portable/Emit/DebugDocumentsBuilder.cs index c57bd62d12305..28af028f65c88 100644 --- a/src/Compilers/Core/Portable/Emit/DebugDocumentsBuilder.cs +++ b/src/Compilers/Core/Portable/Emit/DebugDocumentsBuilder.cs @@ -43,7 +43,7 @@ internal void AddDebugDocument(Cci.DebugSourceDocument document) internal IReadOnlyDictionary DebugDocuments => _debugDocuments; - internal Cci.DebugSourceDocument? TryGetDebugDocument(string path, string basePath) + internal Cci.DebugSourceDocument? TryGetDebugDocument(string path, string? basePath) { return TryGetDebugDocumentForNormalizedPath(NormalizeDebugDocumentPath(path, basePath)); } diff --git a/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs index 779e1504df6cd..5585cc6886379 100755 --- a/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs +++ b/src/Compilers/Core/Portable/PEWriter/ExtendedPEBuilder.cs @@ -33,14 +33,14 @@ public ExtendedPEBuilder( PEHeaderBuilder header, MetadataRootBuilder metadataRootBuilder, BlobBuilder ilStream, - BlobBuilder mappedFieldData, - BlobBuilder managedResources, - ResourceSectionBuilder nativeResources, - DebugDirectoryBuilder debugDirectoryBuilder, + BlobBuilder? mappedFieldData, + BlobBuilder? managedResources, + ResourceSectionBuilder? nativeResources, + DebugDirectoryBuilder? debugDirectoryBuilder, int strongNameSignatureSize, MethodDefinitionHandle entryPoint, CorFlags flags, - Func, BlobContentId> deterministicIdProvider, + Func, BlobContentId>? deterministicIdProvider, bool withMvidSection) : base(header, metadataRootBuilder, ilStream, mappedFieldData, managedResources, nativeResources, debugDirectoryBuilder, strongNameSignatureSize, entryPoint, flags, deterministicIdProvider) diff --git a/src/Compilers/Core/Portable/PEWriter/MetadataBuilderExtensions.cs b/src/Compilers/Core/Portable/PEWriter/MetadataBuilderExtensions.cs new file mode 100644 index 0000000000000..72bb63d32ead6 --- /dev/null +++ b/src/Compilers/Core/Portable/PEWriter/MetadataBuilderExtensions.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using Microsoft.Cci; + +namespace Microsoft.CodeAnalysis; + +internal static class MetadataBuilderExtensions +{ + internal static BlobHandle GetOrAddBlobAndFree(this MetadataBuilder metadataBuilder, PooledBlobBuilder builder) + { + var handle = metadataBuilder.GetOrAddBlob(builder); + builder.Free(); + return handle; + } +} diff --git a/src/Compilers/Core/Portable/PEWriter/MetadataWriter.PortablePdb.cs b/src/Compilers/Core/Portable/PEWriter/MetadataWriter.PortablePdb.cs index e1555162b02e0..f4f954d60830f 100644 --- a/src/Compilers/Core/Portable/PEWriter/MetadataWriter.PortablePdb.cs +++ b/src/Compilers/Core/Portable/PEWriter/MetadataWriter.PortablePdb.cs @@ -635,7 +635,7 @@ private BlobHandle SerializeSequencePoints( return default(BlobHandle); } - var writer = new BlobBuilder(); + var writer = PooledBlobBuilder.GetInstance(); int previousNonHiddenStartLine = -1; int previousNonHiddenStartColumn = -1; @@ -699,7 +699,7 @@ private BlobHandle SerializeSequencePoints( previousNonHiddenStartColumn = sequencePoints[i].StartColumn; } - return _debugMetadataOpt.GetOrAddBlob(writer); + return _debugMetadataOpt.GetOrAddBlobAndFree(writer); } private static DebugSourceDocument TryGetSingleDocument(ImmutableArray sequencePoints) @@ -809,38 +809,38 @@ private void SerializeEncMethodDebugInformation(IMethodBody methodBody, MethodDe if (!encInfo.LocalSlots.IsDefaultOrEmpty) { - var writer = new BlobBuilder(); + var writer = PooledBlobBuilder.GetInstance(); encInfo.SerializeLocalSlots(writer); _debugMetadataOpt.AddCustomDebugInformation( parent: method, kind: _debugMetadataOpt.GetOrAddGuid(PortableCustomDebugInfoKinds.EncLocalSlotMap), - value: _debugMetadataOpt.GetOrAddBlob(writer)); + value: _debugMetadataOpt.GetOrAddBlobAndFree(writer)); } if (!encInfo.Lambdas.IsDefaultOrEmpty) { - var writer = new BlobBuilder(); + var writer = PooledBlobBuilder.GetInstance(); encInfo.SerializeLambdaMap(writer); _debugMetadataOpt.AddCustomDebugInformation( parent: method, kind: _debugMetadataOpt.GetOrAddGuid(PortableCustomDebugInfoKinds.EncLambdaAndClosureMap), - value: _debugMetadataOpt.GetOrAddBlob(writer)); + value: _debugMetadataOpt.GetOrAddBlobAndFree(writer)); } if (!encInfo.StateMachineStates.IsDefaultOrEmpty) { - var writer = new BlobBuilder(); + var writer = PooledBlobBuilder.GetInstance(); encInfo.SerializeStateMachineStates(writer); _debugMetadataOpt.AddCustomDebugInformation( parent: method, kind: _debugMetadataOpt.GetOrAddGuid(PortableCustomDebugInfoKinds.EncStateMachineStateMap), - value: _debugMetadataOpt.GetOrAddBlob(writer)); + value: _debugMetadataOpt.GetOrAddBlobAndFree(writer)); } } diff --git a/src/Compilers/Core/Portable/PEWriter/MetadataWriter.cs b/src/Compilers/Core/Portable/PEWriter/MetadataWriter.cs index 381d10f9e0e90..efc9fa1db1573 100644 --- a/src/Compilers/Core/Portable/PEWriter/MetadataWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/MetadataWriter.cs @@ -899,27 +899,6 @@ private EntityHandle GetExportedTypeImplementation(INamespaceTypeReference names : GetAssemblyReferenceHandle(aref); } - private static uint GetManagedResourceOffset(ManagedResource resource, BlobBuilder resourceWriter) - { - if (resource.ExternalFile != null) - { - return resource.Offset; - } - - int result = resourceWriter.Count; - resource.WriteData(resourceWriter); - return (uint)result; - } - - private static uint GetManagedResourceOffset(BlobBuilder resource, BlobBuilder resourceWriter) - { - int result = resourceWriter.Count; - resourceWriter.WriteInt32(resource.Count); - resource.WriteContentTo(resourceWriter); - resourceWriter.Align(8); - return (uint)result; - } - public static string GetMetadataName(INamedTypeReference namedType, int generation) { string nameWithGeneration = (generation == 0) ? namedType.Name : namedType.Name + "#" + generation; @@ -1696,7 +1675,8 @@ internal EntityHandle GetDefinitionHandle(IDefinition definition) }; } - public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStream, Stream ilStream, Stream portablePdbStreamOpt, out MetadataSizes metadataSizes) +#nullable enable + public void WriteMetadataAndIL(PdbWriter? nativePdbWriterOpt, Stream metadataStream, Stream ilStream, Stream? portablePdbStreamOpt, out MetadataSizes metadataSizes) { Debug.Assert(nativePdbWriterOpt == null ^ portablePdbStreamOpt == null); @@ -1705,8 +1685,8 @@ public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStre // TODO: we can precalculate the exact size of IL stream var ilBuilder = new BlobBuilder(1024); var metadataBuilder = new BlobBuilder(4 * 1024); - var mappedFieldDataBuilder = new BlobBuilder(0); - var managedResourceDataBuilder = new BlobBuilder(0); + PooledBlobBuilder? mappedFieldDataBuilder = null; + PooledBlobBuilder? managedResourceDataBuilder = null; // Add 4B of padding to the start of the separated IL stream, // so that method RVAs, which are offsets to this stream, are never 0. @@ -1721,8 +1701,8 @@ public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStre BuildMetadataAndIL( nativePdbWriterOpt, ilBuilder, - mappedFieldDataBuilder, - managedResourceDataBuilder, + out mappedFieldDataBuilder, + out managedResourceDataBuilder, out Blob mvidFixup, out Blob mvidStringFixup); @@ -1731,8 +1711,8 @@ public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStre Debug.Assert(typeSystemRowCounts[(int)TableIndex.EncMap] == 0); PopulateEncTables(typeSystemRowCounts); - Debug.Assert(mappedFieldDataBuilder.Count == 0); - Debug.Assert(managedResourceDataBuilder.Count == 0); + Debug.Assert(mappedFieldDataBuilder == null); + Debug.Assert(managedResourceDataBuilder == null); Debug.Assert(mvidFixup.IsDefault); Debug.Assert(mvidStringFixup.IsDefault); @@ -1740,7 +1720,7 @@ public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStre // https://github.com/dotnet/roslyn/issues/70389 if (!IsFullMetadata) { - metadata.GetType().GetField("_customAttributeTableNeedsSorting", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(metadata, false); + metadata.GetType().GetField("_customAttributeTableNeedsSorting", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(metadata, false); } // TODO (https://github.com/dotnet/roslyn/issues/3905): @@ -1783,10 +1763,10 @@ public void WriteMetadataAndIL(PdbWriter nativePdbWriterOpt, Stream metadataStre } public void BuildMetadataAndIL( - PdbWriter nativePdbWriterOpt, + PdbWriter? nativePdbWriterOpt, BlobBuilder ilBuilder, - BlobBuilder mappedFieldDataBuilder, - BlobBuilder managedResourceDataBuilder, + out PooledBlobBuilder? mappedFieldDataBuilder, + out PooledBlobBuilder? managedResourceDataBuilder, out Blob mvidFixup, out Blob mvidStringFixup) { @@ -1849,15 +1829,17 @@ public void BuildMetadataAndIL( ReportReferencesToAddedSymbols(); - BlobBuilder dynamicAnalysisDataOpt = null; + PooledBlobBuilder? dynamicAnalysisData = null; if (_dynamicAnalysisDataWriterOpt != null) { - dynamicAnalysisDataOpt = new BlobBuilder(); - _dynamicAnalysisDataWriterOpt.SerializeMetadataTables(dynamicAnalysisDataOpt); + dynamicAnalysisData = PooledBlobBuilder.GetInstance(); + _dynamicAnalysisDataWriterOpt.SerializeMetadataTables(dynamicAnalysisData); } - PopulateTypeSystemTables(methodBodyOffsets, mappedFieldDataBuilder, managedResourceDataBuilder, dynamicAnalysisDataOpt, out mvidFixup); + PopulateTypeSystemTables(methodBodyOffsets, out mappedFieldDataBuilder, out managedResourceDataBuilder, dynamicAnalysisData, out mvidFixup); + dynamicAnalysisData?.Free(); } +#nullable disable public virtual void PopulateEncTables(ImmutableArray typeSystemRowCounts) { @@ -1916,7 +1898,8 @@ private ImmutableArray GetSortedGenericParameters() }).ToImmutableArray(); } - private void PopulateTypeSystemTables(int[] methodBodyOffsets, BlobBuilder mappedFieldDataWriter, BlobBuilder resourceWriter, BlobBuilder dynamicAnalysisDataOpt, out Blob mvidFixup) +#nullable enable + private void PopulateTypeSystemTables(int[] methodBodyOffsets, out PooledBlobBuilder? mappedFieldDataWriter, out PooledBlobBuilder? resourceWriter, BlobBuilder? dynamicAnalysisData, out Blob mvidFixup) { var sortedGenericParameters = GetSortedGenericParameters(); @@ -1930,13 +1913,13 @@ private void PopulateTypeSystemTables(int[] methodBodyOffsets, BlobBuilder mappe this.PopulateExportedTypeTableRows(); this.PopulateFieldLayoutTableRows(); this.PopulateFieldMarshalTableRows(); - this.PopulateFieldRvaTableRows(mappedFieldDataWriter); + this.PopulateFieldRvaTableRows(out mappedFieldDataWriter); this.PopulateFieldTableRows(); this.PopulateFileTableRows(); this.PopulateGenericParameters(sortedGenericParameters); this.PopulateImplMapTableRows(); this.PopulateInterfaceImplTableRows(); - this.PopulateManifestResourceTableRows(resourceWriter, dynamicAnalysisDataOpt); + this.PopulateManifestResourceTableRows(out resourceWriter, dynamicAnalysisData); this.PopulateMemberRefTableRows(); this.PopulateMethodImplTableRows(); this.PopulateMethodTableRows(methodBodyOffsets); @@ -1956,6 +1939,7 @@ private void PopulateTypeSystemTables(int[] methodBodyOffsets, BlobBuilder mappe // This table is populated after the others because it depends on the order of the entries of the generic parameter table. this.PopulateCustomAttributeTableRows(sortedGenericParameters); } +#nullable disable private void PopulateAssemblyRefTableRows() { @@ -2336,8 +2320,11 @@ private void PopulateFieldMarshalTableRows() } } - private void PopulateFieldRvaTableRows(BlobBuilder mappedFieldDataWriter) +#nullable enable + private void PopulateFieldRvaTableRows(out PooledBlobBuilder? mappedFieldDataWriter) { + mappedFieldDataWriter = null; + foreach (IFieldDefinition fieldDef in this.GetFieldDefs()) { if (fieldDef.MappedData.IsDefault) @@ -2345,6 +2332,8 @@ private void PopulateFieldRvaTableRows(BlobBuilder mappedFieldDataWriter) continue; } + mappedFieldDataWriter ??= PooledBlobBuilder.GetInstance(); + // The compiler always aligns each RVA data field to an 8-byte boundary; this accomodates the alignment // needs for all primitive types, regardless of which type is actually being used, at the expense of // potentially wasting up to 7 bytes per field if the alignment needs are less. In the future, this @@ -2362,6 +2351,7 @@ private void PopulateFieldRvaTableRows(BlobBuilder mappedFieldDataWriter) offset: offset); } } +#nullable disable private void PopulateFieldTableRows() { @@ -2417,6 +2407,7 @@ private void PopulateConstantTableRows() continue; } + Debug.Assert(propDef.DefaultValue != null); metadata.AddConstant( parent: GetPropertyDefIndex(propDef), value: propDef.DefaultValue.Value); @@ -2506,15 +2497,19 @@ private void PopulateInterfaceImplTableRows() } } - private void PopulateManifestResourceTableRows(BlobBuilder resourceDataWriter, BlobBuilder dynamicAnalysisDataOpt) +#nullable enable + private void PopulateManifestResourceTableRows(out PooledBlobBuilder? resourceDataWriter, BlobBuilder? dynamicAnalysisData) { - if (dynamicAnalysisDataOpt != null) + resourceDataWriter = null; + + if (dynamicAnalysisData != null) { + resourceDataWriter = PooledBlobBuilder.GetInstance(); metadata.AddManifestResource( attributes: ManifestResourceAttributes.Private, name: metadata.GetOrAddString(""), implementation: default(EntityHandle), - offset: GetManagedResourceOffset(dynamicAnalysisDataOpt, resourceDataWriter) + offset: writeBuilderResourceAndGetOffset(dynamicAnalysisData, resourceDataWriter) ); } @@ -2536,12 +2531,35 @@ private void PopulateManifestResourceTableRows(BlobBuilder resourceDataWriter, B attributes: resource.IsPublic ? ManifestResourceAttributes.Public : ManifestResourceAttributes.Private, name: GetStringHandleForNameAndCheckLength(resource.Name), implementation: implementation, - offset: GetManagedResourceOffset(resource, resourceDataWriter)); + offset: writeManagedResourceAndGetOffset(resource, ref resourceDataWriter)); } // the stream should be aligned: - Debug.Assert((resourceDataWriter.Count % ManagedPEBuilder.ManagedResourcesDataAlignment) == 0); + Debug.Assert(resourceDataWriter == null || (resourceDataWriter.Count % ManagedPEBuilder.ManagedResourcesDataAlignment) == 0); + + static uint writeManagedResourceAndGetOffset(ManagedResource resource, ref PooledBlobBuilder? resourceWriter) + { + if (resource.ExternalFile != null) + { + return resource.Offset; + } + + resourceWriter ??= PooledBlobBuilder.GetInstance(); + int result = resourceWriter.Count; + resource.WriteData(resourceWriter); + return (uint)result; + } + + static uint writeBuilderResourceAndGetOffset(BlobBuilder resource, BlobBuilder resourceWriter) + { + int result = resourceWriter.Count; + resourceWriter.WriteInt32(resource.Count); + resource.WriteContentTo(resourceWriter); + resourceWriter.Align(8); + return (uint)result; + } } +#nullable disable private void PopulateMemberRefTableRows() { diff --git a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs index aa657c2b15566..8f5d3984b17b5 100644 --- a/src/Compilers/Core/Portable/PEWriter/PeWriter.cs +++ b/src/Compilers/Core/Portable/PEWriter/PeWriter.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -35,13 +33,57 @@ public PeWritingException(Exception inner) internal static class PeWriter { + internal struct EmitBuilders + { + internal BlobBuilder IlBlobBuilder; + internal PooledBlobBuilder? MappedFieldDataBlobBuilder; + internal PooledBlobBuilder? ManagedResourceBlobBuilder; + internal PooledBlobBuilder? PortableExecutableBlobBuilder; + internal PooledBlobBuilder? PortablePdbBlobBuilder; + + public EmitBuilders() + { + IlBlobBuilder = new BlobBuilder(32 * 1024); + MappedFieldDataBlobBuilder = null; + ManagedResourceBlobBuilder = null; + PortableExecutableBlobBuilder = null; + PortablePdbBlobBuilder = null; + } + + internal void Free() + { + // There is a bug in LinkSuffix / LinkPrefix which causes the ownership to not + // transfer when these have Count of 0. To avoid this problem we should not be + // creating these builders unless we will actually put content into them. + // + // https://github.com/dotnet/runtime/issues/99266 + Debug.Assert(ManagedResourceBlobBuilder == null || ManagedResourceBlobBuilder.Count > 0); + Debug.Assert(MappedFieldDataBlobBuilder == null || MappedFieldDataBlobBuilder.Count > 0); + + if (PortableExecutableBlobBuilder is null) + { + MappedFieldDataBlobBuilder?.Free(); + ManagedResourceBlobBuilder?.Free(); + } + else + { + // Once PortableExecutableBuilder is created it becomes the owner of the + // MappedFieldDataBuilder and ManagedResourceBuilder instances. Freeing + // it is sufficient to free both of them. + PortableExecutableBlobBuilder.Free(); + } + + PortablePdbBlobBuilder?.Free(); + } + } + internal static bool WritePeToStream( EmitContext context, CommonMessageProvider messageProvider, - Func getPeStream, - Func getPortablePdbStreamOpt, - PdbWriter nativePdbWriterOpt, - string pdbPathOpt, + Func getPeStream, + Func? getPortablePdbStreamOpt, + PdbWriter? nativePdbWriterOpt, + string? pdbPathOpt, bool metadataOnly, bool isDeterministic, bool emitTestCoverageData, @@ -64,16 +106,13 @@ internal static bool WritePeToStream( // based on the contents of the generated stream. Debug.Assert(properties.PersistentIdentifier == default(Guid)); - var ilBuilder = new BlobBuilder(32 * 1024); - var mappedFieldDataBuilder = new BlobBuilder(); - var managedResourceBuilder = new BlobBuilder(1024); - + var emitBuilders = new EmitBuilders(); Blob mvidFixup, mvidStringFixup; mdWriter.BuildMetadataAndIL( nativePdbWriterOpt, - ilBuilder, - mappedFieldDataBuilder, - managedResourceBuilder, + emitBuilders.IlBlobBuilder, + out emitBuilders.MappedFieldDataBlobBuilder, + out emitBuilders.ManagedResourceBlobBuilder, out mvidFixup, out mvidStringFixup); @@ -113,9 +152,10 @@ internal static bool WritePeToStream( nativePdbWriterOpt.WriteCompilerVersion(context.Module.CommonCompilation.Language); } - Stream peStream = getPeStream(); + Stream? peStream = getPeStream(); if (peStream == null) { + emitBuilders.Free(); return false; } @@ -155,7 +195,7 @@ internal static bool WritePeToStream( // We need to calculate the PDB checksum, so we may as well use the calculated hash for PDB ID regardless of whether deterministic build is requested. var portablePdbContentHash = default(ImmutableArray); - BlobBuilder portablePdbToEmbed = null; + PooledBlobBuilder? portablePdbToEmbed = null; if (mdWriter.EmitPortableDebugMetadata) { mdWriter.AddRemainingDebugDocuments(mdWriter.Module.DebugDocumentsBuilder.DebugDocuments); @@ -167,25 +207,25 @@ internal static bool WritePeToStream( new Func, BlobContentId>(content => BlobContentId.FromHash(portablePdbContentHash = CryptographicHashProvider.ComputeHash(context.Module.PdbChecksumAlgorithm, content))) : null; - var portablePdbBlob = new BlobBuilder(); + emitBuilders.PortablePdbBlobBuilder = PooledBlobBuilder.GetInstance(zero: true); var portablePdbBuilder = mdWriter.GetPortablePdbBuilder(metadataRootBuilder.Sizes.RowCounts, debugEntryPointHandle, portablePdbIdProvider); - pdbContentId = portablePdbBuilder.Serialize(portablePdbBlob); + pdbContentId = portablePdbBuilder.Serialize(emitBuilders.PortablePdbBlobBuilder); portablePdbVersion = portablePdbBuilder.FormatVersion; if (getPortablePdbStreamOpt == null) { // embed to debug directory: - portablePdbToEmbed = portablePdbBlob; + portablePdbToEmbed = emitBuilders.PortablePdbBlobBuilder; } else { // write to Portable PDB stream: - Stream portablePdbStream = getPortablePdbStreamOpt(); + Stream? portablePdbStream = getPortablePdbStreamOpt(); if (portablePdbStream != null) { try { - portablePdbBlob.WriteContentTo(portablePdbStream); + emitBuilders.PortablePdbBlobBuilder.WriteContentTo(portablePdbStream); } catch (Exception e) when (!(e is OperationCanceledException)) { @@ -195,7 +235,7 @@ internal static bool WritePeToStream( } } - DebugDirectoryBuilder debugDirectoryBuilder; + DebugDirectoryBuilder? debugDirectoryBuilder; if (pdbPathOpt != null || isDeterministic || portablePdbToEmbed != null) { debugDirectoryBuilder = new DebugDirectoryBuilder(); @@ -209,7 +249,7 @@ internal static bool WritePeToStream( // Emit PDB Checksum entry for Portable and Embedded PDBs. The checksum is not as useful when the PDB is embedded, // however it allows the client to efficiently validate a standalone Portable PDB that // has been extracted from Embedded PDB and placed next to the PE file. - debugDirectoryBuilder.AddPdbChecksumEntry(context.Module.PdbChecksumAlgorithm.Name, portablePdbContentHash); + debugDirectoryBuilder.AddPdbChecksumEntry(context.Module.PdbChecksumAlgorithm.Name!, portablePdbContentHash); } } @@ -234,9 +274,9 @@ internal static bool WritePeToStream( var peBuilder = new ExtendedPEBuilder( peHeaderBuilder, metadataRootBuilder, - ilBuilder, - mappedFieldDataBuilder, - managedResourceBuilder, + emitBuilders.IlBlobBuilder, + emitBuilders.MappedFieldDataBlobBuilder, + emitBuilders.ManagedResourceBlobBuilder, CreateNativeResourceSectionSerializer(context.Module), debugDirectoryBuilder, CalculateStrongNameSignatureSize(context.Module, privateKeyOpt), @@ -245,29 +285,36 @@ internal static bool WritePeToStream( peIdProvider, metadataOnly && !context.IncludePrivateMembers); - var peBlob = new BlobBuilder(); - var peContentId = peBuilder.Serialize(peBlob, out Blob mvidSectionFixup); + // This needs to force the backing builder to zero due to the issue writing COFF + // headers. Can remove once this issue is fixed and we've moved to SRM with the + // fix + // https://github.com/dotnet/runtime/issues/99244 + emitBuilders.PortableExecutableBlobBuilder = PooledBlobBuilder.GetInstance(zero: true); + var peContentId = peBuilder.Serialize(emitBuilders.PortableExecutableBlobBuilder, out Blob mvidSectionFixup); PatchModuleVersionIds(mvidFixup, mvidSectionFixup, mvidStringFixup, peContentId.Guid); if (privateKeyOpt != null && corFlags.HasFlag(CorFlags.StrongNameSigned)) { - strongNameProvider.SignBuilder(peBuilder, peBlob, privateKeyOpt.Value); + Debug.Assert(strongNameProvider != null); + strongNameProvider.SignBuilder(peBuilder, emitBuilders.PortableExecutableBlobBuilder, privateKeyOpt.Value); } try { - peBlob.WriteContentTo(peStream); + emitBuilders.PortableExecutableBlobBuilder.WriteContentTo(peStream); } catch (Exception e) when (!(e is OperationCanceledException)) { throw new PeWritingException(e); } + emitBuilders.Free(); return true; } - private static MethodInfo s_calculateChecksumMethod; + private static MethodInfo? s_calculateChecksumMethod; + // internal for testing internal static uint CalculateChecksum(BlobBuilder peBlob, Blob checksumBlob) { @@ -282,7 +329,7 @@ internal static uint CalculateChecksum(BlobBuilder peBlob, Blob checksumBlob) { peBlob, checksumBlob, - }); + })!; } private static void PatchModuleVersionIds(Blob guidFixup, Blob guidSectionFixup, Blob stringFixup, Guid mvid) @@ -318,7 +365,7 @@ private static string PadPdbPath(string path) return path + new string('\0', Math.Max(0, minLength - Encoding.UTF8.GetByteCount(path) - 1)); } - private static ResourceSectionBuilder CreateNativeResourceSectionSerializer(CommonPEModuleBuilder module) + private static ResourceSectionBuilder? CreateNativeResourceSectionSerializer(CommonPEModuleBuilder module) { // Win32 resources are supplied to the compiler in one of two forms, .RES (the output of the resource compiler), // or .OBJ (the output of running cvtres.exe on a .RES file). A .RES file is parsed and processed into diff --git a/src/Compilers/Core/Portable/PEWriter/PooledBlobBuilder.cs b/src/Compilers/Core/Portable/PEWriter/PooledBlobBuilder.cs index 90ecc60b52491..24f5d7c35c5ac 100644 --- a/src/Compilers/Core/Portable/PEWriter/PooledBlobBuilder.cs +++ b/src/Compilers/Core/Portable/PEWriter/PooledBlobBuilder.cs @@ -4,31 +4,51 @@ using System; using System.Reflection.Metadata; -using Microsoft.CodeAnalysis.PooledObjects; +using System.Runtime.CompilerServices; +using System.Threading; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.Cci { internal sealed class PooledBlobBuilder : BlobBuilder, IDisposable { private const int PoolSize = 128; - private const int ChunkSize = 1024; + private const int PoolChunkSize = 1024; - private static readonly ObjectPool s_chunkPool = new ObjectPool(() => new PooledBlobBuilder(ChunkSize), PoolSize); + private static readonly ObjectPool s_chunkPool = new ObjectPool(() => new PooledBlobBuilder(PoolChunkSize), PoolSize); private PooledBlobBuilder(int size) : base(size) { } - public static PooledBlobBuilder GetInstance() + /// + /// Get a new instance of the that has of + /// at least + /// + /// When true force zero out the backing buffer + /// + /// The can be removed when moving to SRM 9.0 if it contains the bug fix for + /// + /// + /// https://github.com/dotnet/runtime/issues/99244 + /// + public static PooledBlobBuilder GetInstance(bool zero = false) { - return s_chunkPool.Allocate(); + var builder = s_chunkPool.Allocate(); + if (zero) + { + builder.WriteBytes(0, builder.ChunkCapacity); + builder.Clear(); + } + return builder; } protected override BlobBuilder AllocateChunk(int minimalSize) { - if (minimalSize <= ChunkSize) + if (minimalSize <= PoolChunkSize) { return s_chunkPool.Allocate(); } @@ -38,7 +58,28 @@ protected override BlobBuilder AllocateChunk(int minimalSize) protected override void FreeChunk() { - s_chunkPool.Free(this); + if (ChunkCapacity != PoolChunkSize) + { + // The invariant of this builder is that it produces BlobBuilder instances that have a + // ChunkCapacity of exactly 1024. Essentially inside AllocateChuck the pool must be able + // to mindlessly allocate a BlobBuilder where ChunkCapacity is at least 1024. + // + // To maintain this the code must verify that the returned BlobBuilder instances have + // a backing array of the appropriate size. This array can shrink in practice through code + // like the following: + // + // var builder = PooledBlobBuilder.GetInstance(); + // builder.LinkSuffix(new BlobBuilder(256)); + // builder.Free(); // calls FreeChunk where ChunkCapacity is 256 + // + // This shouldn't happen much in practice due to convention of how builders are used but + // it is a legal use of the APIs and must be accounted for. + s_chunkPool.ForgetTrackedObject(this); + } + else + { + s_chunkPool.Free(this); + } } public new void Free()