Merge pull request #40087 from dotnet/features/localsinit

Merge features/localsinit into master

Author: agocke

docs/contributing/Compiler Test Plan.md

@@ -62,6 +62,7 @@ This document provides guidance for thinking about language interactions and tes
 - Dynamic
 - Ref structs, Readonly structs
 - Readonly members on structs (methods, property/indexer accessors, custom event accessors)
+- SkipLocalsInit
 
 # Code
 - Operators (see Eric's list below)

docs/features/skiplocalsinit.md

@@ -0,0 +1,24 @@
+
+# Feature design for SkipLocalsInit
+
+The feature is to add a new well-known attribute to the compiler, System.Runtime.CompilerServices.SkipLocalsInit that causes method bodies nested "inside" the scope of the attribute to elide the ".locals init" CIL directive that causes the CLR to zero-init local variables and space reserved using the "localloc" instruction. "Nested inside the scope of the attribute" is a concept defined as follows:
+
+1. When the attribute is applied to a module, all emitted methods inside that module, including generated methods, will skip local initialization.
+
+2. When the attribute is applied to a type (including interfaces), all methods transitively inside that type, including generated methods, will skip local initialization. This includes nested types.
+
+3. When the attribute is applied to a method, the method and all methods generated by the compiler which contain user-influenced code inside that method (e.g., local functions, lambdas, async methods) will skip local initialization.
+
+For the above, generated methods will skip local initialization only if they have user code inside them, or a significant number of
+compiler-generated locals. For example, the MoveNext method of an async method will skip local initialization, but the constructor of
+a display class probably will not, as the display class constructor likely has no locals anyway.
+
+Some notable decisions:
+
+1. Although SkipLocalsInit does not require unsafe code blocks, it does require the `unsafe` flag to be passed to the compiler. This
+   is to signal that in some cases code could view unassigned memory, including `stackalloc`. 
+
+2. Due to the compiler not controlling localsinit for other modules when emitting to a netmodule, SkipLocalsInit is not respected on
+   the assembly level, even if the AttributeUsage of the SkipLocalsInitAttribute is specified that it is allowed.
+
+3. The `Inherited` property of the SkipLocalsInitAttribute is not respected. SkipLocalsInit does not propagate through inheritance.

src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs

@@ -64,6 +64,13 @@ private enum IndirectReturnState : byte
 
         private LocalDefinition _returnTemp;
 
+        /// <summary>
+        /// True if there was a <see cref="ILOpCode.Localloc"/> anywhere in the method. This will
+        /// affect whether or not we require the locals init flag to be marked, since locals init
+        /// affects <see cref="ILOpCode.Localloc"/>.
+        /// </summary>
+        private bool _sawStackalloc;
+
         public CodeGenerator(
             MethodSymbol method,
             BoundStatement boundBody,
@@ -188,18 +195,24 @@ internal static bool IsStackLocal(LocalSymbol local, HashSet<LocalSymbol> stackL
 
         private bool IsStackLocal(LocalSymbol local) => IsStackLocal(local, _stackLocals);
 
-        public void Generate()
+        public void Generate(out bool hasStackalloc)
         {
             this.GenerateImpl();
+            hasStackalloc = _sawStackalloc;
 
             Debug.Assert(_asyncCatchHandlerOffset < 0);
             Debug.Assert(_asyncYieldPoints == null);
             Debug.Assert(_asyncResumePoints == null);
         }
 
-        public void Generate(out int asyncCatchHandlerOffset, out ImmutableArray<int> asyncYieldPoints, out ImmutableArray<int> asyncResumePoints)
+        public void Generate(
+            out int asyncCatchHandlerOffset,
+            out ImmutableArray<int> asyncYieldPoints,
+            out ImmutableArray<int> asyncResumePoints,
+            out bool hasStackAlloc)
         {
             this.GenerateImpl();
+            hasStackAlloc = _sawStackalloc;
             Debug.Assert(_asyncCatchHandlerOffset >= 0);
 
             asyncCatchHandlerOffset = _builder.GetILOffsetFromMarker(_asyncCatchHandlerOffset);

src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

@@ -1916,6 +1916,7 @@ private void EmitConvertedStackAllocExpression(BoundConvertedStackAllocExpressio
             // we can ignore that if the actual result is unused
             if (used)
             {
+                _sawStackalloc = true;
                 _builder.EmitOpCode(ILOpCode.Localloc);
             }
 

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -892,7 +892,7 @@ private void CompileMethod(
             _cancellationToken.ThrowIfCancellationRequested();
             SourceMemberMethodSymbol sourceMethod = methodSymbol as SourceMemberMethodSymbol;
 
-            if (methodSymbol.IsAbstract)
+            if (methodSymbol.IsAbstract || methodSymbol.ContainingType?.IsDelegateType() == true)
             {
                 if ((object)sourceMethod != null)
                 {
@@ -1412,7 +1412,8 @@ private static MethodBody GenerateMethodBody(
             var localSlotManager = new LocalSlotManager(variableSlotAllocatorOpt);
             var optimizations = compilation.Options.OptimizationLevel;
 
-            ILBuilder builder = new ILBuilder(moduleBuilder, localSlotManager, optimizations);
+            ILBuilder builder = new ILBuilder(moduleBuilder, localSlotManager, optimizations, method.AreLocalsZeroed);
+            bool hasStackalloc;
             DiagnosticBag diagnosticsForThisMethod = DiagnosticBag.GetInstance();
             try
             {
@@ -1449,7 +1450,7 @@ private static MethodBody GenerateMethodBody(
 
                 if (isAsyncStateMachine)
                 {
-                    codeGen.Generate(out int asyncCatchHandlerOffset, out var asyncYieldPoints, out var asyncResumePoints);
+                    codeGen.Generate(out int asyncCatchHandlerOffset, out var asyncYieldPoints, out var asyncResumePoints, out hasStackalloc);
 
                     // The exception handler IL offset is used by the debugger to treat exceptions caught by the marked catch block as "user unhandled".
                     // This is important for async void because async void exceptions generally result in the process being terminated,
@@ -1470,7 +1471,7 @@ private static MethodBody GenerateMethodBody(
                 }
                 else
                 {
-                    codeGen.Generate();
+                    codeGen.Generate(out hasStackalloc);
 
                     if ((object)kickoffMethod != null)
                     {
@@ -1532,6 +1533,8 @@ private static MethodBody GenerateMethodBody(
                     builder.RealizedSequencePoints,
                     debugDocumentProvider,
                     builder.RealizedExceptionHandlers,
+                    builder.AreLocalsZeroed,
+                    hasStackalloc,
                     builder.GetAllScopes(),
                     builder.HasDynamicLocal,
                     importScopeOpt,
@@ -1776,8 +1779,10 @@ private static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationSta
 
         private static BoundStatement BindImplicitConstructorInitializerIfAny(MethodSymbol method, TypeCompilationState compilationState, DiagnosticBag diagnostics)
         {
+            Debug.Assert(!method.ContainingType.IsDelegateType());
+
             // delegates have constructors but not constructor initializers
-            if (method.MethodKind == MethodKind.Constructor && !method.ContainingType.IsDelegateType() && !method.IsExtern)
+            if (method.MethodKind == MethodKind.Constructor && !method.IsExtern)
             {
                 var compilation = method.DeclaringCompilation;
                 var initializerInvocation = BindImplicitConstructorInitializer(method, diagnostics, compilation);

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironment.cs

@@ -127,6 +127,11 @@ internal override IEnumerable<FieldSymbol> GetFieldsToEmit()
 
         public override Symbol ContainingSymbol => _topLevelMethod.ContainingSymbol;
 
+        // Closures in the same method share the same SynthesizedClosureEnvironment. We must
+        // always return true because two closures in the same method might have different
+        // AreLocalsZeroed flags.
+        public sealed override bool AreLocalsZeroed => true;
+
         // The lambda method contains user code from the lambda
         bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency => true;
 

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureMethod.cs

@@ -1,5 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+#nullable enable
+
 using System.Collections.Immutable;
 using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
@@ -15,9 +17,9 @@ namespace Microsoft.CodeAnalysis.CSharp
     /// </summary>
     internal sealed class SynthesizedClosureMethod : SynthesizedMethodBaseSymbol, ISynthesizedMethodBodyImplementationSymbol
     {
-        private readonly MethodSymbol _topLevelMethod;
         private readonly ImmutableArray<NamedTypeSymbol> _structEnvironments;
 
+        internal MethodSymbol TopLevelMethod { get; }
         internal readonly DebugId LambdaId;
 
         internal SynthesizedClosureMethod(
@@ -39,35 +41,34 @@ originalMethod is LocalFunctionSymbol
                     : MakeName(topLevelMethod.Name, topLevelMethodId, closureKind, lambdaId),
                    MakeDeclarationModifiers(closureKind, originalMethod))
         {
-            _topLevelMethod = topLevelMethod;
+            TopLevelMethod = topLevelMethod;
             ClosureKind = closureKind;
             LambdaId = lambdaId;
 
             TypeMap typeMap;
             ImmutableArray<TypeParameterSymbol> typeParameters;
-            ImmutableArray<TypeParameterSymbol> constructedFromTypeParameters;
 
             var lambdaFrame = ContainingType as SynthesizedClosureEnvironment;
             switch (closureKind)
             {
                 case ClosureKind.Singleton: // all type parameters on method (except the top level method's)
                 case ClosureKind.General: // only lambda's type parameters on method (rest on class)
-                    Debug.Assert((object)lambdaFrame != null);
+                    RoslynDebug.Assert(!(lambdaFrame is null));
                     typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(
                         originalMethod,
                         this,
                         out typeParameters,
-                        out constructedFromTypeParameters,
+                        out _,
                         lambdaFrame.OriginalContainingMethodOpt);
                     break;
                 case ClosureKind.ThisOnly: // all type parameters on method
                 case ClosureKind.Static:
-                    Debug.Assert((object)lambdaFrame == null);
+                    RoslynDebug.Assert(lambdaFrame is null);
                     typeMap = TypeMap.Empty.WithConcatAlphaRename(
                         originalMethod,
                         this,
                         out typeParameters,
-                        out constructedFromTypeParameters,
+                        out _,
                         stopAt: null);
                     break;
                 default:
@@ -162,16 +163,17 @@ protected override ImmutableArray<TypeSymbol> ExtraSynthesizedRefParameters
 
         internal override bool GenerateDebugInfo => !this.IsAsync;
         internal override bool IsExpressionBodied => false;
-        internal MethodSymbol TopLevelMethod => _topLevelMethod;
 
         internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
         {
             // Syntax offset of a syntax node contained in a lambda body is calculated by the containing top-level method.
             // The offset is thus relative to the top-level method body start.
-            return _topLevelMethod.CalculateLocalSyntaxOffset(localPosition, localTree);
+            return TopLevelMethod.CalculateLocalSyntaxOffset(localPosition, localTree);
         }
 
-        IMethodSymbolInternal ISynthesizedMethodBodyImplementationSymbol.Method => _topLevelMethod;
+        public sealed override bool AreLocalsZeroed => TopLevelMethod.AreLocalsZeroed;
+
+        IMethodSymbolInternal? ISynthesizedMethodBodyImplementationSymbol.Method => TopLevelMethod;
 
         // The lambda method body needs to be updated when the containing top-level method body is updated.
         bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency => true;

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/DynamicSiteContainer.cs

@@ -4,6 +4,7 @@
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using System.Diagnostics;
 using Microsoft.CodeAnalysis.Symbols;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
@@ -28,6 +29,11 @@ public override TypeKind TypeKind
             get { return TypeKind.Class; }
         }
 
+        public sealed override bool AreLocalsZeroed
+        {
+            get { throw ExceptionUtilities.Unreachable; }
+        }
+
         bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency
         {
             get { return true; }

src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineTypeSymbol.cs

@@ -94,6 +94,8 @@ public sealed override ImmutableArray<CSharpAttributeData> GetAttributes()
             return _attributes;
         }
 
+        public sealed override bool AreLocalsZeroed => KickoffMethod.AreLocalsZeroed;
+
         internal override bool HasCodeAnalysisEmbeddedAttribute => false;
     }
 }

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/PublicSymbols/AnonymousType.TypePublicSymbol.cs

@@ -282,6 +282,11 @@ public override bool IsSerializable
                 get { return false; }
             }
 
+            public sealed override bool AreLocalsZeroed
+            {
+                get { throw ExceptionUtilities.Unreachable; }
+            }
+
             internal override bool HasDeclarativeSecurity
             {
                 get { return false; }