1433952 - Crash in mozilla::RestyleManager::ProcessPostTraversal

Status: NEW

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

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-3efe8e7d-4df5-4aa5-b1ac-aacdd0180129.
=============================================================

This is showing large crash numbers in B4, but with only 5 installs: http://bit.ly/2BBLtRV. Crash reason for all is EXCEPTION_STACK_OVERFLOW. Not sure what may be causing this spike.

Top 10 frames of crashing thread:

0 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:775
1 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
2 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
3 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
4 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
5 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
6 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
7 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
8 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956
9 xul.dll mozilla::ServoRestyleManager::ProcessPostTraversal layout/base/ServoRestyleManager.cpp:956

=============================================================

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Looks like it's simply a too deep tree to restyle. 89 level doesn't sound that deep, but maybe there are omitted levels given the first non-ProcessPostTraversal is frame 2780?

Comment 2

[Jet Villegas (inactive)]

[ Triage 2017/02/20: P2 ] P2 bugs may become P1's after further analysis. Please prioritize diagnosis and repair.

Comment 3

[Mats Palmgren (inactive)]

Still occurs: bp-d1dbfc33-1caf-433e-aa16-8c56c0180606

Comment 4

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 5

[Gabriele Svelto [:gsvelto]]

Added a new signature

Comment 6

[Gabriele Svelto [:gsvelto]]

Actually there's a long tail of signatures that looks similar to this one, see here.

Comment 7

[Daniel Holbert [:dholbert]]

A few notes:
(1) [@ mozilla::ServoRestyleManager::ProcessPostTraversal ] has zero crash volume at this point (probably because ServoRestyleManager no longer exists under that name), so let's remove that from the signatures list.

(2) The attached testcase doesn't trigger the issue at this point. It does crash in Nightly 2019-02-12 (the day it was attached) but it became fixed shortly afterwards, with this fix range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2a82b9a67559d93cd57ff9738b782fb43717afd1&tochange=49b2a4c8be018f92d050512f9646cb3004ec1bec
(Given contenteditable in the testcase, I assume editor-related Bug 1525481 would be the thing that changed our behavior there.)

(3) For the mozilla::RestyleManager::ProcessPostTraversal crashes that come from pre-release builds (e.g. bp-4c9bfd74-8ffb-4df6-b225-b1b670221211), the crash seems to always in fact be a diagnostic assert, with this MOZ_CRASH Reason:

MOZ_DIAGNOSTIC_ASSERT(aElement->HasServoData()) (Element without Servo data on a post-traversal? How?) 

https://searchfox.org/mozilla-central/rev/9b8ebf06145feeccf34dc126cf45b07e86556392/layout/base/RestyleManager.cpp#2703-2704
(This diagnostic assert was added in bug 1458556, FWIW.)

emilio, do you have any ideas about what might be going on here?

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Hard to say without a test-case, that should really not be happening. It means we either have no styled an element that should've or that we've set wrong style bits in display: none subtrees.

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 9043488 [details]
testcase.html

I spun off bug 1806189 to land Tyson's attached testcase.html as a regression test, and I'm marking it obsolete here, since it's no longer a testcase that reproduces this still-unfixed bug here.

Tyson: if fuzzers still happen to hit this crash signature, it'd be great to get new testcase - thanks! (toggling needinfo in case you have any way of grepping to see if this has been hit recently; but feel free to just clear it if you're not seeing anything or don't have that info at your fingertips.)

Comment 10

[Tyson Smith [:tsmith]]

Sorry I don't have new test case. This was last reported while fuzzing m-c 20190214-f0ea53f47215.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 15 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.