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

Warn if stackalloc would throw with a constant size at runtime #6232

Open
alrz opened this issue Jan 12, 2018 · 5 comments
Open

Warn if stackalloc would throw with a constant size at runtime #6232

alrz opened this issue Jan 12, 2018 · 5 comments

Comments

@alrz
Copy link

alrz commented Jan 12, 2018

e.g. Span<int> s = stackalloc int[int.MaxValue]

Similar to dotnet/roslyn#8456
Relates to warning waves

@jcouv
Copy link
Member

jcouv commented Jan 13, 2018

Tagging @VSadov to triage

@svick
Copy link
Contributor

svick commented Jan 14, 2018

What is the maximum stack size the compiler is supposed to assume? Since the Thread constructor that lets you specify stack size takes an int, is it int.MaxValue bytes (i.e. 2 GB)?

E.g. stackalloc int[int.MaxValue / 2] would produce a warning, but stackalloc int[int.MaxValue / 4] or stackalloc byte[int.MaxValue] wouldn't?

And since this is just a warning, I assume that some future version of .Net allowed stacks larger than 2 GB is not an issue?

@VSadov
Copy link
Member

VSadov commented Jan 14, 2018

The idea is that while we do not know how platform/OS handles the stack, we know that ‘localloc’ takes an int. So if the size does not fit in an int, it will certanly throw.

In reality the problems will start much earlier. Default stack limits are generally 1-4 mBs.

That makes it questionable that we would warn on outlandish sizes when anything more that couple kBs is dangerous.

@svick
Copy link
Contributor

svick commented Jan 14, 2018

we know that ‘localloc’ takes an int. So if the size does not fit in an int, it will certanly throw.

As far as I can tell, that's not true. The spec says that localloc takes UIntPtr or uint. So there could be a .Net platform where stackalloc int[int.MaxValue] works fine.

@tannergooding
Copy link
Member

tannergooding commented Jan 14, 2018

Default stack size comes from the SizeOfStackReserve field in the PE Header.

Perhaps the warning could be tied to the default limit (1MB) and a compiler switch that allows the default to be changed could also exist (seems like a more generally useful thing with stack allocations being more readily possible in safe code anyways).

However, the warning should probably emit at percentage (maybe 50-75%) of the actual max stack, rather than at max stack itself. Even the main method has some stack taken away by locals/etc (and other method calls temporarily shrink the available space as well), so stackalloc int[maxStack - 1] will almost certainly trigger a runtime error and should be accounted for.

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

7 participants