remove duplicate frames
Categories
(Socorro :: Signature, defect, P2)
Tracking
(Not tracked)
People
(Reporter: willkg, Unassigned)
References
Details
In bug #1541474, we added a prefix so that signature generation would continue beyond real_drop_in_place frames. After that change, bp-d9291a0a-6061-4c9f-aac7-885d10190403 ends up with this signature:
app@fab7d1e176d1:/app$ socorro-cmd signature d9291a0a-6061-4c9f-aac7-885d10190403
Crash id: d9291a0a-6061-4c9f-aac7-885d10190403
Original: core::ptr::real_drop_in_place<T>
New: core::ptr::real_drop_in_place<T> | core::ptr::real_drop_in_place<T> | core::ptr::real_drop_in_place<T> | geckoservo::glue::Servo_StyleSet_Drop
Same?: False
That's silly. Signatures shouldn't have consecutive duplicate parts.
The stack for this crash is this:
1. static void core::ptr::real_drop_in_place<smallvec::SmallVec<[style::stylist::Rule; 1]>>(struct smallvec::SmallVec<[style::stylist::Rule; 1]>*)
2. static void core::ptr::real_drop_in_place<hashglobe::table::RawTable<style::gecko_string_cache::Atom, smallvec::SmallVec<[style::stylist::Rule; 1]>>>(struct hashglobe::table::RawTable<style::gecko_string_cache::Atom, smallvec::SmallVec<[style::stylist::Rule; 1]>>*)
3. static void core::ptr::real_drop_in_place<style::stylist::CascadeData>(struct style::stylist::CascadeData*)
4. void geckoservo::glue::Servo_StyleSet_Drop(struct style::gecko_bindings::sugar::ownership::Owned<style::gecko_bindings::bindings::RawServoStyleSet>)
This bug covers looking into this and fixing it (remove consecutive duplicate frames) or discovering why we shouldn't do that.
|
Reporter | |
Comment 1•6 years ago
|
||
I don't think the work involved is hard. It looks like it involves adding a de-dupe line around here:
First question, is there any reason to keep consecutive duplicate frames in a signature?
|
|
Reporter | |
Comment 2•6 years ago
|
||
I asked around about this. Seems like we want to differentiate between "fun1 | fun1 | fun2" and "fun1 | fun2" because they're different stacks.
Nathan pointed out that maybe signature generation shouldn't be replacing the interesting bits for <T> at all. Socorro does this for both C++ and Rust code. Lars says that was added as part of bug #1163831. Seems like we should either keep the <T> thing or replace that code with something that has similar length characteristics.
Given that, seems like a bad idea to de-duplicate consecutive duplicate frames. One option is to create a new bug for figuring out a more nuanced handling for Rust template instantiation instead of replacing with <T>.
Anyhow, marking this as WONTFIX.
Description
•