Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Crash/hang [@ nsLineBox::RemovePlaceholderDescendantsOf ][@ ntdll.dll] on avrev.com with Adblock Plus 0.7.1.1 enabled

RESOLVED FIXED in mozilla1.8.1

Status

()

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

People

(Reporter: Weston Carloss, Assigned: Mats Palmgren (vacation - back in August))

Tracking

(4 keywords)

1.8 Branch
mozilla1.8.1
x86
All
crash, regression, verified1.8.0.7, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
wanted1.8.1.x +
blocking1.8.0.7 +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] uses freed memory, crash signature, URL)

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060814 BonEcho/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060814 BonEcho/2.0b1

When I visit avrev.com with Adblock Plus enabled Bon Echo hangs or crashes.  Disabling Adblock Plus allows the site to work correctly.

Reproducible: Always

Steps to Reproduce:
1. Visit avrev.com with Adblock Plus enabled.
2. If Bon Echo does not immediately hang or crash, try clicking a link.





Using crash2 I have this Talkback ID:  TB22100260K
(Reporter)

Comment 1

11 years ago
Here's a Talkback ID with Minefield crashing on the same site with Adblock Plus.

TB22101355E

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060814 Minefield/3.0a1 ID:2006081404 [cairo]
Adblock Plus 0.7.1.1 can be downloaded from here:
https://addons.mozilla.org/firefox/1865/

I don't get a crash with 2006-07-28 branch build, I get the crash with 2006-07-30 branch build.
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-28+05&maxdate=2006-07-30+10&cvsroot=%2Fcvsroot
I don't get a crash with 2006-07-28 1.5.0.x build, I get the crash with 2006-08-14 1.5.0.x build.
I don't get a crash at all with trunk builds.

I think the fix for bug 337883 caused this somehow. It's the only patch, that also was checked in the 1.8.0.x tree.

Getting a minimised testcase out of this would be very tough.
Blocks: 337883
Severity: normal → critical
Component: General → Layout
Keywords: crash, regression
Product: Firefox → Core
QA Contact: general → layout
Summary: Crash/hang on avrev.com with Adblock Plus 0.7.1.1 enabled → Crash/hang [@ nsLineBox::RemovePlaceholderDescendantsOf ][@ ntdll.dll] on avrev.com with Adblock Plus 0.7.1.1 enabled
Version: unspecified → 1.8 Branch

Updated

11 years ago
Flags: blocking1.8.1?
Flags: blocking1.8.0.7?
I can reproduce it in a Firefox trunk debug build on Linux. I'll have a look.
(Marking Security-Sensitive because it uses freed memory.)
Assignee: nobody → mats.palmgren
Group: security
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Well, this was widely discussed in http://forums.mozillazine.org/viewtopic.php?t=451790&start=45
and the bug report was mentioned in:
http://forums.mozillazine.org/viewtopic.php?t=451790&start=45#2432005
Created attachment 233774 [details]
Testcase #1

When we remove a float inside an inline we fail to update the float
containing block's (FCB) float cache which will contain the destroyed
placeholder frame. Removing another frame directly in the FCB will
crash in nsLineBox::RemovePlaceholderDescendantsOf.

Making nsInlineFrame::RemoveFrame() also update the float cache of
the nearest FCB fixes the crash for me... patch soon, need to investigate
if there are other cases and the performance impact first...
Whiteboard: [sg:critical?] uses freed memory
This is more or less the same as bug 337883. Both are in fact regressions
from bug 310638, this line to be precise:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1249&root=/cvsroot&mark=9775#9745

Nulling out mOutOfFlowFrame this early breaks the float cache which depends on
it to find out which FC entry is associated with the float:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.786&root=/cvsroot&mark=5564,5575#5557
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.112&root=/cvsroot&mark=513,517#507
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsLineBox.cpp&rev=1.112&root=/cvsroot&mark=913#907

I think the correct fix is to remove the SetOutOfFlowFrame(nsnull) line
and backout bug 337883. I will run some more tests on this...
Flags: blocking1.8.1? → blocking1.8.1+

