Closed Bug 1546050 Opened 6 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript Engine, defect, P1)

ARM64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla69
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)

Attached file stack (deleted) —

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

Flags: needinfo?(sstangl)
Flags: needinfo?(nicolas.b.pierron)
Attached file testcase (deleted) —

Testcase that I cannot seem to manually reproduce either.

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

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.

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.

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

Flags: needinfo?(nicolas.b.pierron)

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

Depends on: 1546446
Depends on: 1548843

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?

Flags: needinfo?(sdetar)
Flags: needinfo?(nicolas.b.pierron)

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

Flags: needinfo?(sdetar)

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

Flags: needinfo?(nicolas.b.pierron)
Priority: -- → P1

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

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(sstangl)
Resolution: --- → FIXED
Assignee: nobody → nicolas.b.pierron
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: