Closed Bug 1526579 Opened 1 year ago Closed 1 year ago

Assertion failure: IsWordAligned(pc_), at js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:191 or Assertion failure: entryStack == exitStack, at js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:279


(Core :: Javascript: WebAssembly, defect, critical)

Not set



Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 + fixed
firefox67 + fixed


(Reporter: gkw, Assigned: lth)


(Blocks 3 open bugs)


(5 keywords, Whiteboard: [jsbugmon:][post-critsmash-triage][adv-main66+])


(8 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision 4bc31addf415 (build with --enable-debug --enable-simulator=arm64, run with --fuzzing-safe --no-threads --ion-eager w593-out.wrapper w593-out.wasm):

See attachment.


(gdb) bt
#0 vixl::Simulator::ExecuteInstruction (this=0x7ffff5539800) at /home/fuzz3/trees/mozilla-central/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:191
#1 0x0000555557010858 in vixl::Simulator::Run (this=0x7ffff5539800) at /home/fuzz3/trees/mozilla-central/js/src/jit/arm64/vixl/Simulator-vixl.cpp:70
#2 0x0000555557002ff0 in vixl::Simulator::call (this=0x7ffff5539800, entry=0x1cdd73677280 "\377C", argument_count=<optimized out>) at /home/fuzz3/trees/mozilla-central/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:277
#3 0x00005555573b750c in js::wasm::Instance::callExport (this=0x7ffff1108020, cx=<optimized out>, funcIndex=<optimized out>, args=...) at /home/fuzz3/trees/mozilla-central/js/src/wasm/WasmInstance.cpp:1411
#4 0x00005555573d07a1 in WasmCall (cx=0x7ffff5518000, argc=<optimized out>, vp=<optimized out>) at /home/fuzz3/trees/mozilla-central/js/src/wasm/WasmJS.cpp:1398
#5 0x000055555661cd30 in CallJSNative (cx=0x7ffff5518000, native=0x5555573d0670 <WasmCall(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/fuzz3/trees/mozilla-central/js/src/vm/Interpreter.cpp:442

For detailed crash information, see attachment.

Setting s-s as a start. The assertion involves alignment of words which may involve memory addresses, plus I'm not sure if this is specific to the ARM64 simulator.

Attached file Testcase

Due to skipped revisions, the first bad revision could be any of:
user: Lars T Hansen
date: Tue Nov 28 10:05:36 2017 +0100
summary: Bug 1439404 - Wasm on ARM64: gating. r=bbouvier

user: Lars T Hansen
date: Tue Mar 06 16:29:42 2018 +0100
summary: Bug 1439404 - ARM64 simulator bugfix, add missing guard. r=bbouvier

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

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?

Blocks: 1439404
Flags: needinfo?(lhansen)

Well, "regressor". This landed functionality that is necessary for the compiler to work on ARM64 at all. There may be a bug.

Flags: needinfo?(lhansen)

Reproduces in a straight arm64 debug build with --no-ion --no-baseline too so it's unlikely that it's the JS jit fouling things up for us.

Looks like a stray SP adjustment in rabaldr-compiled code.

Something goes wrong with code generation in the function called "4786". Here's a trace from the simulator (source is below), the stack height coming into the function is "16 + (9 * 32) + 16 + 16 + 32", corresponding to adjustments made along the calling path.

There's a weird intermixing of straight-line code with stuff that appears to store into the frame near the TOS at decreasing offsets, and then an incorrect stack adjustment, start at "Weirdness".

0x00002a44c8f6f4f0  d10083ff		sub     sp, sp, #0x20 (32)  // prologue starts here - not interesting

16 + (9 * 32) + 16 + 16 + 32 + 32

0x00002a44c8f6f4f4  f9000ffe		str     x30, [sp, #24]
0x00002a44c8f6f4f8  f90007f7		str     x23, [sp, #8]
0x00002a44c8f6f4fc  f90003fd		str     x29, [sp]
0x00002a44c8f6f500  910003fd		mov     x29, sp
0x00002a44c8f6f504  d2801610		mov     x16, #0xb0
0x00002a44c8f6f508  f2a00010		movk    x16, #0x0, lsl #16
0x00002a44c8f6f50c  cb3063e8		sub     x8, sp, x16
0x00002a44c8f6f510  f94016f0		ldr     x16, [x23, #40]
0x00002a44c8f6f514  eb08021f		cmp     x16, x8
0x00002a44c8f6f518  54000043		b.lo    #+0x8 (addr 0x2a44c8f6f520)
0x00002a44c8f6f520  d101c3ff		sub     sp, sp, #0x70 (112)

16 + (9 * 32) + 16 + 16 + 32 + 32 + 112

0x00002a44c8f6f524  d2800000		mov     x0, #0x0
0x00002a44c8f6f528  f90037e0		str     x0, [sp, #104]
0x00002a44c8f6f52c  f90033e0		str     x0, [sp, #96]
0x00002a44c8f6f530  f9002fe0		str     x0, [sp, #88]
0x00002a44c8f6f534  f9002be0		str     x0, [sp, #80]
0x00002a44c8f6f538  f90027e0		str     x0, [sp, #72]
0x00002a44c8f6f53c  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f540  b94631e0		ldr     w0, [x15, #1584] // get global
0x00002a44c8f6f544  7100001f		cmp     w0, #0x0 (0)     // eqz
0x00002a44c8f6f548  54000061    #+0xc (addr 0x2a44c8f6f554)
0x00002a44c8f6f554  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f558  b94631e0		ldr     w0, [x15, #1584] // get global
0x00002a44c8f6f55c  51000400		sub     w0, w0, #0x1 (1) // sub 1
0x00002a44c8f6f560  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f564  b90631e0		str     w0, [x15, #1584] // set global
0x00002a44c8f6f568  d503201f		nop                      // loop head
0x00002a44c8f6f56c  d503201f		nop                      //   align
0x00002a44c8f6f570  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f574  b94031f0		ldr     w16, [x15, #48]  // get interupt flag
0x00002a44c8f6f578  7100021f		cmp     w16, #0x0 (0)    //   and test
0x00002a44c8f6f57c  54000040		b.eq    #+0x8 (addr 0x2a44c8f6f584)
0x00002a44c8f6f584  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f588  b94631e0		ldr     w0, [x15, #1584] // get global
0x00002a44c8f6f58c  7100001f		cmp     w0, #0x0 (0)     // eqz
0x00002a44c8f6f590  54000081    #+0x10 (addr 0x2a44c8f6f5a0)
0x00002a44c8f6f5a0  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f5a4  b94631e0		ldr     w0, [x15, #1584] // get global
0x00002a44c8f6f5a8  51000400		sub     w0, w0, #0x1 (1) // sub 1
0x00002a44c8f6f5ac  f9403fef		ldr     x15, [sp, #120]  // get tls
0x00002a44c8f6f5b0  b90631e0		str     w0, [x15, #1584] // get global
0x00002a44c8f6f5b4  52800bc0		mov     w0, #0x5e        // const 94
0x00002a44c8f6f5b8  b9006fe0		str     w0, [sp, #108]   // tee_local
0x00002a44c8f6f5bc  7100001f		cmp     w0, #0x0 (0)     // if
0x00002a44c8f6f5c0  54000280		b.eq    #+0x50 (addr 0x2a44c8f6f610)
0x00002a44c8f6f5c4  5282c220		mov     w0, #0x1611      // 5649
0x00002a44c8f6f5c8  b9006fe0		str     w0, [sp, #108]   // tee_local 0
0x00002a44c8f6f5cc  b9006fe0		str     w0, [sp, #108]   // tee_local 0
0x00002a44c8f6f5d0  528302a0		mov     w0, #0x1815      // 51845141
0x00002a44c8f6f5d4  72a062e0		movk    w0, #0x317, lsl #16   //   part 2
0x00002a44c8f6f5d8  f9403ff7		ldr     x23, [sp, #120]  // setup call
0x00002a44c8f6f5dc  f9402ae8		ldr     x8, [x23, #80]
0x00002a44c8f6f5e0  f94032e9		ldr     x9, [x23, #96]
0x00002a44c8f6f5e4  f94012ea		ldr     x10, [x23, #32]
0x00002a44c8f6f5e8  f9004949		str     x9, [x10, #144]
0x00002a44c8f6f5ec  f9402ef7		ldr     x23, [x23, #88]
0x00002a44c8f6f5f0  f94002f5		ldr     x21, [x23]
0x00002a44c8f6f5f4  d63f0100		blr     x8               // call

(This is a call to imported function log-i32; it appears to do the right thing with the stack)
(So now we're back)

0x00002a44c8f6f5f8  f9403ff7		ldr     x23, [sp, #120]
0x00002a44c8f6f5fc  f94002f5		ldr     x21, [x23]
0x00002a44c8f6f600  f94012e8		ldr     x8, [x23, #32]
0x00002a44c8f6f604  f9400ee9		ldr     x9, [x23, #24]
0x00002a44c8f6f608  f9004909		str     x9, [x8, #144]
0x00002a44c8f6f60c  14000055		b       #+0x154 (addr 0x2a44c8f6f760)
0x00002a44c8f6f760  52801000		mov     w0, #0x80       // 128
0x00002a44c8f6f764  b9006fe0		str     w0, [sp, #108]  // tee_local
0x00002a44c8f6f768  5283c121		mov     w1, #0x1e09     // 403578377
0x00002a44c8f6f76c  72a301c1		movk    w1, #0x180e, lsl #16
0x00002a44c8f6f770  b9006fe1		str     w1, [sp, #108]  // tee_local
0x00002a44c8f6f774  b9006fe1		str     w1, [sp, #108]  // tee_local
0x00002a44c8f6f778  52826002		mov     w2, #0x1300     // 538120960
0x00002a44c8f6f77c  72a40262		movk    w2, #0x2013, lsl #16

*** Weirdness

0x00002a44c8f6f780  f9001fe0		str     x0, [sp, #56]   // should be tee_local 0, but offset is wrong
0x00002a44c8f6f784  f9001be1		str     x1, [sp, #48]   // and again
0x00002a44c8f6f788  b9406fef		ldr     w15, [sp, #108] // but here this is a sensible get_local 0
0x00002a44c8f6f78c  f90017ef		str     x15, [sp, #40]  // but not a sensible tee_local 0
0x00002a44c8f6f790  b9006fe2		str     w2, [sp, #108]  // tee_local 0
0x00002a44c8f6f794  b9006fe2		str     w2, [sp, #108]  // tee_local 0
0x00002a44c8f6f798  b9406fe0		ldr     w0, [sp, #108]  // get_local 0
0x00002a44c8f6f79c  f90013e2		str     x2, [sp, #32]   // and again some weird store at a lower address!
0x00002a44c8f6f7a0  b9406fef		ldr     w15, [sp, #108]  // get_local 0
0x00002a44c8f6f7a4  f9000fef		str     x15, [sp, #24]  // and another one
0x00002a44c8f6f7a8  b9006fe0		str     w0, [sp, #108]   // tee_local 0
0x00002a44c8f6f7ac  f9000be0		str     x0, [sp, #16]    // and another one!
0x00002a44c8f6f7b0  f9403fef		ldr     x15, [sp, #120]   // get tls
0x00002a44c8f6f7b4  b94031f0		ldr     w16, [x15, #48]   // get interrupt flag
0x00002a44c8f6f7b8  7100021f		cmp     w16, #0x0 (0)     // check it
0x00002a44c8f6f7bc  54000040		b.eq    #+0x8 (addr 0x2a44c8f6f7c4)
0x00002a44c8f6f7c4  f9403fef		ldr     x15, [sp, #120]   // get tls
0x00002a44c8f6f7c8  b94631e0		ldr     w0, [x15, #1584]  // global 4
0x00002a44c8f6f7cc  7100001f		cmp     w0, #0x0 (0)      // eqz
0x00002a44c8f6f7d0  54000061    #+0xc (addr 0x2a44c8f6f7dc)
0x00002a44c8f6f7dc  f9403fef		ldr     x15, [sp, #120]   // tls
0x00002a44c8f6f7e0  b94631e0		ldr     w0, [x15, #1584]  // global 4
0x00002a44c8f6f7e4  51000400		sub     w0, w0, #0x1 (1)  // sub 1
0x00002a44c8f6f7e8  f9403fef		ldr     x15, [sp, #120]   // tls
0x00002a44c8f6f7ec  b90631e0		str     w0, [x15, #1584]  // set_global 4
0x00002a44c8f6f7f0  52800400		mov     w0, #0x20         // 32
                                         &lt;falls out of loop here>
0x00002a44c8f6f7f4  528001e1		mov     w1, #0xf          // 15
0x00002a44c8f6f7f8  f90007e0		str     x0, [sp, #8]      // ???  Another weirdo thing?
0x00002a44c8f6f7fc  7100003f		cmp     w1, #0x0 (0)      // 15 == 0?
0x00002a44c8f6f800  540001e1    #+0x3c (addr 0x2a44c8f6f83c)
0x00002a44c8f6f83c  52a40000		mov     w0, #0x20000000   // 536870912
0x00002a44c8f6f840  f9000be0		str     x0, [sp, #16]     // 
0x00002a44c8f6f844  f9400be1		ldr     x1, [sp, #16]
0x00002a44c8f6f848  f9400fe0		ldr     x0, [sp, #24]
0x00002a44c8f6f84c  7100003f		cmp     w1, #0x0 (0)      // br_if
0x00002a44c8f6f850  54000060		b.eq    #+0xc (addr 0x2a44c8f6f85c)
0x00002a44c8f6f854  910043ff		add     sp, sp, #0x10 (16)

// Should have done 112 here!  So this adjustment is bogus, the 112 comes below
16 + (9 * 32) + 16 + 16 + 32 + 32 + 112

0x00002a44c8f6f858  140001e3		b       #+0x78c (addr 0x2a44c8f6ffe4)
0x00002a44c8f6ffe4  14000009		b       #+0x24 (addr 0x2a44c8f70008)
0x00002a44c8f70008  9101c3ff		add     sp, sp, #0x70 (112)
0x00002a44c8f7000c  f94003fd		ldr     x29, [sp]
0x00002a44c8f70010  f94007f7		ldr     x23, [sp, #8]
0x00002a44c8f70014  f9400ffe		ldr     x30, [sp, #24]
0x00002a44c8f70018  910083ff		add     sp, sp, #0x20 (32)
0x00002a44c8f7001c  d65f03c0		ret
Assertion failure: entryStack == exitStack, at /home/lhansen/m-i/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp:281
Segmentation fault

And here's the source code corresponding to that trace:

0f5557 func[4786]:
 0f5558: 01 7f                      | local[0] type=i32
 0f555a: 03 7e                      | local[1..3] type=i64
 0f555c: 01 7d                      | local[4] type=f32
 0f555e: 02 40                      | block
 0f5560: 23 04                      |   global.get 4
 0f5562: 45                         |   i32.eqz
 0f5563: 04 40                      |   if
 0f556b: 0b                         |   end
 0f556c: 23 04                      |   global.get 4
 0f556e: 41 01                      |   i32.const 1
 0f5570: 6b                         |   i32.sub
 0f5571: 24 04                      |   global.set 4
 0f5573: 0b                         | end
 0f5574: 02 7f                      | block i32
 0f5576: 03 40                      |   loop
 0f5578: 02 40                      |     block
 0f557a: 23 04                      |       global.get 4
 0f557c: 45                         |       i32.eqz
 0f557d: 04 40                      |       if
 0f5586: 0b                         |       end
 0f5587: 23 04                      |       global.get 4
 0f5589: 41 01                      |       i32.const 1
 0f558b: 6b                         |       i32.sub
 0f558c: 24 04                      |       global.set 4
 0f558e: 0b                         |     end
 0f558f: 02 40                      |     block
 0f5591: 41 de 00                   |       i32.const 94
 0f5594: 22 00                      |       local.tee 0
 0f5596: 04 40                      |       if
 0f5598: 41 91 2c                   |         i32.const 5649
 0f559b: 22 00                      |         local.tee 0
 0f559d: 21 00                      |         local.set 0
 0f559f: 41 95 b0 dc 18             |         i32.const 51845141
 0f55a4: 10 00                      |         call 0 &lt;log-i32>
 0f55a6: 05                         |       else
                                              ... deep and complicated with lots of unreachables
 0f56bb: 0b                         |       end
 0f56bc: 41 80 01                   |       i32.const 128
 0f56bf: 22 00                      |       local.tee 0
 0f56c1: 41 89 bc b8 c0 01          |       i32.const 403578377
 0f56c7: 22 00                      |       local.tee 0
 0f56c9: 22 00                      |       local.tee 0
 0f56cb: 20 00                      |       local.get 0
 0f56cd: 41 80 a6 cc 80 02          |       i32.const 538120960
 0f56d3: 22 00                      |       local.tee 0
 0f56d5: 22 00                      |       local.tee 0
 0f56d7: 20 00                      |       local.get 0
 0f56d9: 20 00                      |       local.get 0
 0f56db: 22 00                      |       local.tee 0
 0f56dd: 03 7f                      |       loop i32
 0f56df: 02 40                      |         block
 0f56e1: 23 04                      |           global.get 4
 0f56e3: 45                         |           i32.eqz
 0f56e4: 04 40                      |           if
 0f56e9: 0b                         |           end
 0f56ea: 23 04                      |           global.get 4
 0f56ec: 41 01                      |           i32.const 1
 0f56ee: 6b                         |           i32.sub
 0f56ef: 24 04                      |           global.set 4
 0f56f1: 0b                         |         end
 0f56f2: 41 20                      |         i32.const 32
 0f56f4: 0b                         |       end
 0f56f5: 41 0f                      |       i32.const 15
 0f56f7: 45                         |       i32.eqz
 0f56f8: 04 7f                      |       if i32
 0f5703: 05                         |       else
 0f5704: 41 80 80 80 80 02          |         i32.const 536870912
 0f570a: 0b                         |       end
 0f570b: 0d 02                      |       br_if 2

The bizarro stores are probably just the result of syncLocal(), which happens all the time because the wasm code keeps hammering on local 0.

Attached file 4786.tar (obsolete) —

The function isolated. Run with IONFLAGS=codegen and --no-ion --no-baseline --no-threads; wasm function #4 (the last one) is the code we're looking for and matches the code above.

Assignee: nobody → lhansen

It looks like the computation of target in PopChunkyBytes is not correct, and/or the fixed frame allocation is not correct. The computation only makes sense if fixedSize() is aligned to ChunkSize, but it is not - it is 112 here.

This /could/ be a fallout of the change to frame rounding that came in with the stack maps, but that is by no means certain.

Attached file 4786.tar

Further greatly reduced. When run, this segfaults the arm64 simulator. If the text is edited and all but the first local is removed, we get the

error initially reported.

I think the bug here goes back a ways; certainly the stackmaps patch was not to blame.

Attachment #9042971 - Attachment is obsolete: true
Attached file 4786.wat

This appears to be minimal (apart from the locals, as mentioned previously).

And with that in hand, the bug in (ARM64) stack management becomes obvious.

Attached file 4786.txt

Annotated disassembly of the minimized test case.

Attached file 4786.wat

Further simplifications are actually possible and now the bug can virtually be read out of the .wat.

Problem: When a stack chunk had to be popped as part of a control flow
instruction, the amount to pop was not always computed as a multiple
of ChunkSize. The reason is that the fixed amount of stack that
should not be popped isn't necessarily a multiple of ChunkSize, yet
this was assumed.

A small adjustment to the calculation fixes that.

Also added an assertion that would have caught this problem more

The TC sets up a situation where we require a chunk to be created and
then destroyed in the 'else' arm of the 'if', at the same time as the
fixed amount of stack is not a multiple of ChunkSize.

Attachment #9043209 - Attachment mime type: application/octet-stream → text/plain
Attachment #9043062 - Attachment mime type: application/octet-stream → text/plain
Duplicate of this bug: 1526568
Attachment #9043217 - Attachment description: Bug 1526579 - Rabaldr arm64 stack alignment fix. r?bbouvier → Bug 1526579 - Rabaldr arm64 stack alignment fix. r=bbouvier
Group: javascript-core-security → core-security-release
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Does this need to be considered for Beta backport or can it ride the trains?

Flags: needinfo?(lhansen)
Flags: in-testsuite+

A beta uplift wouldn't hurt, given that there seem to be betas of Firefox-on-Windows-on-ARM. I'll make sure the patch applies and request one.

Flags: needinfo?(lhansen)

Beta/Release Uplift Approval Request

Feature/Bug causing the regression


User impact if declined

Mysterious failures in wasm content on ARM64; possible security issue.

Is this code covered by automated tests?


Has the fix been verified in Nightly?


Needs manual test from QE?


If yes, steps to reproduce

List of other uplifts needed


Risk to taking this patch


Why is the change risky/not risky? (and alternatives if risky)

Only affects arm64 and we have no arm64 products in market; uplift is requested only to make the windows-on-arm64 betas less error prone, we have lots of time to get this vetted on multiple platforms.

String changes made/needed

Attachment #9043840 - Flags: approval-mozilla-beta?

I think normally we would expect a security rating before landing code from a sec-sensitive bug on m-c. In this situation we aren't shipping arm64 builds and they are only for testing. But, in general, is is best to follow the process for sec-rating, sec-approval, and so on.

The other thing I should mention is for s-s issues we don't normally land tests until the bug is unhidden.

Flags: needinfo?(lhansen)

How long should I wait for a security rating? The bug was opened a week ago. Normally a security rating appears within a few days but maybe there's a hidden process I've not been made aware of.

Anyway, again, this is arm64-specific and since we have no arm64 products in market it's likely sec-moderate like the other arm64 bugs.

Flags: needinfo?(lhansen)
Keywords: sec-moderate

I can see that. Dan do we have some way for people to ask for triage if they're trying to fast track an issue? Just need-info to you or is there a general email?

Flags: needinfo?(dveditz)
Comment on attachment 9043840 [details] [diff] [review]

Fix for assertion, ok for uplift for beta 10.
Attachment #9043840 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #21)

I think normally we would expect a security rating before landing code from a sec-sensitive bug on m-c.

In fact it's required:
If a rating is missing you can ask in #security for people to look at the bug. In this case we don't need sec-approval, whatever the rating, because this bug isn't in a shipping version anywhere yet.

(In reply to Lars T Hansen [:lth] from comment #23)

Anyway, again, this is arm64-specific and since we have no arm64 products in market it's likely sec-moderate like the other arm64 bugs.

We rate bugs in new features (or in this case, platforms) based on how bad they would be in a shipping release if we don't fix it by time it shipped. We can reflect the fact that it's not a current risk by setting the appropriate version status flags to "unaffected" or "disabled".

I don't know what the current plan is (might have slipped), but not long ago there was a plan to get the March Fx66 release into partner's hands. Shipping would be later, and hopefully autoupdate would work, but this isn't about the distant future anymore.

Flags: needinfo?(dveditz)
Keywords: sec-moderatesec-high
Blocks: 1526993
Flags: qe-verify-
Whiteboard: [jsbugmon:] → [jsbugmon:][post-critsmash-triage]
Whiteboard: [jsbugmon:][post-critsmash-triage] → [jsbugmon:][post-critsmash-triage][adv-main66+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.