Updated

11 years ago
Target Milestone: --- → mozilla1.8.1
1.8.0.7 code freeze is coming up soon (Aug 26 I think). Is it reasonable to expect a patch by then?
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Whiteboard: [sg:critical?] uses freed memory → [sg:critical?] uses freed memory [at risk]
Created attachment 234014 [details] [diff] [review]
Patch rev. 1

The real fix for this bug and bug 337883 is to remove that
SetOutOfFlowFrame(nsnull) line (the rest of the patch just backs out
bug 337883). I wasn't aware that the float cache depends on that pointer
being intact during frame destruction, mea culpa.

I have tested this patch on 1.8/1.8.0 branches as well.
Attachment #234014 - Flags: superreview?(roc)
Attachment #234014 - Flags: review?(bzbarsky)
I'm a bit surprised that the various testing tools we have does not trigger
this crash. The testcase in this bug for example is fairly simple and looks
to me as if should be triggered, especially if you waste many, many CPU-hours
on it ;-)
Bug 339651 seems related.
Thanks Martijn. The patch fixes bug 339651 too. Further analysis on that bug.
Blocks: 339651

Comment 12

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

>Index: layout/base/nsCSSFrameConstructor.cpp
>-        ((nsPlaceholderFrame*)childFrame)->SetOutOfFlowFrame(nsnull);

Add a comment here that we should not SetOutOfFlowFrame(nsnull), possibly with a pointer to either this bug or the float cache code or both?

r=bzbarsky with that.
Attachment #234014 - Flags: review?(bzbarsky) → review+
Attachment #234014 - Flags: superreview?(roc) → superreview+
Created attachment 234218 [details] [diff] [review]
rev. 1 + a comment
Checked in to trunk at 2006-08-17 06:15 PDT.
(Will ask for branch approvals in a few days...)

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Attachment #234014 - Flags: approval1.8.1?
Attachment #234014 - Flags: approval1.8.0.7?

Updated

11 years ago
Whiteboard: [sg:critical?] uses freed memory [at risk] → [sg:critical?] uses freed memory [at risk][181approval pending]
Comment on attachment 234014 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #234014 - Flags: approval1.8.0.7? → approval1.8.0.7+
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-21 13:34 PDT.
Keywords: fixed1.8.0.7

Comment 17

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

a=schrep for drivers - approving all [181approval pending] bugs now that tree is open.
Attachment #234014 - Flags: approval1.8.1? → approval1.8.1+
(Reporter)

Comment 18

11 years ago
Has this for certain been checked in on the Branch for 2.0?  I no longer see crashes on the trunk, but I believe I just got the same crash on the same site with Adblock Plus 0.7.1.2.

TB22441403X

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060823 BonEcho/2.0b2 ID:2006082303

Comment 19

11 years ago
> Has this for certain been checked in on the Branch for 2.0? 

It has for certain NOT been checked in.  When it is, the "fixed1.8.1" keyword will be added.
Checked in to MOZILLA_1_8_BRANCH at 2006-08-23 21:16 PDT.
Keywords: fixed1.8.1
Whiteboard: [sg:critical?] uses freed memory [at risk][181approval pending] → [sg:critical?] uses freed memory

Comment 21

11 years ago
v.fixed on both branches with 8/24 nightly builds, no crashes with AdBlock Plus and testcase in this bug.

Weston:  Are you still crashing with today's (20060824) Bon Echo builds?
Keywords: fixed1.8.0.7, fixed1.8.1 → verified1.8.0.7, verified1.8.1
(Reporter)

Comment 22

11 years ago
Neither avrev.com or the testcase crash with today's build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060824 BonEcho/2.0b2 ID:2006082403).  This has been fixed.

Updated

11 years ago
Depends on: 373544
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: in-testsuite?
Group: security

Comment 23

10 years ago
I checked in Mats Palmgren's testcase as a crashtest.
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsLineBox::RemovePlaceholderDescendantsOf ] [@ ntdll.dll]
You need to log in before you can comment on or make changes to this bug.