Closed Bug 1518181 Opened 11 months ago Closed 11 months ago

ARM64 IonMonkey: Assertion failure: !canNotPlacePool_, at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:971

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM64
Unspecified
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: nbp, Assigned: nbp)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Summary: Assertion failure: !canNotPlacePool_, at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:971 → ARM64 IonMonkey: Assertion failure: !canNotPlacePool_, at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:971
Status: NEW → ASSIGNED

The issue I looked at when investigating failures of promise/bug-1298776.js test case is an OOM.

When calling enterNoPool > finishPool, we oom after reserving the space for the guards and return to enterNoPool which set the "canoNotPlacePool_" flag.

Then we looking at the next instruction, we trip over the same limit and attempt to allocate space for the pool, but fail because we assert that we are not in a no-pool region.

When running out-of-memory, we are no longer allowed to read the content of the
MacroAssembler buffer, as such we can safely ignore the soundness of the
enterNoPool and leaveNoPool scopes if we ran out of memory.
Attachment #9034784 - Flags: review?(sstangl)
Comment on attachment 9034784 [details] [diff] [review]
IonAssemblerBufferWithConstantPools skip no-pool constraints after running out-of-memory.

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

::: js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
@@ +1080,5 @@
>      canNotPlacePool_ = true;
>    }
>  
>    void leaveNoPool() {
> +    if (this->oom()) {

What happens if OOM occurs *between* enterNoPool() and leaveNoPool()?

There is a small number of functions that assert on canNotPlacePool_ before checking for OOM.

There are two options:

1.  Those functions could be changed such that the assertions occur after the OOM check. This has the downside that the assertion is no longer a strong invariant.

2. In this function, we could add `canNotPlacePool_ = false` to OOM path.

I think I prefer the second option, but leaving to your discretion.
Attachment #9034784 - Flags: review?(sstangl) → review+

(In reply to Sean Stangl [:sstangl] from comment #3)

  1. In this function, we could add canNotPlacePool_ = false to OOM path.

I think I prefer the second option, but leaving to your discretion.

I will go for the second option, as this seems to be the less intrusive option.

Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d949e9cf60ab
IonAssemblerBufferWithConstantPools skip no-pool constraints after running out-of-memory. r=sstangl
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.