vm: avoid stack overflow on recursive accessor calls#4699
vm: avoid stack overflow on recursive accessor calls#4699jedel1043 merged 3 commits intoboa-dev:mainfrom
Conversation
Test262 conformance changes
|
430081c to
9966682
Compare
|
Follow-up hardening pushed in 9966682. What I changed:
ecursion_in_setter_throws_uncatchable_error
This complements the existing async-generator getter regression and ensures both accessor call directions are covered by runtime-limit behavior. |
|
I'd suggest measuring the performance impact of this change, because keeping track of recursive native calls sounds like it could be detrimental for perf. You can use https://github.com/boa-dev/data/blob/main/bench/bench-v8/combined.js for this. |
9966682 to
8b867e2
Compare
|
@jedel1043 thanks for raising this, I took a careful pass on perf. I built clean release binaries for:
I first tried to run the exact benchmark you suggested from To still get signal, I ran fallback release A/B hot-path measurements (11 runs each, first run dropped as warmup) focused on accessor/property/function throughput:
From this local fallback measurement, I did not observe a regression from I also lowered both recursion regression test limits to |
jedel1043
left a comment
There was a problem hiding this comment.
Thanks!
Maybe in the future we'll finally have coroutines to lift all the inner calls to the main evaluation loop, but in the meantime we need to work around this limitation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4699 +/- ##
==========================================
+ Coverage 47.24% 57.12% +9.88%
==========================================
Files 476 549 +73
Lines 46892 60406 +13514
==========================================
+ Hits 22154 34507 +12353
- Misses 24738 25899 +1161 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This Pull Request fixes/closes boa-dev#4535. ## Summary - Track nested host-driven VM re-entry (JsObject::call / JsObject::construct) with a new Vm::host_call_depth counter. - Include host_call_depth in Context::check_runtime_limits() recursion checks so recursive accessor calls fail with RuntimeLimitError::Recursion instead of overflowing the native stack. - Unignore and strengthen the regression test in m/tests.rs by exercising the async-generator hen getter recursion path with a higher recursion limit. ## Notes - I could not run cargo in this environment (toolchain not available), so CI is expected to validate formatting, lint, and tests.
This Pull Request fixes/closes #4535.
Summary
Notes