1291650 - Crash in lib::capi::mp4parse_new

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

This bug was filed from the Socorro interface and is 
report bp-28af2d67-07ad-4d75-bba5-921c02160801.
=============================================================

Crashed at a SSE2 instruction

b622a6b1:	f2 0f 10 45 b0       	movsd  -0x50(%ebp),%xmm0
^^^^^^^^^	^^^^^^^^^^^^^^^^^^^^^	^^^^^^^^^^^^^^^^^^^^^^^^^

The CPU is a AuthenticAMD family 6 model 8 stepping 1... which is Athlon XP "Thoroughbred" released in 2002 that doesn't support SSE2

Maybe we should display a nice warning then quit early instead of crashing?

Anyway.. feel free to close this as INVALID.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Ralph - how do we stop supporting non-SSE2 CPUs on Linux?

Comment 2

[Ralph Giles (:rillian)]

So there are a couple of options here. I don't think we care about non-SSE2 linux32 cpus. Debian does, but I haven't gotten an answer to how recently they've reconsidered that policy. Gladium, what do you want to do to support Debian here, and what can I do to help?

We can go back to doing our builds with rustc targeting i586-pc-unkown-linux, which would resolve this particular bug, and avoid it for rust code landing in the future. This slows everyone down for a sake of some legacy systems. On the other hand, most linux folks are on 64-bit where the issue doesn't apply.

We could leave it up to distros to handle this in their own build. This likely means we'll get occasional bugs like this we'll have to close as, "Sorry, you need a different build." When we dropped support for these CPUs on Windows, we added a check to the installer, which is a lot more user-friendly (bug 1271759) but I don't know of a way to do that for Linux, where we don't build our own packages. We could write some kind of startup check, but it's likely to be brittle since we need to run a lot of compiled code before we can display a message.

I was inclined to leave it (i.e. close this as INVALID) because using the default rust target is less hassle, and I expect our C++ code to eventually start requiring the same thing, but I'm happy to find a better balance.

Comment 3

[Mike Hommey [:glandium]]

My position is that we shouldn't explicitly make the build force sse2 by default, and leave it to the compiler to pick its defaults, possibly overridden by flags in mozconfig. If Debian wants rust code not to use SSE2, they can change their rust compiler to not default to compile with SSE2 on x86.

Now, the problem here is not really related to that. The problem here is that Firefox runs just fine on machines without SSE2, up until the point where it hits an SSE2 instruction and then crashes. That's not very user friendly. And there, I'd say that until Mozilla makes SSE2 explicitly required (bug 1274196) on the linux builds it ships, rust code should not build with SSE2 enabled.

But this bug also highlights that merely switching SSE2 on on Mozilla shipped builds is not going making a nice user experience for people without SSE2. I don't remember how this was handled on Windows, but this discussion should definitely happen in bug 1274196.

Comment 4

[Ralph Giles (:rillian)]

On Windows the installer checks the cpu for us, so there's a user-facing message before firefox runs.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Ralph Giles (:rillian) needinfo me from comment #4)
> On Windows the installer checks the cpu for us, so there's a user-facing
> message before firefox runs.

What about upgrades?

Comment 6

[Ralph Giles (:rillian)]

We also don't offer upgrades past 48 to windows users without SSE2. Code was adding to 48 to report system capabilities in the update request in bug 1271761, but the code is unfortunately windows-specific. Adding the equivalent checks for linux is bug 1277359 which no one has worked on.

Bug 1271762 is the overall tracking bug.

Comment 7

[Ralph Giles (:rillian)]

I believe this issue is addressed now. Thanks for the report. Last crash report was from build id 20160905004005.