Assertion failure: !canNotPlacePool_, at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:971
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox67 | --- | unaffected |
firefox68 | --- | fixed |
firefox69 | --- | fixed |
People
(Reporter: gkw, Assigned: nbp)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [jsbugmon:])
Attachments
(2 files)
+++ 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.
Reporter | ||
Comment 1•6 years ago
|
||
Testcase that I cannot seem to manually reproduce either.
Comment 2•6 years ago
|
||
The wasm switches are presumably red herrings; there appears to be no wasm in the test.
Comment 3•6 years ago
|
||
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:
CodeGeneratorARM64::visitOutOfLineTableSwitch()
creates anAutoForbidPoolsAndNops
region of the entire table length.EmitData()
callsinsertEntryForwards()
which realizes that too many instructions have been passed, and we're about to become out of range for a pool.EmitData()
callsfinishPool()
. Because there is actually pool data that is about to become unreachable, it triggers a pool flush.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•6 years ago
|
||
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.
Assignee | ||
Comment 5•6 years ago
|
||
For what is worth, I can reproduce this issue with Bug 1545379 test case.
Assignee | ||
Comment 6•6 years ago
|
||
(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•6 years ago
|
||
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•6 years ago
|
||
I am going to let Sean and Nicolas answer the questions as they have the information, I am not sure of the answer.
Assignee | ||
Comment 9•6 years ago
|
||
(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.
Reporter | ||
Comment 10•5 years ago
|
||
Going to assume bug 1546446 fixed this, we'll file new bugs later should the assert still appear.
Updated•5 years ago
|
Description
•