1147608 - Assertion failure: HasSSE2(), at jit/shared/Assembler-x86-shared.h or Assertion failure: callee_saved_value_d == value, at jit/arm/Simulator-arm.cpp involving --unboxed-objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/sunspider/check-string-xcloud.js
(function(s) {
    s.parseJSON = function() {
        eval('(' + this + ')')
    }
})(String.prototype);
'[\
    { "x": 0, "y": 4294967296},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
]'.parseJSON()

asserts js 32-bit debug shell on m-c changeset 5330c6f461a4 with --fuzzing-safe --no-threads --ion-eager --unboxed-objects --no-fpu at Assertion failure: HasSSE2(), at jit/shared/Assembler-x86-shared.h.

Configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build --32" -r 5330c6f461a4

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/94f1fc3d9ec8
user:        Brian Hackett
date:        Thu Mar 12 10:58:19 2015 -0600
summary:     Bug 1135897 - Use unboxed objects for JSON objects and constant literals embedded in scripts, r=jandem.

Brian, is bug 1135897 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x74b23, 0x0077ba26 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`js::jit::AssemblerX86Shared::vxorpd(this=<unavailable>, src1=(reg_ = xmm7, type_ = Double, isInvalid_ = false), src0=(reg_ = xmm7, type_ = Double, isInvalid_ = false), dest=(reg_ = xmm7, type_ = Double, isInvalid_ = false)) + 198 at Assembler-x86-shared.h:2665, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0077ba26 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`js::jit::AssemblerX86Shared::vxorpd(this=<unavailable>, src1=(reg_ = xmm7, type_ = Double, isInvalid_ = false), src0=(reg_ = xmm7, type_ = Double, isInvalid_ = false), dest=(reg_ = xmm7, type_ = Double, isInvalid_ = false)) + 198 at Assembler-x86-shared.h:2665
    frame #1: 0x00674933 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`void js::jit::MacroAssembler::storeUnboxedProperty<js::jit::Address>(js::jit::Address, JSValueType, js::jit::ConstantOrRegister, js::jit::Label*) [inlined] js::jit::MacroAssemblerX86Shared::zeroDouble(js::jit::FloatRegister) + 1603 at MacroAssembler-x86-shared.h:902
    frame #2: 0x00674901 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`void js::jit::MacroAssembler::storeUnboxedProperty<js::jit::Address>(js::jit::Address, JSValueType, js::jit::ConstantOrRegister, js::jit::Label*) [inlined] js::jit::MacroAssemblerX86Shared::convertInt32ToDouble(js::jit::Register, js::jit::FloatRegister) at MacroAssembler-x86-shared.h:691
    frame #3: 0x00674901 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`void js::jit::MacroAssembler::storeUnboxedProperty<js::jit::Address>(js::jit::Address, JSValueType, js::jit::ConstantOrRegister, js::jit::Label*) [inlined] js::jit::MacroAssemblerX86::int32ValueToDouble(js::jit::ValueOperand const&, js::jit::FloatRegister) at MacroAssembler-x86.h:1004
    frame #4: 0x00674901 js-dbg-32-dm-nsprBuild-darwin-5330c6f461a4`void js::jit::MacroAssembler::storeUnboxedProperty<js::jit::Address>(this=0xbfffbe80, address=Address at 0xbfffbcf4, type=JSVAL_TYPE_DOUBLE, value=ConstantOrRegister at 0xbfffbd00, failure=<unavailable>) + 1553 at MacroAssembler.cpp:946
(lldb)

Comment 2

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

// Randomly chosen test: js/src/jit-test/tests/sunspider/check-string-xcloud.js
(function(s) {
    s.parseJSON = function() {
        if (/]/.test(this.replace(/x/, '').replace(/x/, ''))) {
            eval('(' + this + ')')
        }
    }
})(String.prototype);
'[\
    { "x": 0, "y": 4294967296},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
    { "x": 0, "y": 0},\
]'.parseJSON()

This variant asserts js debug 32-bit ARM-simulator builds with --fuzzing-safe --no-threads --no-baseline --no-ion --unboxed-objects at Assertion failure: callee_saved_value_d == value, at jit/arm/Simulator-arm.cpp

Configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-arm-simulator --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build --32 --enable-arm-simulator" -r 5330c6f461a4

Comment 3

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

Attachment: stack of other assert

(lldb) bt 5
* thread #1: tid = 0x31f8d, 0x006cb100 js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::jit::Simulator::callInternal(this=<unavailable>, entry=0x01a32310) + 1872 at Simulator-arm.cpp:4350, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x006cb100 js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::jit::Simulator::callInternal(this=<unavailable>, entry=0x01a32310) + 1872 at Simulator-arm.cpp:4350
    frame #1: 0x006cb1b4 js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::jit::Simulator::call(this=0x01d71000, entry=0x006c2d53, argument_count=<unavailable>) + 164 at Simulator-arm.cpp:4404
    frame #2: 0x0034df45 js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::UnboxedPlainObject::createWithProperties(cx=0x01d72040, group=js::HandleObjectGroup at 0xbfffca44, newKind=TenuredObject, properties=<unavailable>) + 149 at UnboxedObject.cpp:634
    frame #3: 0x00226a4f js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::ObjectGroup::newPlainObject(cx=<unavailable>, properties=0xbfffcc60, nproperties=<unavailable>, newKind=TenuredObject) + 655 at ObjectGroup.cpp:1032
    frame #4: 0x00188fbb js-dbg-32-dm-nsprBuild-armSim-darwin-5330c6f461a4`js::frontend::ParseNode::getConstantValue(this=<unavailable>, cx=0x01d72040, allowObjects=<unavailable>, vp=JS::MutableHandleValue at 0xbfffcd6c, newKind=<unavailable>) + 1835 at BytecodeEmitter.cpp:4199
(lldb)

Comment 4

[Brian Hackett [Laid off!]]

Attachment: patch

This patch fixes both problems.  While we don't explicitly use any FPU registers while constructing unboxed objects in jitcode, if the object has any double properties then the macro assembler will use the scratch FPU register internally.  This means we shouldn't use these stubs if FPU support isn't available, and also runs into an issue on ARM that the scratch double reg is callee saved under the ABI (which seems bizarre).

Comment 5

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8588649 [details] [diff] [review]
patch

Review of attachment 8588649 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/UnboxedObject.cpp
@@ +109,5 @@
>      for (GeneralRegisterForwardIterator iter(savedNonVolatileRegisters); iter.more(); ++iter)
>          masm.Push(*iter);
> +#ifdef JS_CODEGEN_ARM
> +    // The scratch double register, which we might use, is callee saved on ARM.
> +    masm.push(ScratchDoubleReg);

Maybe

if (ScratchDoubleReg.volatile_())
    masm.Push(ScratchDoubleReg);

Or add an #else MOZ_ASSERT(!ScratchDoubleReg.volatile_()).

Comment 7

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f345912dc20c

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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