Closed
Bug 1397363
Opened 7 years ago
Closed 7 years ago
stylo: panicked at 'attempt to subtract with overflow', rust/app_units/src/app_unit.rs:93 [@ mozilla::GeckoEffects::set_clip]
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | fixed |
People
(Reporter: truber, Assigned: manishearth)
References
(Blocks 2 open bugs)
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
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/third_party/rust/app_units/src/app_unit.rs:93 #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: app_units::app_unit::{{impl}}::sub, at third_party/rust/app_units/src/app_unit.rs:93 #11: style::gecko_bindings::structs::root::mozilla::GeckoEffects::set_clip, at 3a3cc34ed79fb81bad85b5e6a8f8022c49cc013e8e667e0b23c72960e78a8d6f57662706ca0b12e5ef6765ec62d8f6890a86e106755f965b51a0474bd3e8341b/toolkit/library/x86_64-unknown- linux-gnu/debug/build/style-20149327afa2228d/out/gecko_properties.rs:15724 #12: style::properties::StyleBuilder::set_clip, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style- 20149327afa2228d/out/properties.rs:125318 #13: style::properties::longhands::clip::cascade_property, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-linux-gnu/debug/b uild/style-20149327afa2228d/out/properties.rs:19268 #14: 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:135332 #15: style::properties::cascade, at ceaec01daa93fb4861b47d2b9a935bc5f30a679f7e03a0d74367f5f8e57a2ad5d7fd67460362c5546428749cdc2f1ab148289235b9b7b0424dfac84da5a37daf/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-20149327afa2228 d/out/properties.rs:134969 #16: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:522 #17: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:256 #18: style::style_resolver::{{impl}}::cascade_styles_with_default_parents::{{closure}}<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:273 #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>::cascade_styles_with_default_parents<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:271 #21: style::traversal::compute_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/traversal.rs:0 #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:281 #27: mozilla::ServoStyleSet::StyleDocument, at layout/style/ServoStyleSet.cpp:980
Flags: in-testsuite?
Reporter | ||
Comment 1•7 years ago
|
||
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8905258 [details] Bug 1397363 - stylo: Use Au's arithmetic operations for accumulation ; https://reviewboard.mozilla.org/r/177060/#review182072 r=me, with that changed to just `Au::new(self.0.animate(&other.0, procedure)?)`, which clamps. Unless there's a very good reason we can't do that, in which case please explain and ideally re-request review. Thanks for fixing! ::: servo/components/style/values/animated/mod.rs:172 (Diff revision 1) > fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> { > - Ok(Au(self.0.animate(&other.0, procedure)?)) > + // We don't use the Animate behavior for i32 because Au operates within a smaller > + // range than i32 and we rely on this fact for panic-free arithmetic operations > + // elsewhere. > + let (self_weight, other_weight) = procedure.weights(); > + Ok(self.scale_by(clamp_to_f32(self_weight)) + other.scale_by(clamp_to_f32(other_weight))) I'd prefer if we reused `Animate` and clamped the `Au` instead, is there any reason it's not done that way? Seems that it'd allow more range, and would have the animate logic less spread around.
Attachment #8905258 -
Flags: review?(emilio) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1b7868a4f3ef stylo: Add crashtest; r=emilio https://hg.mozilla.org/integration/autoland/rev/50d1554d4da4 stylo: Update crashtest expectations; r=orange
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1b7868a4f3ef https://hg.mozilla.org/mozilla-central/rev/50d1554d4da4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•