1397439 - stylo: panicked at 'attempt to subtract with overflow', at gecko_properties.rs:9532 [@ mozilla::GeckoFont::calculate_script_level_size]

Status: RESOLVED FIXED

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

Description

[Jesse Schwartzentruber (:truber)]

The attached testcase causes a panic in m-c rev 20170906-c959327c6b75 with stylo enabled.

thread '<unnamed>' panicked at 'attempt to subtract with overflow', /builds/worker/workspace/build/src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-20149327afa2228d/out/gecko_properties.rs:9532
#0: mozalloc_abort, at memory/mozalloc/mozalloc_abort.cpp:33
#1: abort, at memory/mozalloc/mozalloc_abort.cpp:80
#2: panic_abort::__rust_start_panic, at src/libpanic_abort/lib.rs:61
#3: std::panicking::rust_panic, at src/libstd/panicking.rs:580
#4: std::panicking::rust_panic_with_hook, at src/libstd/panicking.rs:565
#5: std::panicking::begin_panic<collections::string::String>, at src/libstd/panicking.rs:511
#6: std::panicking::begin_panic_fmt, at src/libstd/panicking.rs:495
#7: std::panicking::rust_begin_panic, at src/libstd/panicking.rs:471
#8: core::panicking::panic_fmt, at src/libcore/panicking.rs:69
#9: core::panicking::panic, at src/libcore/panicking.rs:49
#10: style::gecko_bindings::structs::root::mozilla::GeckoFont::calculate_script_level_size, at 3a3cc34ed79fb81bad85b5e6a8f8022c49cc013e8e667e0b23c72960e78a8d6f57662706ca0b12e5ef6765ec62d8f6890a86e106755f965b51a0474bd3e8341b/toolkit/library
/x86_64-unknown-linux-gnu/debug/build/style-20149327afa2228d/out/gecko_properties.rs:9532
#11: style::gecko_bindings::structs::root::mozilla::GeckoFont::inherit_font_size_from, at 3a3cc34ed79fb81bad85b5e6a8f8022c49cc013e8e667e0b23c72960e78a8d6f57662706ca0b12e5ef6765ec62d8f6890a86e106755f965b51a0474bd3e8341b/toolkit/library/x86_
64-unknown-linux-gnu/debug/build/style-20149327afa2228d/out/gecko_properties.rs:9582
#12: style::properties::longhands::font_size::cascade_inherit_font_size, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-lin
ux-gnu/debug/build/style-20149327afa2228d/out/properties.rs:21228
#13: style::properties::apply_declarations<closure,core::iter::FlatMap<style::rule_tree::SelfAndAncestors, core::iter::FilterMap<core::iter::Rev<core::slice::Iter<(style::properties::PropertyDeclaration, style::properties::declaration_bloc
k::Importance)>>, closure>, closure>>, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-20149327a
fa2228d/out/properties.rs:135265
#14: style::properties::cascade, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-20149327afa2228
d/out/properties.rs:134969
#15: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:522
#16: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_primary_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:159
#17: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:179
#18: style::style_resolver::{{impl}}::resolve_style_with_default_parents::{{closure}}<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:218
#19: style::style_resolver::with_default_parent_styles<style::gecko::wrapper::GeckoElement,closure,style::data::ElementStyles>, at servo/components/style/style_resolver.rs:76
#20: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::resolve_style_with_default_parents<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:217
#21: style::traversal::compute_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/traversal.rs:678
#22: style::traversal::recalc_style_at<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly,closure>, at servo/components/style/traversal.rs:473
#23: style::gecko::traversal::{{impl}}::process_preorder<closure>, at servo/components/style/gecko/traversal.rs:37
#24: style::driver::traverse_dom<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly>, at servo/components/style/driver.rs:71
#25: geckoservo::glue::traverse_subtree, at servo/ports/geckolib/glue.rs:250
#26: geckoservo::glue::Servo_TraverseSubtree, at servo/ports/geckolib/glue.rs:288

Comment 1

[Jesse Schwartzentruber (:truber)]

Attachment: testcase.html

Comment 2

[Manish Goregaokar [:manishearth]]

The 

    let delta = self.gecko.mScriptLevel - parent.gecko.mScriptLevel;

line should use saturating_sub or something here.

Comment 3

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1397439 - stylo: Use saturating subtraction for script level;

Review commit: https://reviewboard.mozilla.org/r/177072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177072/

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905272 [details]
Bug 1397439 - stylo: Use saturating subtraction for script level;

https://reviewboard.mozilla.org/r/177072/#review182066

::: layout/style/crashtests/1397439-1.html:12
(Diff revision 1)
> +<body>
> + <math>
> +  <mstyle scriptlevel=101>
> +  <mstyle scriptlevel=-204>
> + </math>
> + <div

nit: Just remove the straneous divs and such around.

No need for the script and similar either.

Comment 5

[Manish Goregaokar [:manishearth]]

Comment on attachment 8905272 [details]
Bug 1397439 - stylo: Use saturating subtraction for script level;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177072/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29eb7102e965
stylo: Add crashtest; r=emilio

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/29eb7102e965