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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

+++ This bug was initially created as a clone of Bug #1534840 +++

I also have yet another similarly hard-to-reproduce issue with an Asan stack. This should be related to IonMonkey on ARM64.

It happened on m-c rev f7a15eb24f3d with the following runtime flags:

--fuzzing-safe --wasm-compiler=baseline --test-wasm-await-tier2 --no-asmjs --cache-ir-stubs=on --ion-pgo=off --ion-instruction-reordering=on --ion-regalloc=testbed --execute="setJitCompilerOption("ion.forceinlineCaches",1)" --ion-warmup-threshold=0 --ion-check-range-analysis --ion-eager --more-compartments --no-streams --spectre-mitigations=on --no-unboxed-objects --no-cgc --gc-zeal=12,246

and --enable-debug --disable-profiling --enable-simulator=arm64 compiled with Asan flags.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: testcase

Testcase that I cannot seem to manually reproduce either.

Comment 2

[Lars T Hansen [:lth]]

The wasm switches are presumably red herrings; there appears to be no wasm in the test.

Comment 3

[Sean Stangl [:sstangl]]

This is in the tableswitch code, which Nicolas just landed a patch to address. It seems that this occurs even with the patch.

We don't really need a reproducible testcase to figure out what's going on here, because the stack makes it obvious. The sequence of events is:

1. CodeGeneratorARM64::visitOutOfLineTableSwitch() creates an AutoForbidPoolsAndNops region of the entire table length.
2. EmitData() calls insertEntryForwards() which realizes that too many instructions have been passed, and we're about to become out of range for a pool.
3. EmitData() calls finishPool(). Because there is actually pool data that is about to become unreachable, it triggers a pool flush.
4. AutoForbidPoolsAndNops flipped a flag that prohibits pool flushes, so we crash.

The solution is to always forcibly flush a pool before the AutoForbid region. With that change, step #3 will be different -- finishPool() will see no pool data, and therefore will become a NOP.

Comment 4

[Sean Stangl [:sstangl]]

In theory this shouldn't happen because AutoForbidPoolsAndNops should see the maximum range and perform the pool-flushing behavior itself. The bug, then, is that this isn't happening for some reason.

Comment 5

[Nicolas B. Pierron [:nbp]]

For what is worth, I can reproduce this issue with Bug 1545379 test case.

Comment 6

[Nicolas B. Pierron [:nbp]]

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

In theory this shouldn't happen because AutoForbidPoolsAndNops should see the maximum range and perform the pool-flushing behavior itself. The bug, then, is that this isn't happening for some reason.

I concur on this idea, and manage to reproduce it by asserting the same condition which lead us to call finishPool inside enterNoPool. This assertion triggered while reproducing the issue, and I am still investigating why for the moment.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

nbp - are we expecting to try to fix this in 68, or is it low-likelihood/impact enough that we should fix-optional it?
Steven - same question, and what priority should it be?

Comment 8

[Steven DeTar [:sdetar]]

I am going to let Sean and Nicolas answer the questions as they have the information, I am not sure of the answer.

Comment 9

[Nicolas B. Pierron [:nbp]]

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #7)

nbp - are we expecting to try to fix this in 68, or is it low-likelihood/impact enough that we should fix-optional it?

Yes, there are patches in Bug 1546446 which are pending for review, and which I am expecting to also fix this issue.
So yes, we should track at least 68.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Going to assume bug 1546446 fixed this, we'll file new bugs later should the assert still appear.