Closed Bug 1546874 Opened 6 years ago Closed 6 years ago

Assertion failure: !used(), at js/src/jit/Label.h:86

Categories

(Core :: JavaScript Engine, defect, P1)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: gkw, Assigned: lth)

References

(Regression)

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision c7a9affeb604 (build with PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig 'CC="clang -m32 -msse2 -mfpmath=sse"' AR=ar 'CXX="clang++ -m32 -msse2 -mfpmath=sse"' sh ./configure --target=i686-pc-linux --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests --disable-cranelift, run with --fuzzing-safe --no-threads --no-baseline --no-ion w301-out.wrapper w301-out.wasm):

See attachment.

Backtrace:

#0 js::jit::Label::~Label (this=0xf43) at js/src/jit/Label.h:86
#1 GenerateImportJitExit (masm=..., fi=..., funcImportIndex=4, throwLabel=0xd30, offsets=0x6c0) at js/src/wasm/WasmStubs.cpp:1728
#2 js::wasm::GenerateStubs (env=..., imports=..., exports=..., code=0xf6b2e298) at js/src/wasm/WasmStubs.cpp:2218
#3 0x58466615 in js::wasm::ModuleGenerator::finishCodeTier (this=0xff833b50) at js/src/wasm/WasmGenerator.cpp:1023
#4 0x58467231 in js::wasm::ModuleGenerator::finishModule (this=0xff833b50, bytecode=..., maybeTier2Listener=0x0) at js/src/wasm/WasmGenerator.cpp:1121
/snip

For detailed crash information, see attachment.

Setting s-s as a start because a similar previously-filed bug 1535482 was also s-s.

I'm not sure why I can't attach the testcases here (b.m.o seems to error out), but I've sent them to Lars over email.

Flags: needinfo?(lhansen)

The previous error was specific to the ARM assembler and it's more likely that this is an improperly handled exception, since it's x86 only.

Flags: needinfo?(lhansen)

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/01a5d9506bb4
user: Nicolas B. Pierron
date: Mon Jul 16 18:13:51 2018 +0000
summary: Bug 1473289 - Work around page table fragmentation caused by mprotect / VirtualProtect using LifoAlloc r=tcampbell

Nicolas, is bug 1473289 a likely regressor?

Component: Javascript: WebAssembly → JavaScript Engine
Flags: needinfo?(nicolas.b.pierron)
Regressed by: 1473289

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #4)

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/01a5d9506bb4
user: Nicolas B. Pierron
date: Mon Jul 16 18:13:51 2018 +0000
summary: Bug 1473289 - Work around page table fragmentation caused by mprotect / VirtualProtect using LifoAlloc r=tcampbell

Nicolas, is bug 1473289 a likely regressor?

This is sounds unlikely. However this patch changes a mechanism meant to protect LifoAlloc buffer against corruption.
However, I would not expect this patch to highlight new failure but to remove interpreter crashes due to the abuse of mprotect.

I know WASM is using LifoAllocScope-like mechanism, and it might be a use-after-free of what remain in the LifoAlloc chunks.

Flags: needinfo?(nicolas.b.pierron)

(In reply to Nicolas B. Pierron [:nbp] from comment #5)

I know WASM is using LifoAllocScope-like mechanism, and it might be a use-after-free of what remain in the LifoAlloc chunks.

I guess the ball is now back in wasm's court?

Flags: needinfo?(lhansen)

Looking.

Flags: needinfo?(lhansen)

It's just a lost OOM in the assembler buffer that triggers the assert. The buffer would exceed 2^27-1 in length. The error return is not lost, it's just not recorded properly. No cause for alarm.

I'll see if I can use the same mechanism as I used on ARM to handle this.

This copies the mechanism that was introduced for ARM into the x86 code: it
uses a flag on the jitcontext, if present, to propagate the OOM flag so that
we won't assert in ~Label.

If including Ion.h into this header file is not good then I can move
oomDetected() into the .cpp file.

Priority: -- → P1
Group: javascript-core-security
Pushed by lhansen@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1ac79bd52a11 Propagate OOM status from x86 assembler buffer. r=nbp
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → lhansen
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: