Closed Bug 1279529 Opened 4 years ago Closed 4 years ago
Assertion failure: Has
SSE2(), at js/src/jit/x86-shared/Assembler-x86-shared .h:2265
The following testcase crashes on mozilla-central revision 3ccccf8e5036 (build with --enable-debug --enable-more-deterministic --32, run with --fuzzing-safe --no-threads --ion-eager --no-fpu): quit(); Actually no testcase is needed: $ ./js-dbg-32-dm-clang-darwin-3ccccf8e5036 --fuzzing-safe --no-threads --ion-eager --no-fpu -e "quit();" Assertion failure: HasSSE2(), at /Users/skywalker/trees/mozilla-central/js/src/jit/x86-shared/Assembler-x86-shared.h:2265 Segmentation fault: 11 Backtrace: 0 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x006ae9dc js::jit::AssemblerX86Shared::vucomisd(js::jit::FloatRegister, js::jit::FloatRegister) + 172 (Assembler-x86-shared.h:2265) 1 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x006ae74e js::jit::MacroAssembler::canonicalizeDouble(js::jit::FloatRegister) + 62 (MacroAssembler-x86-shared.h:109) 2 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x006136c7 js::jit::MacroAssembler::PushRegsInMask(js::jit::LiveSet<js::jit::RegisterSet>) + 471 3 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x0048bd0e js::jit::MacroAssembler::assumeUnreachable(char const*) + 62 (RegisterSets.h:324) 4 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x00683b89 js::jit::JitRuntime::generateProfilerExitFrameTailStub(JSContext*) + 905 (Trampoline-x86.cpp:1060) 5 js-dbg-32-dm-clang-darwin-3ccccf8e5036 0x00329d55 js::jit::JitRuntime::initialize(JSContext*, js::AutoLockForExclusiveAccess&) + 341 (Ion.cpp:216) /snip For detailed crash information, see attachment.
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/2c86039b1c68 user: Jeff Walden date: Mon May 23 22:49:56 2016 +0200 summary: Bug 1245627: Canonicalize before storing a floating point value in deterministic mode; r=nbp Waldo, is bug 1245627 a likely regressor?
Review commit: https://reviewboard.mozilla.org/r/59134/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/59134/
Attachment #8762457 - Flags: review?(nicolas.b.pierron)
https://reviewboard.mozilla.org/r/59134/#review56168 Why would this patch be sufficient? It feels to me that many other storeDouble calls can be used and that this would just delay the fuzz-bug to another test case?
(In reply to Nicolas B. Pierron [:nbp] from comment #4) > https://reviewboard.mozilla.org/r/59134/#review56168 > > Why would this patch be sufficient? > It feels to me that many other storeDouble calls can be used and that this > would just delay the fuzz-bug to another test case? Well, this is only happening in --enable-more-deterministic builds, which are only used by the fuzzers, so I wouldn't spend too much time thinking about it. The issue here is just that the cmpsd instruction is not available with the flags passed at runtime (--no-fpu). There is a whole category of similar fuzz bugs indeed just waiting to happen like this, but I think spending more time with the storeDouble vs storeUncanonicalizedDouble issue (which happens only in fuzzing) is a waste of time. Anyway, if you have any better fix in mind, I'd be glad to review it.
Based on the current context of SSE2 support, I think an alternative would be to remove this --no-fpu flag. AFAIK, it used to disable the FPU instruction on ARM, and also SSE2 support. So, we might as well remove this flag and prevent the fuzzers from using it.
I am more than happy to stop fuzzing --no-fpu if we feel that it is no longer necessary.
(In reply to Nicolas B. Pierron [:nbp] from comment #9) > Based on the current context of SSE2 support, I think an alternative would > be to remove this --no-fpu flag. AFAIK, it used to disable the FPU > instruction on ARM, and also SSE2 support. > > So, we might as well remove this flag and prevent the fuzzers from using it. ARM hardware without a FPU is still a potential concern, even if we no longer support x86 without SSE2. Do we still support these ARM devices?
(In reply to Jan de Mooij [:jandem] from comment #11) > ARM hardware without a FPU is still a potential concern, even if we no > longer support x86 without SSE2. Do we still support these ARM devices? For what it's worth, --no-fpu is a no-op on other platforms than x86/x64. The ARM simulator uses a different path for enabling soft/hard FP.
Stepping back a bit, it seems that SSE2 is required on windows 32 bits and mac os 32 bits, but it's unclear for linux 32 bits (64 bits intel processors all have SSE2, fwiw).
Do we still want support non-SSE2 linux platforms?
I'm not aware of an official decision on non-SSE2 x86 linux, but we're certainly moving in that direction. In practice our official builds require SSE2, but it's still possible for downstream packagers to undo that. Unless there's a reversal I expect it will become required soon, so dropping this from fuzzing sounds reasonable to me. A number of linux distros claim to support non-SSE2, but it's not clear they have testing to ensure this for everything they build.
Comment on attachment 8762457 [details] Bug 1279529; Remove the --no-fpu option in the shell; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59134/diff/1-2/
All green on try, fwiw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93fdf5e2c851
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Comment on attachment 8762457 [details] Bug 1279529; Remove the --no-fpu option in the shell; https://reviewboard.mozilla.org/r/59134/#review57196 Nice that it will make jit-tests faster in automation.
Attachment #8762457 - Flags: review?(jdemooij) → review+
> Nice that it will make jit-tests faster in automation. \o/
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4348f782dae4 Remove the --no-fpu option from the shell; r=jandem
You need to log in before you can comment on or make changes to this bug.