Repository navigation
[wasm][aot] System.Text.RegularExpressions.Unit.Tests failing due to Maximum call stack size exceeded #66118
Description
Hitting on rolling builds, and PRs. This is when building the tests with AOT, and happens on linux, and windows.
info: Discovered: System.Text.RegularExpressions.Unit.Tests.dll (found 13 of 13 test cases)
info: Using random seed for test cases: 1824947855
info: Using random seed for collections: 1824947855
info: Starting: System.Text.RegularExpressions.Unit.Tests.dll
fail: {}
fail: RangeError: Maximum call stack size exceeded
fail: at pthread_getspecific (<anonymous>:wasm-function[74267]:0xf7c81e)
fail: at get_context (<anonymous>:wasm-function[56607]:0xd70b64)
fail: at interp_sufficient_stack (<anonymous>:wasm-function[56663]:0xd7ee7c)
fail: at mini_interp_sufficient_stack (<anonymous>:wasm-function[72917]:0xf53721)
fail: at ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_SufficientExecutionStack (<anonymous>:wasm-function[60228]:0xdec29e)
fail: at aot_wrapper_corlib_System_dot_Runtime_dot_CompilerServices_System_dot_Runtime_dot_CompilerServices_dot_RuntimeHelpers__SufficientExecutionStack_pinvoke_bool_bool_ (<anonymous>:wasm-function[27205]:0x7454cf)
fail: at corlib_System_Runtime_CompilerServices_RuntimeHelpers_TryEnsureSufficientExecutionStack (<anonymous>:wasm-function[26554]:0x72be45)
fail: at System_Text_RegularExpressions_Unit_Tests_System_Threading_StackHelper_TryEnsureSufficientExecutionStack (<anonymous>:wasm-function[44081]:0xa5dcf9)
fail: at System_Text_RegularExpressions_Unit_Tests_System_Text_RegularExpressions_RegexNode_FindAndMakeLoopsAtomic (<anonymous>:wasm-function[44280]:0xa79a36)
fail: at System_Text_RegularExpressions_Unit_Tests_System_Text_RegularExpressions_RegexNode_FindAndMakeLoopsAtomic (<anonymous>:wasm-function[44280]:0xa79a8e)
...
Activity
radical commentedon Mar 2, 2022
This seems to be only relevant commit compared the previous build that passed.
cc @stephentoub
stephentoub commentedon Mar 3, 2022
The purpose of TryEnsureSufficientExecutionStack is that code should be able to call it to know when it's too deep on the stack to do what it's about to do, and Regex uses that API to determine whether it can do certain processing. Seems like a mono bug if calling that API is producing some kind of exception about being too deep.
The cited PR also didn't add any code that would trigger this, but it did move some tests around. Was the regex test suite previously being suppressed somewhere in this configuration? If so, its new unit tests project might also need to be suppressed in the same place.
But it also seems like there's a bug here in mono that might have been previously hidden because of such suppression.
vargaz commentedon Mar 3, 2022
SufficientExecutionStack probably has some problems on wasm. Does this happen on linux+wasm as well ?
radical commentedon Mar 3, 2022
Yep, linux and windows.
radical commentedon Mar 3, 2022
AppBundlefrom my Mac.AppBundle.zip
vargaz commentedon Mar 3, 2022
So it looks like that on wasm, TryEnsureSufficientExecutionStack cannot really be implemented since the real execution stack is hidden from the program. We can only check that we don't run out of the C stack which is in linear memory.
In this case, the 'real' execution stack overflows, and the wasm vm aborts.
vargaz commentedon Mar 4, 2022
Decreasing the depth in MinMaxLengthIsCorrect_HugeDepth to 5000 works around the problem. The recursion code should have some kind of hardcoded limit imho,
FindAndMakeLoopsAtomic () was on the stack 8k times when it crashed.
[wasm][aot] Disable `System.Text.RegularExpressions.Unit.Tests`
[wasm][aot] Disable `System.Text.RegularExpressions.Unit.Tests` (#66311)
stephentoub commentedon Mar 7, 2022
It's using TryEnsureSufficientExecutionStack to avoid needing a hardcoded limit. This particular test is a stress test validating that behavior is functional.
stephentoub commentedon Mar 7, 2022
(Multiple places use TryEnsureSufficientExecutionStack for this same purpose, e.g.
awaitfor determining whether it should queue long async continuation chains, System.Linq.Expressions, I believe Roslyn in various places, etc.)lewing commentedon Mar 10, 2022
This is especially tricky for wasm given that different wasm runtimes have different limits and at times use wildly different amounts of execution stack for the same code. For AOT we may need to make a configurable limit based on testing.
stephentoub commentedon Mar 10, 2022
A configurable limit in TryEnsureSufficientExecutionStack, or a configurable limit everywhere that uses TryEnsureSufficientExecutionStack? I don't think the latter is reasonable in general (even if we were to do so everywhere we use it, we can't expect that to be true for all 3rd-party usage), but if there's really no way to correctly implement TryEnsureSufficientExecutionStack on wasm, imbuing it there with a conservative limit in TryEnsureSufficientExecutionStack seems like an ok fallback.
lewing commentedon Mar 10, 2022
A limit in TryEnsureSufficientExecutionStack, agreed about the latter. @radekdoulik can you take a look at this.
danmoseley commentedon Apr 22, 2022
removing 'blocking-clean-ci' as it's not, as zero regex tests are now running on WASM, pending a fix for TryEnsureSufficientExecutionStack.