Closed Bug 462968 Opened 16 years ago Closed 16 years ago

Crash [@ nsCSSFrameConstructor::FindPrimaryFrameFor] with -moz-column, clear, td, huge padding

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: jruderman, Assigned: roc)

References

Details

(Keywords: crash, testcase, verified1.9.1, Whiteboard: [sg:critical])

Crash Data

Attachments

(2 files)

Attached file testcase
Testcase is similar to the one in bug 452165, which WFM.

Crash [@ nsCSSFrameConstructor::FindPrimaryFrameFor], calling a random address.
Flags: blocking1.9.1?
Whiteboard: [sg:critical]
Crashes on Linux as well.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre
http://crash-stats.mozilla.com/report/index/8161d2e8-ab70-11dd-b27f-001a4bd43e5c
OS: Mac OS X → All
Summary: Crash [@ nsCSSFrameConstructor::FindPrimaryFrameFor] with -moz-column, clear, td → Crash [@ nsCSSFrameConstructor::FindPrimaryFrameFor] with -moz-column, clear, td, huge padding
Assignee: nobody → roc
Attached patch fixSplinter Review
This is somewhat band-aid-ish. We're dealing with integer overflows here and there are a *lot* of ways to trigger those. I think screening out negative heights like this is worth it for blocks since they have vertical breaking. Obviously it's only a partial solution but I'm not keen on doing saturating arithmetic for every height and width calculation. Maybe we should up the priority of switching to float nscoords.
Attachment #347940 - Flags: superreview?(dbaron)
Attachment #347940 - Flags: review?(dbaron)
Is it possible to make layout not blow up when widths and heights are wrong?  We might find that it's hard to rely on float arithmetic being "correct" as well.
We rarely depend on widths and heights being "correct", but in some places we do depend on them not being absurd, e.g., negative. Or we depend on invariants like a>0 and b>0 implies a+b>0.

Yes, it would be nice if we didn't depend on these. But figuring out the cause of a crash after layout has gone horribly wrong for a while is hard. And adding code to try to handle completely invalid internal states is hard and adds complexity.
If we are going to do that, I'd prefer to do it using a simpler testcase than this one.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Whiteboard: [sg:critical] → [sg:critical][needs review]
Attachment #347940 - Flags: superreview?(dbaron)
Attachment #347940 - Flags: superreview+
Attachment #347940 - Flags: review?(dbaron)
Attachment #347940 - Flags: review+
Whiteboard: [sg:critical][needs review] → [sg:critical][needs landing]
Initially landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ebf0c7cf17be
http://hg.mozilla.org/mozilla-central/rev/026147c91538
and then backed out due to crashes caused by bug 430332:
http://hg.mozilla.org/mozilla-central/rev/9eaf91dad68c
and then relanded on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/6d6a178f9132
and thus fixed on both mozilla-central and releases/mozilla-1.9.1.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs landing] → [sg:critical]
Target Milestone: --- → mozilla1.9.1b3
Flags: in-testsuite?
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327
Status: RESOLVED → VERIFIED
Hardware: x86 → All
Crash Signature: [@ nsCSSFrameConstructor::FindPrimaryFrameFor]
Group: core-security
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f8221e516a3
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.