Closed Bug 1704851 Opened 8 months ago Closed 8 months ago

Use a RAII class for recursion checks

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
89 Branch
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.

The use of a C++ RAII class lets us simplify a few things and prepares for
the WASI changes in bug 1704771.

In EnterBaseline, it's simpler to use checkWithExtra.
After that we can inline CheckRecursionLimitWithStackPointer into checkWithExtra.

Depends on D111871

This is now a private method so is better encapsulated.

Depends on D111876

The next patch wants to remove JS_CHECK_STACK_SIZE, so stop calling it directly.

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

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

It's easier to understand the code withou these helpers.

Depends on D112001

Keywords: leave-open
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
Keywords: leave-open
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
You need to log in before you can comment on or make changes to this bug.