Closed Bug 509839 Opened 16 years ago Closed 15 years ago

Crash [@ nsViewManager::CreateView][@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal] on closing print preview with -moz-transform, position: fixed table displays

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5
Tracking Status
blocking1.9.2 --- .4+
status1.9.2 --- .4-fixed
blocking1.9.1 --- .10+
status1.9.1 --- .10-fixed

People

(Reporter: martijn.martijn, Assigned: MatsPalmgren_bugz)

Details

(5 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(6 files)

Attached file testcase
See testcase, which crashes current trunk build when closing the print preview window. http://crash-stats.mozilla.com/report/index/6009c81f-304b-4c1d-90e9-a8d5a2090811?p=1 0 kernel32.dll kernel32.dll@0x12afb 1 mozcrt19.dll _CxxThrowException throw.cpp:159 2 mozcrt19.dll operator new obj-firefox/memory/jemalloc/crtsrc/new.cpp:57 3 xul.dll nsViewManager::CreateView view/src/nsViewManager.cpp:276 4 xul.dll nsBoxFrame::CacheAttributes layout/xul/base/src/nsBoxFrame.cpp:287 5 xul.dll nsMenuPopupFrame::Init layout/xul/base/src/nsMenuPopupFrame.cpp:147 http://crash-stats.mozilla.com/report/index/31a95af1-31e4-415d-8ea6-96d662090811?p=1 0 kernel32.dll kernel32.dll@0x12afb 1 mozcrt19.dll _CxxThrowException throw.cpp:159 2 mozcrt19.dll operator new obj-firefox/memory/jemalloc/crtsrc/new.cpp:57 3 xul.dll nsCSSFrameConstructor::AddFrameConstructionItemsInternal layout/base/nsCSSFrameConstructor.cpp:5338 4 xul.dll nsCSSFrameConstructor::ProcessChildren layout/base/nsCSSFrameConstructor.cpp:9579 5 xul.dll nsCSSFrameConstructor::ConstructFrameFromItemInternal layout/base/nsCSSFrameConstructor.cpp:3927 6 xul.dll nsCSSFrameConstructor::ConstructFramesFromItem layout/base/nsCSSFrameConstructor.cpp:5531 7 xul.dll nsCSSFrameConstructor::ProcessChildren layout/base/nsCSSFrameConstructor.cpp:9592 8 xul.dll nsCSSFrameConstructor::ConstructFrameFromItemInternal layout/base/nsCSSFrameConstructor.cpp:3927 9 xul.dll nsCSSFrameConstructor::ConstructFramesFromItem layout/base/nsCSSFrameConstructor.cpp:5531 10 xul.dll nsCSSFrameConstructor::ProcessChildren layout/base/nsCSSFrameConstructor.cpp:9592 etc...
this potentially may be "use after free" judging from the linux stack and 0xdd... #4 <signal handler called> #5 0x00007f47f3af4730 in nsPropertyTable::PropertyList::Equals ( this=0xdddddddddddddddd, aCategory=0, aPropertyName=0x7f47f4d0ab90) at /opt/pub/firefox-35/src/content/base/src/nsPropertyTable.cpp:85 #6 0x00007f47f3af3fd8 in nsPropertyTable::GetPropertyListFor ( this=<value optimized out>, aCategory=0, aPropertyName=0x7f47f4d0ab90) at /opt/pub/firefox-35/src/content/base/src/nsPropertyTable.cpp:295 #7 0x00007f47f3af427f in nsPropertyTable::GetPropertyInternal ( this=0x7f47ea815b78, aObject={mObject = 0x7fff0f315f30}, aCategory=0, aPropertyName=0x7f47f4d0ab90, aRemove=1, aResult=0x0) at /opt/pub/firefox-35/src/content/base/src/nsPropertyTable.cpp:190 (gdb) frame 5 #5 0x00007f47f3af4730 in nsPropertyTable::PropertyList::Equals ( this=0xdddddddddddddddd, aCategory=0, aPropertyName=0x7f47f4d0ab90) at /opt/pub/firefox-35/src/content/base/src/nsPropertyTable.cpp:85 85 return mCategory == aCategory && mName == aPropertyName; (gdb) x/i $rip 0x7f47f3af4730 <_ZN15nsPropertyTable12PropertyList6EqualsEtP7nsIAtom+2>: cmp %si,0x48(%rdi) (gdb) p/x $rdi $1 = 0xdddddddddddddddd
I guess it might be security sensitive then. In Firefox3.5, it might be the propertyEquals crash thing, though.
Group: core-security
Flags: blocking1.9.2?
###!!! ASSERTION: Shouldn't be incomplete if availableHeight is UNCONSTRAINED.: 'aReflowState.availableHeight != NS_UNCONSTRAINEDSIZE', file /home/dbaron/builds/mozilla-central/mozilla/layout/generic/nsBlockFrame.cpp, line 1401 seems very likely to be related
print preview got involved in few similar memory corruptions when dealing with XBL in the past. is it possible that print preview has a fundamental bug that breaks invariants or is it just triggering an existing bug? > ###!!! ASSERTION: Shouldn't be incomplete if availableHeight is UNCONSTRAINED i see this assertion (or something worded very similarly) relatively often when fuzzing, so if this is the root cause there may be other bugs.
i ran valgrind before my first comment here and i didn't see dbaron's error - valgrind just reported wild pointer once. using valgrind 3.4.1 with options --tool=memcheck --trace-children=yes the testcase doesn't crash on today trunk and valgrind is silent. am i missing some advanced usage of valgrind? (i may have a build with strange malloc/new which valgrind misses for some reason)
Also worksforme in current trunk build.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
(In reply to comment #6) > i ran valgrind before my first comment here and i didn't see dbaron's error - > valgrind just reported wild pointer once. > using valgrind 3.4.1 with options --tool=memcheck --trace-children=yes > > the testcase doesn't crash on today trunk and valgrind is silent. > > am i missing some advanced usage of valgrind? (i may have a build with strange > malloc/new which valgrind misses for some reason) You're not missing advanced usage of valgrind; just advanced debugging of Gecko code. In particular (I usually mention it when I post valgrind output done this way, but forgot to here), when dealing with use-after-free problems on layout data structures, it's best to uncomment the line: //#define DEBUG_TRACEMALLOC_PRESARENA 1 in layout/base/nsPresArena.cpp (which until a few days ago was in nsPresShell.cpp, with s/PRESARENA/FRAMEARENA/ ). This disables pool allocation and recycling of such structures and makes each one use malloc/free separately, so that they are tracked by valgrind.
thanks dbaron, that was valuable info. since the defines are changing, i am tempted to file bugs about: 1. buildconfig option that does the right #define 2. realistic testcases about use after and overflow that are detected by valgrind in standard build 3. since you seem to be using custom allocator, it better put heap canaries to check some integrity of the heap (libc does a decent job) [if not already done]
Actually, I still might have a testcase that crashes, I haven't minimized it yet.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
I no longer see the valgrind warning that I saw with the original testcase.
no valgrind warning here too.
Flags: wanted1.9.2-
Flags: blocking1.9.2?
Flags: blocking1.9.2-
>> //#define DEBUG_TRACEMALLOC_PRESARENA 1 this used to work but breaks today build: /opt/pub/firefox-central/src/layout/base/nsPresArena.cpp:206: error: expected ‘;’ before ‘(’ token this code is: void* Allocate(PRUnit32 /* unused */, size_t aSize) i am not sure this is valid c++, file a bug ?
Attached file testcase2
Whiteboard: [sg:critical?]
Mats, any chance you can work on this soon?
WFM, on Linux trunk. No warnings when run under Valgrind. Crashes in my Linux 1.9.2 debug build. I'm debugging this now.
Martijn, can you still reproduce on trunk? If it's fixed on trunk Linux, we should look for a fix range.
It was fixed on trunk in the range 2010-01-11-03 -- 2010-01-12-03: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=120667a01fd2&tochange=a43e2f7eda8f most likely by bug 526394.
OS: Windows XP → All
Hardware: x86 → All
Version: Trunk → 1.9.2 Branch
Ouch, that's unfortunate since I don't think we should fix 526394 on the branch!
I found the problem, here's the crash stack on 1.9.2: #0 ?? () #1 nsFrame::Destroy #2 nsBlockFrame::Destroy #3 nsFrameList::DestroyFrames #4 nsFrameList::Destroy #5 DestroyPropertyEnumerator #6 PL_DHashTableEnumerate #7 nsPropertyTable::DeleteAllProperties #8 PresShell::Destroy ... the property we're deleting is ExcessOverflowContainersProperty, so we're destroying a frame after the frame tree has been destroyed by the frame manager. (funny, we did discuss this very problem just recently in another bug) The reason we missed this frame list during frame tree destruction is that its removal is conditional on IsFrameOfType(nsIFrame::eCanContainOverflowContainers): http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsContainerFrame.cpp#277 and we did add this property on a frame that doesn't have that bit: nsPageContentFrame
This is the stack and frame tree when we create the property. (The assertion at the top is one I added to catch this) BTW, the bug also occurs on trunk, but it doesn't crash there.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Flags: blocking1.9.0.19?
Version: 1.9.2 Branch → unspecified
Attached patch fix for trunkSplinter Review
Attachment #437555 - Flags: review?(fantasai.bugs)
Attachment #437556 - Flags: review?(fantasai.bugs)
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Fantasai, can you hurry the review? Marking blocking for now, but if this misses the code freeze on Monday the 12th then we might have to wait for 3.6.5 :(
blocking1.9.1: ? → .10+
blocking1.9.2: ? → .4+
Attachment #437555 - Flags: review?(fantasai.bugs) → review+
Attachment #437556 - Flags: review?(fantasai.bugs) → review+
a=LegNeato for 1.9.2.4, 1.9.1.10 (approving before the request to move this along)
Attachment #437556 - Flags: approval1.9.2.4+
Attachment #437556 - Flags: approval1.9.1.10+
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/5df7dac32b28 (without the crashtests for now) Will push directly to branches as soon as the tree goes green...
Whiteboard: [sg:critical?][needs review] → [sg:critical?]
Status: REOPENED → RESOLVED
Closed: 16 years ago15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Attachment #437556 - Flags: approval1.9.0.19?
Verified for 1.9.2 using testcase 2 (which crashes 1.9.2.3) with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4pre) Gecko/20100413 Namoroka/3.6.4pre (.NET CLR 3.5.30729). Verified for 1.9.1 using same testcase and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.10pre) Gecko/20100413 Shiretoko/3.5.10pre (.NET CLR 3.5.30729).
Attachment #437556 - Flags: approval1.9.0.19? → approval1.9.0.next+
Comment on attachment 437556 [details] [diff] [review] fix for 1.9.2, 1.9.1 and 1.9.0 Approved for 1.9.0.20, a=dveditz (but please don't land the crash-tests until after the releases)
Checking in layout/generic/nsContainerFrame.cpp; /cvsroot/mozilla/layout/generic/nsContainerFrame.cpp,v <-- nsContainerFrame.cpp new revision: 1.316; previous revision: 1.315 done Checking in layout/generic/nsPageContentFrame.h; /cvsroot/mozilla/layout/generic/nsPageContentFrame.h,v <-- nsPageContentFrame.h new revision: 1.18; previous revision: 1.17 done
Flags: blocking1.9.0.19?
Keywords: fixed1.9.0.20
Group: core-security
Crash Signature: [@ nsViewManager::CreateView] [@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal]
Crash Signature: [@ nsViewManager::CreateView] [@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal] → [@ nsViewManager::CreateView] [@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal]
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: