Use a RAII class for recursion checks
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox89 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(18 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
The goal is to change:
if (!CheckRecursionLimit(cx)) {
return false;
}
to:
AutoCheckRecursionLimit recursion(cx);
if (!recursion.check(cx)) {
return false;
}
This will simplify the WASI changes in bug 1704771.
|
Assignee | |
Comment 1•3 years ago
|
||
The use of a C++ RAII class lets us simplify a few things and prepares for
the WASI changes in bug 1704771.
|
|
Assignee | |
Comment 2•3 years ago
|
||
Depends on D111868
|
|
Assignee | |
Comment 3•3 years ago
|
||
Depends on D111869
|
|
Assignee | |
Comment 4•3 years ago
|
||
Depends on D111870
|
|
Assignee | |
Comment 5•3 years ago
|
||
In EnterBaseline, it's simpler to use checkWithExtra.
After that we can inline CheckRecursionLimitWithStackPointer into checkWithExtra.
Depends on D111871
|
|
Assignee | |
Comment 6•3 years ago
|
||
Depends on D111872
|
|
Assignee | |
Comment 7•3 years ago
|
||
Depends on D111874
|
|
Assignee | |
Comment 8•3 years ago
|
||
Depends on D111875
|
|
Assignee | |
Comment 9•3 years ago
|
||
This is now a private method so is better encapsulated.
Depends on D111876
|
|
Assignee | |
Comment 10•3 years ago
|
||
Depends on D111877
|
|
Assignee | |
Comment 11•3 years ago
|
||
Depends on D111878
|
|
Assignee | |
Comment 12•3 years ago
|
||
Depends on D111879
|
|
Assignee | |
Comment 13•3 years ago
|
||
Depends on D111880
|
|
Assignee | |
Comment 14•3 years ago
|
||
Depends on D111882
|
|
Assignee | |
Comment 15•3 years ago
|
||
The next patch wants to remove JS_CHECK_STACK_SIZE, so stop calling it directly.
|
|
Assignee | |
Comment 16•3 years ago
|
||
All stack overflow checks are now routed through checkLimitImpl, so move the
JS_STACK_OOM_POSSIBLY_FAIL testing check there. This also gives us a place to
check the WASI limit later on.
Depends on D111999
|
|
Assignee | |
Comment 17•3 years ago
|
||
For check we have an optimization to check the untrusted limit first, to avoid a
slow getStackLimit call.
This patch implements check using checkDontReport, and checkDontReport using
checkWithStackPointerDontReport, and moves the optimization into
checkWithStackPointerDontReport, so that all these functions now have this optimization.
Renames getStackLimit to getStackLimitSlow now that it's only used on the slow path.
Depends on D112000
|
|
Assignee | |
Comment 18•3 years ago
|
||
It's easier to understand the code withou these helpers.
Depends on D112001
|
|
Assignee | |
Updated•3 years ago
|
|
||
Comment 19•3 years ago
|
||
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/52b63cb9abed part 1 - Change CheckRecursionLimit to AutoCheckRecursionLimit::check. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/684284cd28cf part 2 - Change CheckRecursionLimitConservative to AutoCheckRecursionLimit::checkConservative. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/7766df072ce2 part 3 - Change CheckSystemRecursionLimit to AutoCheckRecursionLimit::checkSystem. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/d544b912da81 part 4 - Change CheckRecursionLimitWithExtra to AutoCheckRecursionLimit::checkWithExtra. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/580046346d2e part 5 - Remove CheckRecursionLimitWithStackPointer. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/47ce7690cd37 part 6 - Change CheckRecursionLimitWithStackPointerDontReport to AutoCheckRecursionLimit::checkWithStackPointerDontReport. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/f6b6012baa23 part 7 - Change CheckRecursionLimitDontReport to AutoCheckRecursionLimit::checkDontReport. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/7688bc2999ea part 8 - Change CheckRecursionLimitConservativeDontReport to AutoCheckRecursionLimit::checkConservativeDontReport. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/aebec6a49d31 part 9 - Change CheckRecursionLimitDontReport to AutoCheckRecursionLimit::checkLimitDontReport. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/c99d9f76b0b2 part 10 - Change CheckRecursionLimit to AutoCheckRecursionLimit::checkLimit. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/f8d5545d982b part 11 - Change GetNativeStackLimit to AutoCheckRecursionLimit::getStackLimit. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/903bbc357458 part 12 - Change GetNativeStackLimitHelper to AutoCheckRecursionLimit::getStackLimitHelper. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/626aae1e53f9 part 13 - Change RunningWithTrustedPrincipals to AutoCheckRecursionLimit::runningWithTrustedPrincipals. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/c2e80cdf57fc part 14 - Fix and improve comment. r=tcampbell
|
|
Assignee | |
Updated•3 years ago
|
|
|
||
Comment 20•3 years ago
|
||
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5d6cf556bc48 part 15 - Clean up stack overflow check in ComponentFinder. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/1f2846c1c6ad part 16 - Replace JS_CHECK_STACK_SIZE macro with checkLimitImpl. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/043dfe053910 part 17 - Optimize slow getStackLimit call in more cases. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/d3acac58e972 part 18 - Remove checkLimit and checkLimitDontReport helpers. r=tcampbell
|
||
Comment 21•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/52b63cb9abed
https://hg.mozilla.org/mozilla-central/rev/684284cd28cf
https://hg.mozilla.org/mozilla-central/rev/7766df072ce2
https://hg.mozilla.org/mozilla-central/rev/d544b912da81
https://hg.mozilla.org/mozilla-central/rev/580046346d2e
https://hg.mozilla.org/mozilla-central/rev/47ce7690cd37
https://hg.mozilla.org/mozilla-central/rev/f6b6012baa23
https://hg.mozilla.org/mozilla-central/rev/7688bc2999ea
https://hg.mozilla.org/mozilla-central/rev/aebec6a49d31
https://hg.mozilla.org/mozilla-central/rev/c99d9f76b0b2
https://hg.mozilla.org/mozilla-central/rev/f8d5545d982b
https://hg.mozilla.org/mozilla-central/rev/903bbc357458
https://hg.mozilla.org/mozilla-central/rev/626aae1e53f9
https://hg.mozilla.org/mozilla-central/rev/c2e80cdf57fc
|
||
Comment 22•3 years ago
|
||
| bugherder | ||
Description
•