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.

Sign up for GitHub

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

Jump to bottom

Report StackOverflowException on NativeAOT on Linux #93901

Closed
wants to merge 118 commits into from
Closed

Report StackOverflowException on NativeAOT on Linux #93901

wants to merge 118 commits into from

Conversation

jtschuster
Copy link
Member

@jtschuster jtschuster commented Oct 24, 2023 •
edited
Loading

Uses almost the same code as CoreCLR to allocate an alternate stack for handling SIGSEGV, and properly reports a StackOverflowException.

Fixes #82334

@jtschuster
Report stack overflow in linux
8260630 
Use SA_ONSTACK for SIGSEGV handler
Call sigaltstack() to set the alternate stack on the main thread only
Check if faulting address is near the stack pointer, if so, report stack overflow
@jtschuster
Set up signal alternate stack on all new threads
263262c
@jtschuster
Merge branch 'main' of https://github.com/dotnet/runtime into StackOv…
adc342f 
…erflowLinuxNAOT
@jtschuster
Add test to naot smoke tests
666d231
@ghost ghost assigned jtschuster Oct 24, 2023
@ghost
Copy link

ghost commented Oct 24, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Uses almost the same code as CoreCLR to allocate an alternate stack for handling SIGSEGV, and properly reports a StackOverflowException.

Author: jtschuster
Assignees: jtschuster
Labels:

area-NativeAOT-coreclr
Milestone: -

@jkotas jkotas requested a review from janvorli October 24, 2023 02:39
@build-analysis build-analysis bot mentioned this pull request Oct 24, 2023
Closed
janvorli
janvorli approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the nit comment

src/coreclr/nativeaot/Runtime/thread.cpp Outdated Show resolved Hide resolved
@janvorli
Copy link
Member

One thing I wonder about - don't we also want to call PalRaiseFailFastException instead of RhFailFast (like we do on Windows for stack overflow in RhpVectoredExceptionHandler)?

@jkotas
Copy link
Member

jkotas commented Oct 24, 2023

One thing I wonder about - don't we also want to call PalRaiseFailFastException instead of RhFailFast (like we do on Windows for stack overflow in RhpVectoredExceptionHandler)?

RhFailFast calls PalRaiseFailFastException with default arguments. I do not think calling PalRaiseFailFastException in this change would make any difference.

@janvorli
Copy link
Member

Ah, thank you, I've missed that.

@jtschuster
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtschuster
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Oct 27, 2023
Closed
Closed
@janvorli
Copy link
Member

@jtschuster I am not sure if merging of the tests into a single exe won't complicate the work on merging coreclr tests that @trylek works on. I wonder what the reason for this change was.

pavelsavara and others added 11 commits November 7, 2023 11:32
@pavelsavara
[browser][MT] fix stale memory on suspended thread (dotnet#94299)
bd27e27
@pavelsavara
[browser] allow zero as debug level (dotnet#94456)
5de6ca4
@jakobbotsch
JIT: Remove dead GT_NOP code in LSRA (dotnet#94409)
85d3773 
Rationalization removes all GT_NOP nodes, and we do not seem to
introduce any of these before LSRA (and especially not the typed
"passthrough" GT_NOP nodes). This code is just wrong for the passthrough
nodes, so just switch all the LSRA code to only handle potential void
NOPs.

Fix dotnet#93048
@dotnet-maestro
[main] Update dependencies from dotnet/installer (dotnet#94462)
50d4a99 
Microsoft.Dotnet.Sdk.Internal
 From Version 9.0.100-alpha.1.23554.1 -> To Version 9.0.100-alpha.1.23557.2

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
@DrewScoggins
Add file handle limit setting for Alpine (dotnet#94439)
b0e1627 
* Add file handle limit setting for Alpine

Alpine does not automatically increase the file handle limit to the maximum. dotnet#82719 (comment)
As a result we need to increase the limit manually, as we are seeing "Too many files" errors.

* Bump limit to hard cap

* Remove space
@MichalStrehovsky
Do not release frozen segment if we failed extending it (dotnet#94450)
021ecb8 
Maybe I'm missing something, but if we try to commit more of the reserved segment and this fails, it doesn't feel right to release the whole thing. The previously committed part of the segment is still in use.
@ViktorHofer
Use live M.Bcl.AsyncInterfaces dependency (dotnet#94451)
4feec00 
* Use live M.Bcl.AsyncInterfaces dependency

* Update Microsoft.Bcl.TimeProvider.csproj
@cincuranet
Add option for R2R configured runs. (dotnet#94412)
2801995
@dotnet-maestro
Update dependencies from https://github.com/dotnet/source-build-exter…
4d2ef11 
…nals build 20231106.2 (dotnet#94463)

Microsoft.SourceBuild.Intermediate.source-build-externals
 From Version 9.0.0-alpha.1.23530.1 -> To Version 9.0.0-alpha.1.23556.2

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
@jtschuster
Merge branch 'StackOverflowLinuxNAOT' of https://github.com/jtschuste…
8b6b9cd 
…r/runtime into StackOverflowLinuxNAOT
@jtschuster
Use IsSingleFile and update Main signature
4aa9912
@jtschuster jtschuster marked this pull request as draft November 7, 2023 19:12
@jtschuster jtschuster mentioned this pull request Nov 7, 2023
Merged
@jtschuster
Copy link
Member Author

I made some mistake when I pulled main back into this branch. Closing this PR and opened a new one.

@jtschuster jtschuster closed this Nov 7, 2023
@build-analysis build-analysis bot mentioned this pull request Nov 7, 2023
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@agocke agocke agocke left review comments

@jkotas jkotas jkotas left review comments

@MichalStrehovsky MichalStrehovsky MichalStrehovsky left review comments

@janvorli janvorli janvorli approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@sbomer sbomer Awaiting requested review from sbomer sbomer is a code owner

@radical radical Awaiting requested review from radical

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@pavelsavara pavelsavara Awaiting requested review from pavelsavara pavelsavara is a code owner

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

@kg kg Awaiting requested review from kg

@vargaz vargaz Awaiting requested review from vargaz

@lambdageek lambdageek Awaiting requested review from lambdageek lambdageek is a code owner

@BrzVlad BrzVlad Awaiting requested review from BrzVlad BrzVlad is a code owner

@kotlarmilos kotlarmilos Awaiting requested review from kotlarmilos kotlarmilos is a code owner

@fanyang-mono fanyang-mono Awaiting requested review from fanyang-mono fanyang-mono is a code owner

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@akoeplinger akoeplinger Awaiting requested review from akoeplinger akoeplinger will be requested when the pull request is marked ready for review akoeplinger is a code owner

@maraf maraf Awaiting requested review from maraf maraf will be requested when the pull request is marked ready for review maraf is a code owner

@ilonatommy ilonatommy Awaiting requested review from ilonatommy ilonatommy will be requested when the pull request is marked ready for review ilonatommy is a code owner

@steveisok steveisok Awaiting requested review from steveisok steveisok will be requested when the pull request is marked ready for review steveisok is a code owner

Assignees

@jtschuster jtschuster

Labels
area-NativeAOT-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[NativeAOT] Stackoverflow reporting on Linux