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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Comment 1

[Nicolas B. Pierron [:nbp]]

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.

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: IonAssemblerBufferWithConstantPools skip no-pool constraints after running out-of-memory.

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.

Comment 3

[Sean Stangl [: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.

Comment 4

[Nicolas B. Pierron [:nbp]]

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

2. 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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d949e9cf60ab
IonAssemblerBufferWithConstantPools skip no-pool constraints after running out-of-memory. r=sstangl

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/d949e9cf60ab