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

__chkstk in debug and release-safe modes only and on all OSes #530

Closed
andrewrk opened this issue Oct 9, 2017 · 11 comments · Fixed by #2440
Closed

__chkstk in debug and release-safe modes only and on all OSes #530

andrewrk opened this issue Oct 9, 2017 · 11 comments · Fixed by #2440
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Oct 9, 2017

__chkstk is a great idea, it causes a stack overflow to become a segfault. Let's do it on all OSes that we can, not just windows.

And let's turn it off for ReleaseFast mode.

@andrewrk andrewrk added the enhancement Solving this issue will likely involve adding new logic or components to the codebase. label Oct 9, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Oct 9, 2017
@PavelVozenilek
Copy link

Stack size should large enough by default (say 10 MB instead of MSVC's 1 MB). For embedded application one could always set it lower.

Certain version of AIX C++ compiler had default stack size 40 kB (yes, kilobytes) and it was big pain to find out why program (working elsewhere else) crashed.

@andrewrk
Copy link
Member Author

Stack size should large enough by default

Large enough for what?

You could have a recursive function use 9.9 MB and then call a function with a greater-than-page-size stack frame.

Edge cases matter.

@PavelVozenilek
Copy link

PavelVozenilek commented Oct 10, 2017

Large enough for big applications. LLVM itself uses 10 MB stack. I set it to 20 MB, to keep troubles away. I had never run out of stack due to recursion.

Edit: I do not think __chkstk is bad or anything, it just is not a problem I got. (Though: I had once a silly idea to replace C stdlib from MSVC with something leaner. Minimal size executables and such. Handling these internal and undocumented functions took lot of time and I eventually abandoned the attempt, due to random crashes.)

@kyle-github
Copy link

Hi @PavelVozenilek, I am not sure I get the point of your comment. Stack is not actually allocated from main memory except on demand. The __chkstk function just makes sure that the local allocation of each function does not go past the guard page. It has nothing to do with the total amount of virtual memory address space that is allocated for the stack. Am I missing something???

I do like __chkstk. While I tend not to use large on-stack data, there is code that does with good effect. It is a great way to handle allocation without using malloc in C if your use cases can handle that approach.

@kyle-github
Copy link

I did not run across an alloca() equivalent in Zig. If there is not, is stack allocation known statically on each function invocation at compile time? If so, then you could inject the equivalent of __chkstk only when you knew that there was a lot of data in the function's activation record.

@PavelVozenilek
Copy link

@kyle-github: I made aside remark, about usefullness of setting large stack limit.

@andrewrk
Copy link
Member Author

andrewrk commented Oct 11, 2017

then you could inject the equivalent of __chkstk only when you knew that there was a lot of data in the function's activation record.

Status quo: when targeting windows, LLVM emits a call to __chkstk when the stack frame is larger than the guard page size, in all build modes.

@kyle-github
Copy link

@andrewrk, cool.

@andrewrk
Copy link
Member Author

I did not run across an alloca() equivalent in Zig.

See #225

@kyle-github
Copy link

@andrewrk, Ah missed that one. Thanks. While alloca() is useful in some circumstances, it is easily abused.

@andrewrk andrewrk modified the milestones: 0.2.0, 0.3.0 Oct 19, 2017
@andrewrk
Copy link
Member Author

Looks like "probe-stack" is relevant: http://llvm.org/docs/LangRef.html#function-attributes

@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 28, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Sep 28, 2018
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 May 3, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.5.0 Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants