1653502 - Crash [@ ??] with SIGILL in Cranelift

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision f6127ce5c744 (debug build, run with --no-threads --fuzzing-safe --shared-memory=off --no-wasm-reftypes --no-wasm-simd --wasm-compiler=cranelift --disable-oom-functions test.js):

See attachment.

Backtrace:

==25445==ERROR: UndefinedBehaviorSanitizer: ILL on unknown address 0x1f51ac6c921c (pc 0x1f51ac6c921c bp 0xffffe00e50a0 sp 0xffffe00e50a0 T25445)
    #0 0x1f51ac6c921c  (<unknown module>)

UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: ILL (<unknown module>) 
==25445==ABORTING

This crash is very similar to bug 1650482 but that was fixed and this one still reproduces.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Another invalid instruction generated by Cranelift aarch64, one of us should look into it.

Comment 3

[Chris Fallin [:cfallin]]

Interesting -- this seems to be a similar shift-amount-masking bug, but with the built-in reg extend/shift mode that some arithmetic instructions support. Here's the relevant bit of vcode from the RUST_LOG=debug output:

  Inst 111:   asr x3, x3, #32
  Inst 112:   movz x4, #65519
  Inst 113:   movk x4, #65535, LSL #16
  Inst 114:   sub w3, w3, w4, LSL 51

(The shift amount of 51 on the 32-bit sub is the bug.)

Should be an easy fix -- will have a patch soon!

Comment 4

[Chris Fallin [:cfallin]]

This PR in Cranelift should fix the issue, once we merge it and then bump the version of Cranelift in-tree: https://github.com/bytecodealliance/wasmtime/pull/2042

Comment 5

[Christian Holler (:decoder)]

(In reply to Chris Fallin [:cfallin] from comment #4)

This PR in Cranelift should fix the issue, once we merge it and then bump the version of Cranelift in-tree: https://github.com/bytecodealliance/wasmtime/pull/2042

I can confirm that this will fix the remaining crashes I have been seeing.

Comment 6

[Chris Fallin [:cfallin]]

Attachment: Bug 1653502: Update vendored Cranelift to fix fuzzbug. r=bbouvier

This patch pulls in revision 1b3b2dbfd00492161032760992a8699d19b640ca of
Cranelift. This includes PR bytecodealliance/wasmtime#2042, which fixes
bug 1653502 by properly masking the shift amount in a shift incorporated
into an aarch64 arithmetic instruction.

This patch also includes various other miscellaneous Cranelift
improvements that have been merged since the last version-bump,
including some aarch64 codegen improvements.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5657e2d4be9
Update vendored Cranelift to fix fuzzbug. r=bbouvier

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d5657e2d4be9