[FIX] Crash [@ nsStyleContext::FindChildWithRules] with ::first-line, appending rows and table-cells, etc

RESOLVED FIXED

Status

()

Core
Layout
--
critical
RESOLVED FIXED
11 years ago
4 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: mats)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
x86
All
crash, fixed1.8.1, regression, testcase, verified1.8.0.8
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.7 -
blocking1.8.0.8 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical][baking 8/29][reflow-refactor], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
See upcoming testcase, which crashes current branch (1.8.0.x and 1.8.1) and trunk builds.
It doesn't crash Mozilla1.7.13.
This seems to have regressed between 2005-01-03 and 2005-01-04:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-01-03+11&maxdate=2005-01-04+08&cvsroot=%2Fcvsroot
Maybe a regression from bug 271422 somehow?

Talkback ID: TB22561642M
nsStyleContext::FindChildWithRules   nsStyleSet::GetContext   nsStyleSet::ReParentStyleContext   nsFrameManager::ReParentStyleContext   nsFirstLineFrame::PullOneFrame   nsLineLayout::ReflowFrame  

I have several in-between testcases, that crash in different areas, mostly in the style system. I can attach those, if wanted.
(Reporter)

Comment 1

11 years ago
Created attachment 235640 [details]
testcase, crashes on load
(Reporter)

Comment 2

11 years ago
Created attachment 235641 [details]
zip file of original testcase and all in-between testcases
(Assignee)

Comment 3

11 years ago
Patch coming up...
Assignee: nobody → mats.palmgren
Summary: Crash [@ nsStyleContext::FindChildWithRules] with ::first-line, appending rows and table-cells, etc → [FIX] Crash [@ nsStyleContext::FindChildWithRules] with ::first-line, appending rows and table-cells, etc
Flags: blocking1.9?
Flags: blocking1.8.1?

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+
(Assignee)

Comment 4

11 years ago
Created attachment 235697 [details] [diff] [review]
Patch rev. 1

The problem is that we iterate past the beginning on a line iterator.
We actually have an assertion for this but still continue to execute
leading to a near certain crash on Windows (it crashes less often on
Linux for some reason).

Test results with the patch:
I ran all the testcases in the zip archive for at least 5 minutes,
up to an hour for some, on both Windows and Linux and there were
no crashes. There are plenty of nasty assertions though, especially
the table related ones looks interesting...

I wanted to have two separate assertions, the first indicates a broken
frame tree while the second could also happen for a valid frame tree.
Attachment #235697 - Flags: superreview?(bzbarsky)
Attachment #235697 - Flags: review?(bzbarsky)
(Assignee)

Comment 5

11 years ago
sg: we're using random memory and in some cases overwrite the stack.
Flags: blocking1.8.0.7?
OS: Windows XP → All
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

Looks ok for branches.  On trunk, I'd just as soon not touch this code -- it's completely gone on reflow branch, and changing it will just make merging the branch that much more work.
Attachment #235697 - Flags: superreview?(bzbarsky)
Attachment #235697 - Flags: superreview+
Attachment #235697 - Flags: review?(bzbarsky)
Attachment #235697 - Flags: review+
Whiteboard: [reflow-refactor]
Landing it on the trunk is fine -- these pieces are the easy ones to merge.
If dbaron's ok with it, then go ahead and land on trunk too, sure.
(Assignee)

Comment 9

11 years ago
Landed on trunk at 2006-08-28 00:30 PDT.

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
This will have to wait for 1.8.0.8
Flags: blocking1.8.0.7? → blocking1.8.0.8?
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers

We are slipping the candidate build until tomorrow. Please land this today to make 1.8.0.7
Attachment #235697 - Flags: approval1.8.0.7+
Flags: blocking1.8.0.8? → blocking1.8.0.7+
Attachment #235697 - Flags: approval1.8.1?

Updated

11 years ago
Whiteboard: [reflow-refactor] → [baking 8/29][reflow-refactor]
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

we're building now, I guess this will have to be 1.8.0.8 after all.
Attachment #235697 - Flags: approval1.8.0.7+ → approval1.8.0.8?
Flags: blocking1.8.0.8+
Flags: blocking1.8.0.7-
Flags: blocking1.8.0.7+

Comment 13

11 years ago
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

a=schrep for drivers.
Attachment #235697 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 14

11 years ago
Landed on MOZILLA_1_8_BRANCH at 2006-08-31 09:57 PDT.
Keywords: fixed1.8.1
Restoring lost blocking flag
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8? → blocking1.8.0.8+
Comment on attachment 235697 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #235697 - Flags: approval1.8.0.9? → approval1.8.0.8+
(Assignee)

Comment 17

11 years ago
Landed on MOZILLA_1_8_0_BRANCH at 2006-10-05 16:56 PDT.
Flags: blocking1.9?
Keywords: fixed1.8.0.8
Whiteboard: [baking 8/29][reflow-refactor] → [sg:critical][baking 8/29][reflow-refactor]

Comment 18

11 years ago
v.fixed on 1.8.0 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.8pre) Gecko/20061020 Firefox/1.5.0.8pre, no crash.
Keywords: fixed1.8.0.8 → verified1.8.0.8
Group: security
Comment #0 mentions the in-between testcases that crash in "different areas". Does the fix in this bug fix all of those or should we have spun off different bugs?
Group: security
(Assignee)

Comment 20

11 years ago
Comment 4 claims all crashes were fixed.  There were a few "interesting"
assertions though that might be worth reporting if they still occur.
Group: security
Flags: in-testsuite?
Crash Signature: [@ nsStyleContext::FindChildWithRules]
(Assignee)

Comment 21

4 years ago
Crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b4983feaedc
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/1b4983feaedc
You need to log in before you can comment on or make changes to this bug.