Roslyn: Compiler should emit `localinits` when method body contains `stackalloc`

[VSadov]

Currently the localinits is triggered by the presence of IL locals. However the flag has effect on everything that is allocated from local frame, including stackalloc.

NOTE: the absence of the flag results in stackallocated data containing nondeterministic garbage and as such emitting the flag is not compat breaking.

While the language spec does not specify or require that, stackallocated memory is nearly always zero-inited and users often take dependency on that. Suddenly not emitting the flag could result in bugs that are very hard to reproduce.
We should make sure that the flag is emitted in _all_ cases and document the default behavior as such.

NOTE: the cases with opposite expectations are also known -
Some users use this bug as a way to avoid costs of zeroing out stack-allocated memory. We should provide a more stable and documented way to do that.

Relying on essentially a bug that is triggered by unstable condition such as absence of IL locals is not a maintainable strategy anyways.

Proposal: https://github.com/dotnet/csharplang/blob/master/proposals/skip-localsinit.md

[tannergooding]

Some users use this bug as a way to avoid costs of zeroing out stack-allocated memory. We should provide a more stable and documented way to do that.

There are a number of people that would be very happy if there was a way to deterministically indicate we don't want the .locals init flag on a method.

[VSadov]

@tannergooding - see https://github.com/dotnet/csharplang/pull/1223

It does not seem to be big enough to be a real C# language feature (as in "required by all conforming C# implementations"), but we can have it a s a _compiler_ feature where compiler may optionally recognize a specific attribute as a directive/hint to omit the flag.

[VSadov]

CC: @agocke

[koszeggy]

Today I bumped into this issue in .NET Core 3.0. From #1279:

C# will keep zero initializing by default. Changing the default would be too breaking.

Is this still the planned case? Or is there a compiler switch or something to enforce the compatible behavior in every version? Zeroing manually would be an unnecessary overhead if the code targets older frameworks and I would like to avoid that.

[mgravell]

@koszeggy from what I can see, the compiler is emitting the .locals init for the second version in all cases - but I can repro the Reinterpret2 method returning garbage on netfx (not netcoreapp), so presumably it is the JIT that is choosing to do something differently between netfx and netcoreapp, and : the netfx JIT simply isn't going to get updated. So if you want deterministic behavior, the only option is for your code to assume "undefined". Note that you might be able to avoid initializing each byte manually, though - you could thunk to a wider type for the zero, or use a block-zero / block-assign API

[jaredpar]

@agocke isn't this fixed now by #39612

[koszeggy]

It seems, using fixed buffers can be a possible workaround for avoiding possible doubled initialization. They seem to be initialized in all cases and since they don't need to be pinned anymore they are just as fast as using stackalloc.

However, I'm not sure whether I can generally rely on this behavior.

[tannergooding]

Avoiding double-initialization will be achievable with the [SkipLocalsInit] attribute/feature: https://github.com/dotnet/csharplang/pull/1223.

The feature branch for it is here: https://github.com/dotnet/roslyn/tree/features/localsinit

[jaredpar]

Note that the fix and the [SkipLocalsInit] feature are being done in the same branch. Hence customers won't be left in a state where they're stuck with the double init waiting for a feature. Rather the bug fix and the opt-in to the (somewhat) unsafe behavior will be provided simultaneously.

[agocke]

SkipLocalsInit has been merged, so this is now fixed.