1704851 - Use a RAII class for recursion checks

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

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.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 1 - Change CheckRecursionLimit to AutoCheckRecursionLimit::check. r?tcampbell!

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

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 2 - Change CheckRecursionLimitConservative to AutoCheckRecursionLimit::checkConservative. r?tcampbell!

Depends on D111868

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 3 - Change CheckSystemRecursionLimit to AutoCheckRecursionLimit::checkSystem. r?tcampbell!

Depends on D111869

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 4 - Change CheckRecursionLimitWithExtra to AutoCheckRecursionLimit::checkWithExtra. r?tcampbell!

Depends on D111870

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 5 - Remove CheckRecursionLimitWithStackPointer. r?tcampbell!

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

Depends on D111871

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 6 - Change CheckRecursionLimitWithStackPointerDontReport to AutoCheckRecursionLimit::checkWithStackPointerDontReport. r?tcampbell!

Depends on D111872

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 7 - Change CheckRecursionLimitDontReport to AutoCheckRecursionLimit::checkDontReport. r?tcampbell!

Depends on D111874

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 8 - Change CheckRecursionLimitConservativeDontReport to AutoCheckRecursionLimit::checkConservativeDontReport. r?tcampbell!

Depends on D111875

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 9 - Change CheckRecursionLimitDontReport to AutoCheckRecursionLimit::checkLimitDontReport. r?tcampbell!

This is now a private method so is better encapsulated.

Depends on D111876

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 10 - Change CheckRecursionLimit to AutoCheckRecursionLimit::checkLimit. r?tcampbell!

Depends on D111877

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 11 - Change GetNativeStackLimit to AutoCheckRecursionLimit::getStackLimit. r?tcampbell!

Depends on D111878

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 12 - Change GetNativeStackLimitHelper to AutoCheckRecursionLimit::getStackLimitHelper. r?tcampbell!

Depends on D111879

Comment 13

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 13 - Change RunningWithTrustedPrincipals to AutoCheckRecursionLimit::runningWithTrustedPrincipals. r?tcampbell!

Depends on D111880

Comment 14

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 14 - Fix and improve comment. r?tcampbell!

Depends on D111882

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 15 - Clean up stack overflow check in ComponentFinder. r?tcampbell!

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

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 16 - Replace JS_CHECK_STACK_SIZE macro with checkLimitImpl. r?tcampbell!

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

Comment 17

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 17 - Optimize slow getStackLimit call in more cases. r?tcampbell!

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

Comment 18

[Jan de Mooij [:jandem]]

Attachment: Bug 1704851 part 18 - Remove checkLimit and checkLimitDontReport helpers. r?tcampbell!

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

Depends on D112001

Comment 19

[Pulsebot]

Pushed by [e-mail]:
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

Comment 20

[Pulsebot]

Pushed by [e-mail]:
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

[Cosmin Sabou [:CosminS]]

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

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5d6cf556bc48
https://hg.mozilla.org/mozilla-central/rev/1f2846c1c6ad
https://hg.mozilla.org/mozilla-central/rev/043dfe053910
https://hg.mozilla.org/mozilla-central/rev/d3acac58e972