1379895 - stylo: Crash in mozalloc_abort | abort | core::result::unwrap_failed<T> | style::properties::longhands::system_font::{{impl}}::to_computed_value

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Chris Peterson [:cpeterson]]

This bug was filed from the Socorro interface and is 
report bp-6bf9bef3-8cb6-448f-85ca-bf4940170709.
=============================================================

1 crash in Nightly 56

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

unwrap_failed from system_font::to_computed_value? That's fun... I don't see any Result type inside that function directly. It is annoying that there is no crash message. My first guess would be that system.weight ends up being a number which isn't a multiple of 100, which triggers the unwrap-ish in font_weight::computed_value::T::from_gecko_weight.

We may want to remove that .expect or round it to a multiple of 100 for now.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> unwrap_failed from system_font::to_computed_value? That's fun... I don't see
> any Result type inside that function directly. It is annoying that there is
> no crash message. My first guess would be that system.weight ends up being a
> number which isn't a multiple of 100, which triggers the unwrap-ish in
> font_weight::computed_value::T::from_gecko_weight.
> 
> We may want to remove that .expect or round it to a multiple of 100 for now.

That's an expect() though, right? That shouldn't have this crash signature...

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

There were 9 crashes in the past month on a single install, and only on 20170709100223.

Given that nothing actionable here... let's close it?

Comment 4

[Bobby Holley (:bholley)]

This actually appears to be the highest-volume crasher (by far) in stylo code over the last few days:

https://crash-stats.mozilla.com/search/?telemetry_environment=~layout.css.servo.enabled&signature=~Gecko_&signature=~Servo_&signature=~geckoservo&signature=~style%3A%3A&build_id=%3E%3D20170721000000&product=Firefox&version=56.0a1&date=%3E%3D2017-07-17T18%3A38%3A00.000Z&date=%3C2017-07-24T18%3A38%3A00.000Z&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

Comment 5

[Manish Goregaokar [:manishearth]]

Yeah, someone reported this on Reddit too.

I'm going to look into this today.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

The MOZ_CRASH Reason field shows "from_gecko_weight: called with invalid weight: ()", which matches my guess in comment 1. So I think the issue here is that the system font has a weird font weight. Also, all of this crash happens on Linux, which could be a good side evidence because Linux users tend to have unusual system settings.

I guess we can fix this via not requiring font weight to be multiple of 100 at some places. We would need to remove that restriction eventually, so I guess it isn't a big deal.

Comment 7

[Manish Goregaokar [:manishearth]]

Looks like on linux Pango has font-weight values ultraheavy (1000),  semilight (350), and book (380). Any of these can trigger the assert.

I'm going to allow weird positive font weight values to exist but not be specifiable.

Comment 8

[Manish Goregaokar [:manishearth]]

https://github.com/servo/servo/pull/17846

Comment 9

[Manish Goregaokar [:manishearth]]

https://hg.mozilla.org/mozilla-central/rev/131e19a573e9