JSRuntime::jitStackLimit_ keeps UINTPTR_MAX when selfhosting RegExp.prototype.exec.

RESOLVED FIXED in Firefox 44

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: arai, Assigned: jandem)

Tracking

Trunk
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(2 attachments)

(this is not an ongoing bug in m-c)

While selfhosting RegExp.prototype.exec in bug 887016, I hit strange behavior related to JSRuntime::jitStackLimit_.

Attached patch adds selfhosted function for RegExp.prototype.exec, and it calls regexp_exec (just for testing, actual patch does more optimization), and adds some code for testing.  test.js in the attached patch calls RegExp.prototype.exec 50000 times, it's supposed to be inlined and most of them is executed in fully jitted code.

With the patch applied, however, executing test.js prints following:
  regexp_exec: 999, regex_exec_raw: 44463, stack_ng: 44436
each number means:
  * regexp_exec [1] was called 999 times
  * regex_exec_raw [2] was called 44463 times
  * stack check in NativeRegExpMacroAssembler::GenerateCode [3] failed 44436 times
so, JSRuntime::jitStackLimit_ is changed to UINTPTR_MAX, and never changed back from some point, LRegExpExec fallbacks to OutOfLineRegExpExec every time after that.

The issue disappears by reverting changes to regexp_methods, so it seems to be related to selfhosted code.  Also, it disappears by removing |if (!IsObject(R)) ...| or |var lastIndex = ...| from RegExp_Exec in RegExp.js, so the size of selfhosted function or interaction between them and regexp_exec seems to be related.

What could be the possible reason?

[1] https://dxr.mozilla.org/mozilla-central/rev/6457f01e4bcb818060f89c3b80ae0596e1583345/js/src/builtin/RegExp.cpp#876
[2] https://dxr.mozilla.org/mozilla-central/rev/6457f01e4bcb818060f89c3b80ae0596e1583345/js/src/builtin/RegExp.cpp#884
[3] https://dxr.mozilla.org/mozilla-central/rev/6457f01e4bcb818060f89c3b80ae0596e1583345/js/src/irregexp/NativeRegExpMacroAssembler.cpp#160
this issue doesn't happend when adding one of--ion-offthread-compile=off, --ion-osr=off, or --no-threads.
Jandem, maybe you can take a look?
Flags: needinfo?(jdemooij)
What's happening here is that we trigger an interrupt to link compiled Ion code and set the stack limit to UINTPTR_MAX to interrupt JIT code. Normal JIT code then calls CheckOverRecursed, which does the work and resets the limit etc.

RegExp JIT code OTOH checks the stack limit but doesn't call CheckOverRecursed, so we keep failing the check.

Good catch, I can fix this.
(In reply to Jan de Mooij [:jandem] from comment #3)
> RegExp JIT code OTOH checks the stack limit but doesn't call
> CheckOverRecursed, so we keep failing the check.

And usually there's no problem because the slow path will do the check after entering regex code from the VM, but in this case the slow path can do a simple string match without invoking the regex engine...
Posted patch PatchSplinter Review
Two ways to fix this:

(1) Add interrupt checks to js::regexp_exec_raw and js::regexp_test_raw, the slow paths we use when invoking irregexp code from Ion.

(2) Use a separate stack limit that's not reset on interrupts.

This patch does (2).
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8672259 - Flags: review?(bhackett1024)
Comment on attachment 8672259 [details] [diff] [review]
Patch

Review of attachment 8672259 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay.
Attachment #8672259 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/b6018e109399
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.