Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler should emit localinits when method body contains stackalloc #23951

Closed
VSadov opened this issue Dec 28, 2017 · 10 comments
Closed

Compiler should emit localinits when method body contains stackalloc #23951

VSadov opened this issue Dec 28, 2017 · 10 comments

Comments

@VSadov
Copy link
Member

VSadov commented Dec 28, 2017

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

@VSadov VSadov added this to the 15.7 milestone Dec 28, 2017
@VSadov VSadov self-assigned this Dec 28, 2017
@tannergooding
Copy link
Member

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
Copy link
Member Author

VSadov commented Dec 28, 2017

@tannergooding - see dotnet/csharplang#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 VSadov assigned t-camaia and unassigned VSadov Feb 5, 2018
@VSadov
Copy link
Member Author

VSadov commented Feb 5, 2018

CC: @agocke

@koszeggy
Copy link

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
Copy link
Member

mgravell commented Nov 26, 2019

@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
Copy link
Member

@agocke isn't this fixed now by #39612

@jaredpar jaredpar modified the milestones: Compiler.Next, 16.5 Nov 26, 2019
@koszeggy
Copy link

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
Copy link
Member

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

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

@jaredpar
Copy link
Member

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
Copy link
Member

agocke commented Apr 3, 2020

SkipLocalsInit has been merged, so this is now fixed.

@agocke agocke closed this as completed Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants