The attached testcase causes a panic in m-c rev c22502562670 with stylo enabled by pref. thread '<unnamed>' panicked at 'attempt to add with overflow', /home/worker/workspace/build/src/third_party/rust/app_units/src/app_unit.rs:84 stack backtrace: 0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace 1: std::sys_common::backtrace::_print 2: std::panicking::default_hook::{{closure}} 3: std::panicking::default_hook 4: std::panicking::rust_panic_with_hook 5: std::panicking::begin_panic 6: std::panicking::begin_panic_fmt 7: rust_begin_unwind 8: core::panicking::panic_fmt 9: core::panicking::panic 10: <app_units::app_unit::Au as core::ops::Add>::add 11: <app_units::app_unit::Au as core::ops::AddAssign>::add_assign 12: style::values::computed::length::<impl style::values::computed::ToComputedValue for style::values::specified::calc::CalcLengthOrPercentage>::to_computed_value 13: style::values::computed::length::<impl style::values::computed::ToComputedValue for style::values::specified::length::Length>::to_computed_value 14: style::properties::longhands::_webkit_text_stroke_width::cascade_property 15: style::properties::apply_declarations 16: style::properties::cascade 17: <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::cascade_style 18: <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::resolve_primary_style 19: <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::resolve_style 20: style::traversal::resolve_style 21: Servo_ResolveStyleLazily

This will be a crash in debug builds, but will work fine in release afaict... I don't think this overflow is problematic, but I'm not sure we should change all our additions to the special rust method to add without overflow...

If web content can trigger a panic in debug builds we need to fix it, otherwise the fuzzers won't be able to do their job. In general, my guess is that the set of integer overflows that can be reliably triggered is pretty small, and we should audit/fix them when we find them. IIUC, this crash is triggering the additions in [1]. What does the spec say to do there? [1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/servo/components/style/values/computed/length.rs#231

Gecko seems to do saturating add in calc ops. We should do the same.

Comment on attachment 8890078 [details] Bug 1383492: stylo: Bump app units version; https://reviewboard.mozilla.org/r/161148/#review166486 ::: servo/components/style/values/computed/length.rs:236 (Diff revision 1) > impl ToComputedValue for specified::CalcLengthOrPercentage { > type ComputedValue = CalcLengthOrPercentage; > > fn to_computed_value(&self, context: &Context) -> CalcLengthOrPercentage { > let mut length = Au(0); > + println!("l {:?} {:?} {:?}", length, ::app_units::MAX_AU, ::app_units::MIN_AU); Remove all the printfs please :)

Comment on attachment 8890080 [details] Bug 1383492: stylo: Change nscoord_MAX to 1<<30 - 1 ; https://reviewboard.mozilla.org/r/161152/#review166512 This needs a try run, and I'd rather get the signoff from dbaron too (though this was discussed with bz and he was ok with this, so it may be ok).

Try passes except for one assertion going away (marked as expected)

Looks like this causes some new assertions in the crashtests. They seem to just be uncovering existing bugs since the saturated value is now different.

Comment on attachment 8890080 [details] Bug 1383492: stylo: Change nscoord_MAX to 1<<30 - 1 ; https://reviewboard.mozilla.org/r/161152/#review166594

Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b4dfcf4f795e stylo: Bump app units version; r=emilio https://hg.mozilla.org/integration/autoland/rev/11cadad891a4 stylo: Change nscoord_MAX to 1<<30 - 1 ; r=emilio,dbaron

Backing these and the two followups out for continued Windows build bustage and crashtest assertions like https://treeherder.mozilla.org/logviewer.html#?job_id=118275761&repo=autoland https://treeherder.mozilla.org/logviewer.html#?job_id=118286406&repo=autoland https://hg.mozilla.org/integration/autoland/rev/7f5d8fe1c87062853ed44b9ede27ad24c03e5812

Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f89500a307f6 stylo: Bump app units version; r=emilio https://hg.mozilla.org/integration/autoland/rev/7fbc3c5786d1 stylo: Change nscoord_MAX to 1<<30 - 1 ; r=emilio,dbaron

https://hg.mozilla.org/mozilla-central/rev/f89500a307f6 https://hg.mozilla.org/mozilla-central/rev/7fbc3c5786d1