Fix SkipLocalsInit with stackalloc

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

@@ -1411,6 +1411,7 @@ private static MethodBody GenerateMethodBody(
             var optimizations = compilation.Options.OptimizationLevel;
 
             ILBuilder builder = new ILBuilder(moduleBuilder, localSlotManager, optimizations, method.AreLocalsZeroed);
+            bool hasStackalloc;
             DiagnosticBag diagnosticsForThisMethod = DiagnosticBag.GetInstance();
             try
             {
@@ -1447,7 +1448,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,
@@ -1468,7 +1469,7 @@ private static MethodBody GenerateMethodBody(
                 }
                 else
                 {
-                    codeGen.Generate();
+                    codeGen.Generate(out hasStackalloc);
 
                     if ((object)kickoffMethod != null)
                     {
@@ -1531,6 +1532,7 @@ private static MethodBody GenerateMethodBody(
                     debugDocumentProvider,
                     builder.RealizedExceptionHandlers,
                     builder.AreLocalsZeroed,
+                    hasStackalloc,
                     builder.GetAllScopes(),
                     builder.HasDynamicLocal,
                     importScopeOpt,

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs

@@ -6,6 +6,7 @@
 using System.Reflection;
 using System.Reflection.Metadata;
 using System.Reflection.Metadata.Ecma335;
+using System.Reflection.PortableExecutable;
 using System.Runtime.InteropServices;
 using System.Text;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
@@ -20,19 +21,67 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
 {
     public static class WellKnownAttributeTestsUtil
     {
-        public static bool? HasLocalsInit(this CompilationVerifier verifier, string methodName, bool realIL = false)
+        public static bool? HasLocalsInit(this CompilationVerifier verifier, string qualifiedMethodName, bool realIL = false)
         {
-            var il = verifier.VisualizeIL(methodName, realIL);
-
-            if (il.Contains(".locals init ("))
+            if (realIL)
             {
-                return true;
+                var peReader = new PEReader(verifier.EmittedAssemblyData);
+                var metadataReader = peReader.GetMetadataReader();
+
+                int lastDotIndex = qualifiedMethodName.LastIndexOf('.');
+                var spanName = qualifiedMethodName.AsSpan();
+                var typeName = spanName.Slice(0, lastDotIndex);
+                var methodName = spanName.Slice(lastDotIndex + 1);
+
+                TypeDefinition typeDef = default;
+
+                foreach (var typeHandle in metadataReader.TypeDefinitions)
+                {
+                    var type = metadataReader.GetTypeDefinition(typeHandle);
+                    var name = metadataReader.GetString(type.Name);
+
+                    if (name.AsSpan().Equals(typeName, StringComparison.Ordinal))
+                    {
+                        typeDef = type;
+                        break;
+                    }
+                }
+
+                Assert.NotEqual(default, typeDef);
+
+                MethodDefinition methodDef = default;
+
+                foreach (var methodHandle in typeDef.GetMethods())
+                {
+                    var method = metadataReader.GetMethodDefinition(methodHandle);
+                    var name = metadataReader.GetString(method.Name);
+
+                    if (name.AsSpan().Equals(methodName, StringComparison.Ordinal))
+                    {
+                        methodDef = method;
+                        break;
+                    }
+                }
+
+                Assert.NotEqual(default, methodDef);
+
+                var block = peReader.GetMethodBody(methodDef.RelativeVirtualAddress);
+                return block.LocalVariablesInitialized;
             }
-            if (il.Contains(".locals ("))
+            else
             {
-                return false;
+                var il = verifier.VisualizeIL(qualifiedMethodName, realIL);
+
+                if (il.Contains(".locals init ("))
+                {
+                    return true;
+                }
+                if (il.Contains(".locals ("))
+                {
+                    return false;
+                }
+                return null;
             }
-            return null;
         }
     }
 
@@ -8929,6 +8978,20 @@ public static int Main()
 
         #region SkipLocalsInitAttribute
 
+        private CompilationVerifier CompileAndVerifyWithSkipLocalsInit(string src)
+        {
+            const string skipLocalsInitDef = @"
+namespace System.Runtime.CompilerServices
+{
+    public class SkipLocalsInitAttribute : System.Attribute
+    {
+    }
+}";
+
+            var comp = CreateCompilation(new[] { src, skipLocalsInitDef }, options: TestOptions.UnsafeReleaseDll);
+            return CompileAndVerify(comp, verify: Verification.Fails);
+        }
+
         [Fact]
         public void SkipLocalsInitRequiresUnsafe()
         {
@@ -9040,6 +9103,39 @@ partial class C
             Assert.False(comp.HasLocalsInit("C.M"));
         }
 
+        [Fact]
+        public unsafe void StackallocWithSkipLocalsInit()
+        {
+            var src = @"
+public class C
+{
+    [System.Runtime.CompilerServices.SkipLocalsInitAttribute]
+    public unsafe void M1()
+    {
+        int *ptr = stackalloc int[10];
+        System.Console.WriteLine(ptr[0]);
+    }
+
+    public unsafe void M2()
+    {
+        int *ptr = stackalloc int[10];
+        System.Console.WriteLine(ptr[0]);
+    }
+
+    public unsafe void M3()
+    {
+       int* p = stackalloc int[10]; // unused
+    }
+}";
+            var verifier = CompileAndVerifyWithSkipLocalsInit(src);
+            Assert.Null(verifier.HasLocalsInit("C.M1")); // no locals
+            Assert.False(verifier.HasLocalsInit("C.M1", realIL: true));
+            Assert.Null(verifier.HasLocalsInit("C.M2")); // no locals
+            Assert.True(verifier.HasLocalsInit("C.M2", realIL: true));
+            Assert.Null(verifier.HasLocalsInit("C.M3")); // no locals
+            Assert.False(verifier.HasLocalsInit("C.M3", realIL: true));
+        }
+
         [Fact]
         public void WhenMethodsDifferBySkipLocalsInitAttributeTheyMustHaveDifferentRVA()
         {

src/Compilers/Core/Portable/CodeGen/MethodBody.cs

@@ -52,6 +52,7 @@ public MethodBody(
             DebugDocumentProvider debugDocumentProvider,
             ImmutableArray<Cci.ExceptionHandlerRegion> exceptionHandlers,
             bool areLocalsZeroed,
+            bool hasStackalloc,
             ImmutableArray<Cci.LocalScope> localScopes,
             bool hasDynamicLocalVariables,
             Cci.IImportScope importScopeOpt,
@@ -75,6 +76,7 @@ public MethodBody(
             _locals = locals;
             _exceptionHandlers = exceptionHandlers;
             _areLocalsZeroed = areLocalsZeroed;
+            HasStackalloc = hasStackalloc;
             _localScopes = localScopes;
             _hasDynamicLocalVariables = hasDynamicLocalVariables;
             _importScopeOpt = importScopeOpt;
@@ -144,5 +146,10 @@ ImmutableArray<EncHoistedLocalInfo> Cci.IMethodBody.StateMachineHoistedLocalSlot
         public ImmutableArray<LambdaDebugInfo> LambdaDebugInfo => _lambdaDebugInfo;
 
         public ImmutableArray<ClosureDebugInfo> ClosureDebugInfo => _closureDebugInfo;
+
+        /// <summary>
+        /// True if there's a stackalloc somewhere in the method.
+        /// </summary>
+        public bool HasStackalloc { get; }
     }
 }

src/Compilers/Core/Portable/Emit/NoPia/CommonEmbeddedMethod.cs

@@ -113,6 +113,8 @@ public EmptyBody(CommonEmbeddedMethod method)
                 ImmutableArray<Cci.ExceptionHandlerRegion> Cci.IMethodBody.ExceptionRegions =>
                     ImmutableArray<Cci.ExceptionHandlerRegion>.Empty;
 
+                bool Cci.IMethodBody.HasStackalloc => false;
+
                 bool Cci.IMethodBody.AreLocalsZeroed => false;
 
                 ImmutableArray<Cci.ILocalDefinition> Cci.IMethodBody.LocalVariables =>

src/Compilers/Core/Portable/PEWriter/Members.cs

@@ -328,6 +328,11 @@ ImmutableArray<ExceptionHandlerRegion> ExceptionRegions
         /// </summary>
         bool AreLocalsZeroed { get; }
 
+        /// <summary>
+        /// True if there's a stackalloc somewhere in the method.
+        /// </summary>
+        bool HasStackalloc { get; }
+
         /// <summary>
         /// The local variables of the method.
         /// </summary>

src/Compilers/Core/Portable/PEWriter/MetadataWriter.cs

@@ -2933,7 +2933,8 @@ private int SerializeMethodBody(MethodBodyStreamEncoder encoder, IMethodBody met
                 exceptionRegionCount: exceptionRegions.Length,
                 hasSmallExceptionRegions: MayUseSmallExceptionHeaders(exceptionRegions),
                 localVariablesSignature: localSignatureHandleOpt,
-                attributes: (methodBody.AreLocalsZeroed ? MethodBodyAttributes.InitLocals : 0));
+                attributes: (methodBody.AreLocalsZeroed ? MethodBodyAttributes.InitLocals : 0),
+                hasDynamicStackAllocation: methodBody.HasStackalloc);
 
             // Don't do small body method caching during deterministic builds until this issue is fixed
             // https://github.com/dotnet/roslyn/issues/7595

src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb

@@ -1611,6 +1611,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                                       debugDocumentProvider,
                                       builder.RealizedExceptionHandlers,
                                       areLocalsZeroed:=True,
+                                      hasStackalloc:=False,
                                       localScopes,
                                       hasDynamicLocalVariables:=False,
                                       importScopeOpt:=importScopeOpt,

src/Test/Utilities/Portable/Metadata/ILValidation.cs

@@ -300,6 +300,22 @@ public static unsafe string GetMethodIL(this ImmutableArray<byte> ilArray)
             return result.ToString();
         }
 
+        public static unsafe MethodBodyBlock GetMethodBodyBlock(this ImmutableArray<byte> ilArray)
+        {
+            fixed (byte* ilPtr = ilArray.AsSpan())
+            {
+                int offset = 0;
+                // skip padding:
+                while (offset < ilArray.Length && ilArray[offset] == 0)
+                {
+                    offset++;
+                }
+
+                var reader = new BlobReader(ilPtr + offset, ilArray.Length - offset);
+                return MethodBodyBlock.Create(reader);
+            }
+        }
+
         public static Dictionary<int, string> GetSequencePointMarkers(string pdbXml, string source = null)
         {
             var doc = new XmlDocument() { XmlResolver = null };