1947697 - Assertion failure: *def->output() == alloc, at jit/RegisterAllocator.cpp:270

Status: RESOLVED FIXED

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

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Debug stack

new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
  (global $i (mut i32) (i32.const 0))
  (memory 7)
  (func $0 (result i32)
    unreachable
  )
  (func
    (local $0 i32)
    (local $1 i64)
    block $block
      loop $label
        (local.get 1)
        i64.extend32_s
        (local.set 1)
        (i32.const 1)
        if
          (i32.const 1)
          i32.load
          if (result i32)
            unreachable
          else
            (local.get 0)
            if (result i32)
              (br $block)
            else
              (call 0)
            end
            if (result i64)
              (local.get 1)
              (i32.const 0)
              (i32.const 0)
              (local.get 0)
              select
              if (result i64)
                (local.get 1)
              else
                unreachable
              end
              i64.shr_s
            else
              loop $label1 (result i32)
                (global.get $i)
                (br_if $label1)
                (i32.const 0)
                (local.get 0)
                (local.get 0)
                select
              end
              if (result i64)
                (br $block)
              else
                (i64.const 0)
              end
            end
            (local.get 1)
            i64.gt_u
          end
          (global.set $i)
        end
        loop $1 (result i32)
          (br $label)
        end
        if
        end
      end
    end
  )
`)))

Debug stack:

(gdb) bt
#0  0x58da6ad5 in MOZ_CrashSequence (aAddress=0x0, aLine=270)
    at /home/i32g7900a/shell-cache/js-dbg-32-armsim32-linux-x86_64-11a45cb6835c/objdir-js/dist/include/mozilla/Assertions.h:263
#1  js::jit::AllocationIntegrityState::checkIntegrity (this=0xffff9044, block=0xf6965ca0, ins=<optimized out>, vreg=14, alloc=...)
    at /home/i32g7900a/trees/mozilla-central/js/src/jit/RegisterAllocator.cpp:270
#2  0x58da45dc in js::jit::AllocationIntegrityState::check (this=0xffff9044) at /home/i32g7900a/trees/mozilla-central/js/src/jit/RegisterAllocator.cpp:213
#3  0x59140995 in js::jit::GenerateLIR (mir=0xffffa008) at /home/i32g7900a/trees/mozilla-central/js/src/jit/Ion.cpp:1555
#4  0x594bc573 in js::wasm::IonCompileFunctions (codeMeta=..., compilerEnv=..., lifo=..., inputs=..., code=0xf6936e80, error=0xffffbc84)
    at /home/i32g7900a/trees/mozilla-central/js/src/wasm/WasmIonCompile.cpp:10586
#5  0x5949ac8b in ExecuteCompileTask (task=<optimized out>, error=0x10e) at /home/i32g7900a/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:563
#6  0x5949b972 in js::wasm::ModuleGenerator::locallyCompileCurrentTask (this=0xffffadc8)
    at /home/i32g7900a/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:656
/snip

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/12ba7ff7a874
user:        André Bargull
date:        Tue Feb 04 16:41:01 2025 +0000
summary:     Bug 1944797 - Part 1: Add LIRGeneratorShared::useLowWord to allocate register for int64 low-word. r=jandem

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig 'CC="clang -msse2 -mfpmath=sse"' 'CXX="clang++ -msse2 -mfpmath=sse"' sh ../configure --host=x86_64-pc-linux-gnu --target=i686-pc-linux --enable-simulator=arm --enable-debug --enable-debug-symbols --with-ccache --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 11a45cb6835c.

Setting s-s just in case. Andre, is bug 1944797 a likely regressor?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1944797

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1947697: Don't use rhs at start. r=jandem!

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1947697: Add test case. r=jandem!

Comment 4

[André Bargull [:anba]]

I don't think this is exploitable, because at worst we shift with the value in an Int64 high-word, instead of using the low-word.

Relevant block from the original test case:

block8:
movegroup [stack:12 -> r0, g]
movegroup [stack:12 -> r1, g], [stack:16 -> r0, g]
{v20<g>:r1, v21<g>:r0} <- shifti64 (r0), (r1), (r0)
goto s=(block17)

The shift amount is in stack:12, which is first moved to r0 , but then overridden when moving the high-word from the left-hand side operator from stack:16 to r0.

Comment 5

[Daniel Veditz [:dveditz]]

You don't sound entirely convinced. jandem, since you have to review this anyway, what's your opinion about whether this is exploitable or not? The assertion doesn't seem to be anywhere near the code fix here.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

See previously-marked sec-high bug 1707774 (CVE-2021-29981) and also sec-high bug 1766806 (CVE-2022-31740), for the same exact assertion failure (not necessarily implying anything else).

Comment 7

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #4)

I don't think this is exploitable, because at worst we shift with the value in an Int64 high-word, instead of using the low-word.

We also don't do range analysis for non-Int32 shift instructions so if we just get the wrong result for the shift I agree it's a correctness issue but not a security issue.

Comment 8

[André Bargull [:anba]]

(In reply to Jan de Mooij [:jandem] from comment #7)

We also don't do range analysis for non-Int32 shift instructions so if we just get the wrong result for the shift I agree it's a correctness issue but not a security issue.

I guess that also means I don't have to wait for further sec-rating and can just push the change?

Comment 9

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #8)

I guess that also means I don't have to wait for further sec-rating and can just push the change?

Yeah we can just land the patch now. It's a Nightly-only regression so it doesn't need sec-approval even if it was sec-high.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce8ec4d7235e
Don't use rhs at start. r=jandem
https://hg.mozilla.org/integration/autoland/rev/a8dc1c9cc09b
Add test case. r=jandem

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/ce8ec4d7235e
https://hg.mozilla.org/mozilla-central/rev/a8dc1c9cc09b