dluca: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&tier=1%2C2%2C3&searchStr=android-hw&revision=752c683e631d73518b21da4b0924ac80ce1f6d5f&selectedJob=220576455 is not this bug.

It has

exception output: /data/local/tests/jit-tests/jit-tests/tests/auto-regress/bug710192.js:10:1 Error: Assertion failed: got (void 0), expected "1234"
TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/basic/bug1355573.js | Segmentation fault (code 139, args "") [5.8 s]

which is not this bug.

ebalazs_: See comment 5.

bc: Thank you, We'll file a bug for the new failure.

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=3e26b297e547ecc40e2dd32dbb127c73ac594643

Joel can we land this?

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72c3b5930c7d
Disable jit-test/tests/wasm/spec/{address.wast,call_indirect.wast}.js on arm-32, r=jmaher

Thanks, I did see that. I've been monitoring this bug for a couple of days and I'm fairly worried, but I have very few spare cycles at the moment :( Will try to dig into it in the morning.

Sorry, I didn't get to it right away. Tree closure and a meeting DOSed me.

https://hg.mozilla.org/mozilla-central/rev/72c3b5930c7d

Failure is caused by the test labeled "// address.wast:535" even when run in isolation. That is this test case:

(module
  (type (;0;) (func))
  (type (;1;) (func (param i32) (result f32)))
  (import "$3" "32_good1" (func (;0;) (type 1)))
  (func (;1;) (type 0)
    block  ;; label = @1
      i32.const 65525
      call 0
      i32.reinterpret_f32
      f32.const 0x0p+0 (;=0;)
      i32.reinterpret_f32
      i32.eq
      i32.eqz
      br_if 0 (;@1;)
      return
    end
    unreachable)
  (export "run" (func 1)))

and the function "32_good1" comes from here:

(module
  (type (;0;) (func (param i32) (result f32)))
  (type (;1;) (func (param i32)))
  (func (;0;) (type 0) (param i32) (result f32)
    local.get 0
    f32.load)
  (func (;1;) (type 0) (param i32) (result f32)
    local.get 0
    f32.load align=1)
  (func (;2;) (type 0) (param i32) (result f32)
    local.get 0
    f32.load offset=1 align=1)
  (func (;3;) (type 0) (param i32) (result f32)
    local.get 0
    f32.load offset=2 align=2)
  (func (;4;) (type 0) (param i32) (result f32)
    local.get 0
    f32.load offset=8)
  (func (;5;) (type 1) (param i32)
    local.get 0
    f32.load offset=4294967295
    drop)
  (memory (;0;) 1)
  (export "32_good1" (func 0))
  (export "32_good2" (func 1))
  (export "32_good3" (func 2))
  (export "32_good4" (func 3))
  (export "32_good5" (func 4))
  (export "32_bad" (func 5))
  (data (;0;) (i32.const 0) "\00\00\00\00\00\00\a0\7f\01\00\d0\7f"))

This looks like an f32 load from an unaligned address without any alignment specifier; a known dark corner of our ARM implementation...

OOB is a strange error to see here since the access is clearly in-bounds, so more likely the load causes an exception that is interpreted by us as OOB. But we'll see.

Basically, HandleTrap() treats SIGSEGV and SIGBUS the same, so a SIGBUS alignment trap will be treated as a SIGSEGV out-of-bounds trap, which is why we see OOB here.

It would appear from this test file that this is only an issue for floating point accesses, since the integer tests earlier in the file did not fail; this again suggests the Android kernel does some kind of filtering of alignment errors, since the ARMv8 only has a single control word bit for alignment checking; if set, everything is checked; if unset, only some instructions (like atomics) are checked. The filtering would amount to fixing up integer accesses but letting floating point accesses flow through to a SIGBUS. Joy.

The chipset on the Pixel 2 is a Snapdragon 835 which has a Kryo 280 cpu which is a modified ARMv8 Cortex-A73, supposedly.

I take back some of the analysis in comment 16, since this is ARMv7 code and it has a more complicated trap model, and FP accesses can trap independently of int accesses. "It's complicated." ARMv8 manual E2.4.

In principle, something like this in HandleTrap:

  if (isSigbus) {
    // Faulting memory access.
#ifdef __arm__
    uint32_t instr = *(uint32_t*)pc;
    if ((instr & 0x0F300E00) == 0x0D100A00) { // VLDR
      bool isDouble = (instr & 0x00000100) != 0;
      bool add = (instr & 0x00800000) != 0;
      bool dBit = (instr >> 22) & 1;
      bool offs = (instr & 0xFF) << 2;
      bool rn = (instr >> 16) & 0xF;
      MOZ_ASSERT(rn != 15);     // Or we must align it
      uint8_t* p = ReadGPR(context, rn) + (add ? offs : -offs);
      // Do we need to worry about whether p is in bounds?  As in, is SEGV
      // handled before BUS or not?
      if (isDouble) {
        bool vd = ((instr >> 12) & 0xF) | (dBit << 4);
        double val;
        memcpy(&val, p, sizeof(val));
        WriteFPR64(context, vd, val);
      } else {
        bool vd = ((instr >> 11) & (0xF << 1)) | dBit;
        float val;
        memcpy(&val, p, sizeof(val));
        WriteFPR32(context, vd, val);
      }
    }
#endif // __arm__
  }

However, the WriteFPR() functions may not be easily implementable, in
https://android.googlesource.com/platform/bionic/+/c124baa/libc/arch-arm/include/machine/ucontext.h we read that the FP state is not really accessible and is anyway kernel-configuration dependent. So it's uncertain whether we can do a good job this way.

An alternative is to set up an exception return to code that performs the emulation back in userland, where the registers are right. That might not be so bad.

There are no failures in the last 7 days, the ones from the above comment are from before the disable patch got landed.

This (and several others) will be closed the minute we're sure the patch for bug 1283121 is not backed out :)