Crash [@ js::jit::MacroAssembler::patchCall] or Assertion failure: vixl::is_int26(relTarget00), at js/src/jit/arm64/MacroAssembler-arm64.cpp:672
Categories
(Core :: JavaScript: WebAssembly, defect, P2)
Tracking
()
People
(Reporter: gkw, Assigned: lth)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [jsbugmon:][post-critsmash-triage][adv-main68+])
Crash Data
Attachments
(3 files, 1 obsolete file)
The following testcase crashes on mozilla-central revision 124ee436c421 (build with --enable-debug --enable-more-deterministic --enable-simulator=arm64, run with --fuzzing-safe --no-threads --no-baseline --no-ion w280-out.wrapper w280-out.wasm):
See attachment.
Backtrace:
#0 js::jit::MacroAssembler::patchCall (this=<optimized out>, callerOffset=<optimized out>, calleeOffset=<optimized out>)
at /home/ubuntu/trees/mozilla-central/js/src/jit/arm64/MacroAssembler-arm64.cpp:672
#1 0x00005555573e1ec1 in js::wasm::ModuleGenerator::linkCallSites (this=0x7fffffffa830)
at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:519
#2 0x00005555573e3556 in js::wasm::ModuleGenerator::finishCodegen (this=0x7fffffffa830)
at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:895
#3 0x00005555573e4310 in js::wasm::ModuleGenerator::finishCodeTier (this=0x7fffffffa830)
at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:1013
#4 0x00005555573e4e78 in js::wasm::ModuleGenerator::finishModule (this=0x7fffffffa830, bytecode=..., maybeTier2Listener=0x0)
at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:1100
#5 0x00005555573699c0 in js::wasm::CompileBuffer (args=..., bytecode=..., error=0x7fffffffbd40, warnings=<optimized out>, listener=0x0)
at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmCompile.cpp:578
/snip
For detailed crash information, see attachment.
This also crashes js opt shell ARM64 [@ js::jit::MacroAssembler::patchCall]. Not sure if this is wasm or ARM64-related but setting s-s for now and needinfo? from Lars as a start.
Reporter | ||
Comment 1•6 years ago
|
||
Due to skipped revisions, the first bad revision could be any of:
changeset: https://hg.mozilla.org/mozilla-central/rev/5014a6b01aea
user: Lars T Hansen
date: Tue Nov 28 10:05:36 2017 +0100
summary: Bug 1439404 - Wasm on ARM64: gating. r=bbouvier
changeset: https://hg.mozilla.org/mozilla-central/rev/56d7b668df45
user: Lars T Hansen
date: Tue Mar 06 16:29:42 2018 +0100
summary: Bug 1439404 - ARM64 simulator bugfix, add missing guard. r=bbouvier
changeset: https://hg.mozilla.org/mozilla-central/rev/27bc0a49fe15
user: Lars T Hansen
date: Fri Nov 17 13:17:29 2017 +0100
summary: Bug 1439404 - Wasm baseline, support the chunky ARM64 stack. r=bbouvier
changeset: https://hg.mozilla.org/mozilla-central/rev/32fdbeda9305
user: Lars T Hansen
date: Mon Nov 27 09:07:04 2017 +0100
summary: Bug 1439404 - Wasm baseline, fill in the porting APIs for ARM64. r=bbouvier
Lars, is bug 1439404 a likely regressor?
Assignee | ||
Comment 2•6 years ago
|
||
Gary, test case?
It looks like this is another one of those "code is too large for architecture" problems, we're seeing quite a few of these now because we're looking and we haven't been looking before (and Wasm makes this problem much worse than JS, since we compile entire modules into single blobs of code and don't always handle the limitations correctly).
Reporter | ||
Comment 3•6 years ago
|
||
Oops, sorry for missing this out.
Assignee | ||
Comment 4•6 years ago
|
||
Easy to repro here.
Assignee | ||
Comment 5•6 years ago
|
||
Architecture-arm64.h:
// TODO:
// This constant needs to be updated to account for whatever near/far branching
// strategy is used by ARM64.
static const uint32_t JumpImmediateRange = UINT32_MAX;
/me greps for TODO in jit/arm64 and finds 46 instances, this is the first one, others are actionable.
/me greps for FIXME in jit/arm64 and finds 15 instances, a few are actionable.
Assignee | ||
Comment 6•6 years ago
|
||
There are comments in the code suggesting that we've made plans at some point to handle very far jumps via patching + indirect jumps, but all of those comments are TODO/FIXME. Absent such a strategy, the furthest jump is 2^27-1 bytes, and we need to define JumpImmediateRange to reflect that.
Assignee | ||
Comment 7•6 years ago
|
||
This is a MOZ_RELEASE_ASSERT and hence this is not really a security problem, but the cluster of problems the bug belongs to is mildly security-sensitive, so keeping the s-s setting and setting sec-low.
Assignee | ||
Comment 8•6 years ago
|
||
Comment 9•6 years ago
|
||
Comment 10•6 years ago
|
||
Is this something we should consider for Beta uplift or can it just ride the trains?
Assignee | ||
Comment 11•6 years ago
|
||
I don't think we should uplift, for a couple of reasons. First, we're being stopped from doing anything bad here by a MOZ_RELEASE_ASSERT and that's fine; the test case is synthetic and there's no exploit path, nor any real-world hardship at this point. Second, this is part of a cluster of patches and I'm not sure yet what else we'd need to uplift. I think if any of the other patches demand it we could uplift this, but it's more likely that other patches will make this patch even less critial.
Updated•6 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Comment 12•5 years ago
|
||
Compile with --enable-debug --enable-more-deterministic --enable-simulator=arm64 and run with --fuzzing-safe --no-threads --no-baseline --no-ion w280-out.wrapper w280-out.wasm on m-c rev 124ee436c421:
~/js-dbg-64-dm-armsim64-linux-x86_64-124ee436c421 --fuzzing-safe --no-threads --no-baseline --no-ion w280-out.wrapper w280-out.wasm
Assertion failure: vixl::is_int26(relTarget00), at /home/fuzz3/trees/mozilla-central/js/src/jit/arm64/MacroAssembler-arm64.cpp:672
Segmentation fault
Reduced using wasm-reduce.
Assignee | ||
Comment 13•5 years ago
|
||
Unable to repro the test case in comment 12.
Reporter | ||
Comment 14•5 years ago
|
||
(In reply to Lars T Hansen [:lth] from comment #13)
Unable to repro the test case in comment 12.
No action needed, m-c rev 124ee436c421 did not contain this fix, as I just wanted to practise getting a smaller wasm testcase off the large one in comment 3.
If you wanted to try and reproduce the problem, you'd have to try m-c rev 124ee436c421 and not tip (which contains the fix via rev c13b55d67ea2).
Updated•4 years ago
|
Description
•