Assertion failure: inhibitNops_ && (isPoolEmptyFor(InstSize) || canNotPlacePool_), at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:1121 with Baseline Interpreter on the stack
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | --- | wontfix |
firefox69 | --- | wontfix |
firefox70 | --- | wontfix |
firefox71 | --- | fixed |
People
(Reporter: gkw, Assigned: jandem)
References
(Regression)
Details
(4 keywords, Whiteboard: [jsbugmon:])
Attachments
(2 files)
I don't have a good testcase for this but it reproduced in m-c rev f6a1009171e5 - all testcases so far are non-reproducible.
Backtrace:
#0 js::jit::AssemblerBufferWithConstantPools<1024u, 4u, js::jit::Instruction, js::jit::Assembler, 0u>::assertNoPoolAndNoNops (this=0x1affd9a8) at js/src/jit/shared/IonAssemblerBufferWithConstantPools.h:1121
#1 js::jit::Assembler::writeCodePointer (this=0x1affe554, label=0x1affdfe8) at js/src/jit/arm/Assembler-arm.cpp:781
#2 0x5815dfd9 in js::jit::BaselineInterpreterGenerator::emitInterpreterLoop (this=0x1affe4e8) at js/src/jit/BaselineCodeGen.cpp:7019
#3 0x5815e9cc in js::jit::BaselineInterpreterGenerator::generate (this=0x1affe4e8, interpreter=...) at js/src/jit/BaselineCodeGen.cpp:7071
#4 0x582e2153 in js::jit::GenerateBaselineInterpreter (cx=0xd70aa800, interpreter=...) at js/src/jit/BaselineJIT.cpp:1049
#5 0x583c236e in js::jit::JitRuntime::initialize (this=0x17c10000, cx=0xd70aa800) at js/src/jit/Ion.cpp:218
#6 0x57aa36af in JSRuntime::createJitRuntime (this=0x2d111000, cx=0xd70aa800) at js/src/vm/Realm.cpp:138
#7 0x57d301e6 in JS::InitSelfHostedCode (cx=0xd70aa800) at js/src/jsapi.cpp:428
/snip
For detailed crash information, see attachment.
This has appeared since m-c rev 872c3c79f906 in July 2019 and happens on 32-bit ARM simulator, 64-bit ARM64 simulator and aarch64 Linux shells, so I'm guessing this is related to ARM and baseline interpreter code interactions.
Jan, is this likely related to bug 1567327 based on the stack?
Comment 1•5 years ago
|
||
I was the one to add this assertion. What it means is that we are attempting to write a jump table, but we did no protected the jump table region with AutoForbidPoolsAndNops
, or we called it with not enough space which could cause a pool entry to be inserted.
This assertion highlight issues which could lead to random memory execution, by inserting random bytes in the expected sequence of a jump table. However, we might downgrade the sec-critical status if there is no user-controlled bits executed before the GenerateBaselineInterpreter
function. I will let Jan judge that.
Assignee | ||
Comment 2•5 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
I was the one to add this assertion. What it means is that we are attempting to write a jump table, but we did no protected the jump table region with
AutoForbidPoolsAndNops
, or we called it with not enough space which could cause a pool entry to be inserted.
It should be called with enough space here: https://searchfox.org/mozilla-central/rev/8ea946dcf51f0d6400362cc1d49c8d4808eeacf1/js/src/jit/BaselineCodeGen.cpp#7007-7008 Does anything there look wrong to you?
I wonder if it could be a weird OOM issue; the interpreter code is like a trampoline and should basically be identical each time it's generated.
Comment 3•5 years ago
|
||
I looked at the code which is being called. I do not see what might be going wrong. The reserved amount of instructions sounds correct.
I will just note that the AssemblerBufferWithConstantPools is way to complex for what it is supposed to be doing.
Assignee | ||
Comment 4•5 years ago
|
||
I think it's an OOM issue and we just need to fix the assertion to take that into account.
Gary, if I give you a patch can you confirm that, or is it too unreliable for you to tell?
Reporter | ||
Comment 5•5 years ago
|
||
Too unreliable I think, it happens once every few days or week? I'd say land the patch and we can follow up as necessary.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
Most of the pool-related methods return early on OOM.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3c352d78e1c5 Fix assertion in assertNoPoolAndNoNops to account for OOM. r=nbp
Comment 8•5 years ago
|
||
bugherder |
Comment 9•5 years ago
|
||
Do you want to request uplift to beta? If not let's mark it wontfix but I'm happy to take a patch now in early beta.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 10•5 years ago
|
||
(In reply to Liz Henry (:lizzard) from comment #9)
Do you want to request uplift to beta? If not let's mark it wontfix but I'm happy to take a patch now in early beta.
This is a debug only assertion, so unless this is required by fuzzers, I suggest to only follow the train.
Comment 11•5 years ago
|
||
We don't perform beta uplifts for fuzzing-only anymore, so this can ride the trains.
Updated•5 years ago
|
Updated•2 years ago
|
Description
•