Resources should not be emitted into ref assemblies (#31244)

docs/compilers/CSharp/Compiler Breaking Changes - VS2019.md

@@ -78,3 +78,6 @@ Each entry should include a short description of the break, followed by either a
         if (x is C<T>) { } // warning: the given expression is never of the provided ('C<T>') type.
     }
   ```
+
+10. Previously, reference assemblies were emitted including embedded resources. In Visual Studio 2019, embedded resources are no longer emitted into ref assemblies.
+  See https://github.com/dotnet/roslyn/issues/31197

docs/compilers/Visual Basic/Compiler Breaking Changes - VS2019.md

@@ -0,0 +1,9 @@
+**This document lists known breaking changes in Roslyn 3.0 (Visual Studio 2019) from Roslyn 2.* (Visual Studio 2017)
+
+<!--
+*Breaks are formatted with a monotonically increasing numbered list to allow them to referenced via shorthand (i.e., "known break #1").
+Each entry should include a short description of the break, followed by either a link to the issue describing the full details of the break or the full details of the break inline.*
+-->
+
+1. Previously, reference assemblies were emitted including embedded resources. In Visual Studio 2019, embedded resources are no longer emitted into ref assemblies.
+  See https://github.com/dotnet/roslyn/issues/31197

docs/features/refout.md

@@ -25,6 +25,7 @@ Ref assemblies further remove metadata (private members) from metadata-only asse
 - But all types (including private or nested types) are kept in ref assemblies. All attributes are kept (even internal ones).
 - All virtual methods are kept. Explicit interface implementations are kept. Explicitly-implemented properties and events are kept, as their accessors are virtual (and are therefore kept).
 - All fields of a struct are kept. (This is a candidate for post-C#-7.1 refinement)
+- Any resources included on the command-line are not emitted into ref assemblies (produced either with `/refout` or `/refonly`). (This was fixed in dev16)
 
 ## API changes
 

src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs

@@ -18,6 +18,9 @@ internal abstract class PEAssemblyBuilderBase : PEModuleBuilder, Cci.IAssemblyRe
         private readonly ImmutableArray<NamedTypeSymbol> _additionalTypes;
         private ImmutableArray<Cci.IFileReference> _lazyFiles;
 
+        /// <summary>This is a cache of a subset of <seealso cref="_lazyFiles"/>. We don't include manifest resources in ref assemblies</summary>
+        private ImmutableArray<Cci.IFileReference> _lazyFilesWithoutManifestResources;
+
         private SynthesizedEmbeddedAttributeSymbol _lazyEmbeddedAttribute;
         private SynthesizedEmbeddedAttributeSymbol _lazyIsReadOnlyAttribute;
         private SynthesizedEmbeddedAttributeSymbol _lazyIsByRefLikeAttribute;
@@ -103,41 +106,54 @@ internal override ImmutableArray<NamedTypeSymbol> GetEmbeddedTypes(DiagnosticBag
 
         public sealed override IEnumerable<Cci.IFileReference> GetFiles(EmitContext context)
         {
-            if (_lazyFiles.IsDefault)
+            if (!context.IsRefAssembly)
             {
-                var builder = ArrayBuilder<Cci.IFileReference>.GetInstance();
-                try
+                return getFiles(ref _lazyFiles);
+            }
+            return getFiles(ref _lazyFilesWithoutManifestResources);
+
+            ImmutableArray<Cci.IFileReference> getFiles(ref ImmutableArray<Cci.IFileReference> lazyFiles)
+            {
+                if (lazyFiles.IsDefault)
                 {
-                    var modules = _sourceAssembly.Modules;
-                    for (int i = 1; i < modules.Length; i++)
+                    var builder = ArrayBuilder<Cci.IFileReference>.GetInstance();
+                    try
                     {
-                        builder.Add((Cci.IFileReference)Translate(modules[i], context.Diagnostics));
-                    }
+                        var modules = _sourceAssembly.Modules;
+                        for (int i = 1; i < modules.Length; i++)
+                        {
+                            builder.Add((Cci.IFileReference)Translate(modules[i], context.Diagnostics));
+                        }
 
-                    foreach (ResourceDescription resource in ManifestResources)
-                    {
-                        if (!resource.IsEmbedded)
+                        if (!context.IsRefAssembly)
                         {
-                            builder.Add(resource);
+                            // resources are not emitted into ref assemblies
+                            foreach (ResourceDescription resource in ManifestResources)
+                            {
+                                if (!resource.IsEmbedded)
+                                {
+                                    builder.Add(resource);
+                                }
+                            }
                         }
-                    }
 
-                    // Dev12 compilers don't report ERR_CryptoHashFailed if there are no files to be hashed.
-                    if (ImmutableInterlocked.InterlockedInitialize(ref _lazyFiles, builder.ToImmutable()) && _lazyFiles.Length > 0)
-                    {
-                        if (!CryptographicHashProvider.IsSupportedAlgorithm(_sourceAssembly.HashAlgorithm))
+                        // Dev12 compilers don't report ERR_CryptoHashFailed if there are no files to be hashed.
+                        if (ImmutableInterlocked.InterlockedInitialize(ref lazyFiles, builder.ToImmutable()) && lazyFiles.Length > 0)
                         {
-                            context.Diagnostics.Add(new CSDiagnostic(new CSDiagnosticInfo(ErrorCode.ERR_CryptoHashFailed), NoLocation.Singleton));
+                            if (!CryptographicHashProvider.IsSupportedAlgorithm(_sourceAssembly.HashAlgorithm))
+                            {
+                                context.Diagnostics.Add(new CSDiagnostic(new CSDiagnosticInfo(ErrorCode.ERR_CryptoHashFailed), NoLocation.Singleton));
+                            }
                         }
                     }
+                    finally
+                    {
+                        builder.Free();
+                    }
                 }
-                finally
-                {
-                    builder.Free();
-                }
-            }
 
-            return _lazyFiles;
+                return lazyFiles;
+            }
         }
 
         protected override void AddEmbeddedResourcesFromAddedModules(ArrayBuilder<Cci.ManagedResource> builder, DiagnosticBag diagnostics)

src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
@@ -814,7 +815,7 @@ public enum Match
         /// Are the metadata-only assemblies identical with two source code modifications?
         /// Metadata-only assemblies can either include private/internal members or not.
         /// </summary>
-        private void CompareAssemblies(string sourceTemplate, string change1, string change2, Match expectedMatch, bool includePrivateMembers)
+        private static void CompareAssemblies(string sourceTemplate, string change1, string change2, Match expectedMatch, bool includePrivateMembers)
         {
             bool expectMatch = includePrivateMembers ?
                 expectedMatch == Match.BothMetadataAndRefOut :
@@ -853,6 +854,112 @@ private void CompareAssemblies(string sourceTemplate, string change1, string cha
             }
         }
 
+        [ConditionalFact(typeof(DesktopOnly))]
+        [WorkItem(31197, "https://github.com/dotnet/roslyn/issues/31197")]
+        public void RefAssembly_InvariantToResourceChanges()
+        {
+            var arrayOfEmbeddedData1 = new byte[] { 1, 2, 3, 4, 5 };
+            var arrayOfEmbeddedData2 = new byte[] { 1, 2, 3, 4, 5, 6 };
+
+            IEnumerable<ResourceDescription> manifestResources1 = new[] {
+                new ResourceDescription(resourceName: "A", fileName: "x.goo", () => new MemoryStream(arrayOfEmbeddedData1), isPublic: true)};
+            IEnumerable<ResourceDescription> manifestResources2 = new[] {
+                new ResourceDescription(resourceName: "A", fileName: "x.goo", () => new MemoryStream(arrayOfEmbeddedData2), isPublic: true)};
+            verify();
+
+            manifestResources1 = new[] {
+                new ResourceDescription(resourceName: "A", () => new MemoryStream(arrayOfEmbeddedData1), isPublic: true)}; // embedded
+            manifestResources2 = new[] {
+                new ResourceDescription(resourceName: "A", () => new MemoryStream(arrayOfEmbeddedData2), isPublic: true)}; // embedded
+            verify();
+
+            void verify()
+            {
+                // Verify refout
+                string name = GetUniqueName();
+                var (image1, metadataImage1) = emitRefOut(manifestResources1, name);
+                var (image2, metadataImage2) = emitRefOut(manifestResources2, name);
+                AssertEx.NotEqual(image1, image2, message: "Expecting different main assemblies produced by refout");
+                AssertEx.Equal(metadataImage1, metadataImage2, message: "Expecting identical ref assemblies produced by refout");
+
+                var refAssembly1 = Assembly.ReflectionOnlyLoad(metadataImage1.ToArray());
+                Assert.DoesNotContain("A", refAssembly1.GetManifestResourceNames());
+
+                // Verify refonly
+                string name2 = GetUniqueName();
+                var refOnlyMetadataImage1 = emitRefOnly(manifestResources1, name2);
+                var refOnlyMetadataImage2 = emitRefOnly(manifestResources2, name2);
+                AssertEx.Equal(refOnlyMetadataImage1, refOnlyMetadataImage2, message: "Expecting identical ref assemblies produced by refonly");
+
+                var refOnlyAssembly1 = Assembly.ReflectionOnlyLoad(refOnlyMetadataImage1.ToArray());
+                Assert.DoesNotContain("A", refOnlyAssembly1.GetManifestResourceNames());
+            }
+
+            (ImmutableArray<byte>, ImmutableArray<byte>) emitRefOut(IEnumerable<ResourceDescription> manifestResources, string name)
+            {
+                var source = Parse("");
+                var comp = CreateCompilation(source, options: TestOptions.DebugDll.WithDeterministic(true), assemblyName: name);
+                comp.VerifyDiagnostics();
+
+                var metadataPEStream = new MemoryStream();
+                var refoutOptions = EmitOptions.Default.WithEmitMetadataOnly(false).WithIncludePrivateMembers(false);
+                var peStream = comp.EmitToArray(refoutOptions, metadataPEStream: metadataPEStream, manifestResources: manifestResources);
+
+                return (peStream, metadataPEStream.ToImmutable());
+            }
+
+            ImmutableArray<byte> emitRefOnly(IEnumerable<ResourceDescription> manifestResources, string name)
+            {
+                var source = Parse("");
+                var comp = CreateCompilation(source, options: TestOptions.DebugDll.WithDeterministic(true), assemblyName: name);
+                comp.VerifyDiagnostics();
+
+                var refonlyOptions = EmitOptions.Default.WithEmitMetadataOnly(true).WithIncludePrivateMembers(false);
+                return comp.EmitToArray(refonlyOptions, metadataPEStream: null, manifestResources: manifestResources);
+            }
+        }
+
+        [Fact, WorkItem(31197, "https://github.com/dotnet/roslyn/issues/31197")]
+        public void RefAssembly_CryptoHashFailedIsOnlyReportedOnce()
+        {
+            var hash_resources = new[] {new ResourceDescription("hash_resource", "snKey.snk",
+                () => new MemoryStream(TestResources.General.snKey, writable: false),
+                true)};
+
+            CSharpCompilation moduleComp = CreateEmptyCompilation("",
+                options: TestOptions.DebugDll.WithDeterministic(true).WithOutputKind(OutputKind.NetModule));
+
+            var reference = ModuleMetadata.CreateFromImage(moduleComp.EmitToArray()).GetReference();
+
+            CSharpCompilation compilation = CreateCompilation(
+@"
+[assembly: System.Reflection.AssemblyAlgorithmIdAttribute(12345)]
+
+class Program
+{
+    void M() {}
+}
+", references: new[] { reference }, options: TestOptions.ReleaseDll);
+
+            // refonly
+            var refonlyOptions = EmitOptions.Default.WithEmitMetadataOnly(true).WithIncludePrivateMembers(false);
+            var refonlyDiagnostics = compilation.Emit(new MemoryStream(), pdbStream: null,
+                options: refonlyOptions, manifestResources: hash_resources).Diagnostics;
+
+            refonlyDiagnostics.Verify(
+                // error CS8013: Cryptographic failure while creating hashes.
+                Diagnostic(ErrorCode.ERR_CryptoHashFailed));
+
+            // refout
+            var refoutOptions = EmitOptions.Default.WithEmitMetadataOnly(false).WithIncludePrivateMembers(false);
+            var refoutDiagnostics = compilation.Emit(peStream: new MemoryStream(), metadataPEStream: new MemoryStream(), pdbStream: null,
+                options: refoutOptions, manifestResources: hash_resources).Diagnostics;
+
+            refoutDiagnostics.Verify(
+                // error CS8013: Cryptographic failure while creating hashes.
+                Diagnostic(ErrorCode.ERR_CryptoHashFailed));
+        }
+
         [Fact]
         public void RefAssemblyClient_RefReadonlyParameters()
         {

src/Compilers/Core/Portable/EmbeddedText.cs

@@ -33,6 +33,7 @@ public sealed class EmbeddedText
         /// The path to the file to embed.
         /// </summary>
         /// <remarks>See remarks of <see cref="SyntaxTree.FilePath"/></remarks>
+        /// <remarks>Empty file paths are disallowed, as the debugger finds source by looking up files by their name (and then verifying their signature)</remarks>
         public string FilePath { get; }
 
         /// <summary>

src/Compilers/Core/Portable/Emit/CommonPEModuleBuilder.cs

@@ -350,6 +350,13 @@ public string GetStringFromToken(uint token)
 
         public ImmutableArray<Cci.ManagedResource> GetResources(EmitContext context)
         {
+            if (context.IsRefAssembly)
+            {
+                // Manifest resources are not included in ref assemblies
+                // Ref assemblies don't support added modules
+                return ImmutableArray<Cci.ManagedResource>.Empty;
+            }
+
             if (_lazyManagedResources.IsDefault)
             {
                 var builder = ArrayBuilder<Cci.ManagedResource>.GetInstance();

src/Compilers/VisualBasic/Portable/Emit/PEAssemblyBuilder.vb

@@ -17,6 +17,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
         Private ReadOnly _additionalTypes As ImmutableArray(Of NamedTypeSymbol)
         Private _lazyFiles As ImmutableArray(Of Cci.IFileReference)
 
+        ''' <summary>This is a cache of a subset of <seealso cref="_lazyFiles"/>. We don't include manifest resources in ref assemblies</summary>
+        Private _lazyFilesWithoutManifestResources As ImmutableArray(Of Cci.IFileReference)
+
         ''' <summary>
         ''' This value will override m_SourceModule.MetadataName.
         ''' </summary>
@@ -59,7 +62,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
         End Function
 
         Public NotOverridable Overrides Function GetFiles(context As EmitContext) As IEnumerable(Of Cci.IFileReference)
-            If _lazyFiles.IsDefault Then
+            If Not context.IsRefAssembly Then
+                Return GetFilesCore(context, _lazyFiles)
+            End If
+
+            Return GetFilesCore(context, _lazyFilesWithoutManifestResources)
+        End Function
+
+        Private Function GetFilesCore(context As EmitContext, ByRef lazyFiles As ImmutableArray(Of Cci.IFileReference)) As IEnumerable(Of Cci.IFileReference)
+            If lazyFiles.IsDefault Then
                 Dim builder = ArrayBuilder(Of Cci.IFileReference).GetInstance()
                 Try
                     Dim modules = m_SourceAssembly.Modules
@@ -68,14 +79,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
                         builder.Add(DirectCast(Translate(modules(i), context.Diagnostics), Cci.IFileReference))
                     Next
 
-                    For Each resource In ManifestResources
-                        If Not resource.IsEmbedded Then
-                            builder.Add(resource)
-                        End If
-                    Next
+                    If Not context.IsRefAssembly Then
+                        ' resources are not emitted into ref assemblies
+                        For Each resource In ManifestResources
+                            If Not resource.IsEmbedded Then
+                                builder.Add(resource)
+                            End If
+                        Next
+                    End If
 
                     ' Dev12 compilers don't report ERR_CryptoHashFailed if there are no files to be hashed.
-                    If ImmutableInterlocked.InterlockedInitialize(_lazyFiles, builder.ToImmutable()) AndAlso _lazyFiles.Length > 0 Then
+                    If ImmutableInterlocked.InterlockedInitialize(lazyFiles, builder.ToImmutable()) AndAlso lazyFiles.Length > 0 Then
                         If Not CryptographicHashProvider.IsSupportedAlgorithm(m_SourceAssembly.HashAlgorithm) Then
                             context.Diagnostics.Add(New VBDiagnostic(ErrorFactory.ErrorInfo(ERRID.ERR_CryptoHashFailed), NoLocation.Singleton))
                         End If
@@ -86,7 +100,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
                 End Try
             End If
 
-            Return _lazyFiles
+            Return lazyFiles
         End Function
 
         Private Shared Function Free(builder As ArrayBuilder(Of Cci.IFileReference)) As Boolean