36902 - Trunk crash [FIX]SEGV in [@ nsImageFrame::Paint] when visiting Dejanews powersearch

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P3)

Description

[Chris Siebenmann]

Build: recent CVS pull

 Visiting the URL mentionted results in a SEGV in libraptorhtml.so's
nsImageFrame::Paint routine. Partial stack backtrace:

(gdb) where
#0  0x40bc99bf in nsImageFrame::Paint ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#1  0x40bb6fbf in nsContainerFrame::PaintChild ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#2  0x40bb6e94 in nsContainerFrame::PaintChildren ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#3  0x40bc3a4e in nsHTMLContainerFrame::Paint ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#4  0x40bb6fbf in nsContainerFrame::PaintChild ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#5  0x40bb33c5 in nsBlockFrame::PaintChildren ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so
#6  0x40bb3215 in nsBlockFrame::Paint ()
   from /scratch/mozilla/dist/bin/components/libraptorhtml.so

[I will put in the full stack backtrace as an attachment.]

Comment 1

[Chris Siebenmann]

Attachment: full gdb stack backtrace

Comment 2

[Andreas Franke (gone)]

Attachment: similar backtrace (no debug symbols), PC/Linux, 2000042113

Comment 3

[Andreas Franke (gone)]

Confirming bug. Reproduced on PC/Linux with builds 2000042113 and 2000042518.
See above attachment for a backtrace (without debug symbols in libraptorhtml.so)
However, I always crash like this:
#0  0x0 in ?? ()

Steps to reproduce:
1. load http://www.deja.com/home_ps.shtml
2. enter asdfasdfasdfasdf in the "DISCUSSIONS SEARCH" text input field
   which is located at the top right of the page
3. click on the "SEARCH>>" button right next to it
4. watch the window title to change to "Deja.com: Error - Mozilla", and crash.

Adding crash keyword, setting severity to critical.

Comment 4

[Andreas Franke (gone)]

Attachment: Testcase for crash, probably non-minimal

Comment 5

[Andreas Franke (gone)]

Attachment: full stack trace for above testcase (with function symbols)

Comment 6

[Andreas Franke (gone)]

This full stack trace shows that the testcase attached is something completely
different from the original bug report. It crashes in nsLineBox::DeleteLineList
and not in "Paint()" methods. Unfortunately I had mixed these things up.
Bug 37493 is about www.starwars.com being another testcase for the crash 
I observed, so my issue could be handled there as well.

[e-mail]: Can you provide a little more
information about your case? Have you been able to reproduce it, and does 
it still occur in the actual tree? Or is it some random behaviour?
Can you think of any reason why I can always load the mentioned URL 
without segv'ing? I'm sorry that I made it such a mess.

Comment 7

[Chris Siebenmann]

 I can still reproduce this 100% in the near-latest CVS tree.

 What appears to do it is using a proxy. If I don't use a proxy,
all is fine. If I use my usual proxy (the junkbuster proxy from
http://www.junkbusters.com/) it dies promptly and reliably.

 Prefs.js entries I'm using that may be relevant:
	user_pref("network.http.proxy.keep-alive", 0);
	user_pref("network.proxy.http", "128.100.102.185");
	user_pref("network.proxy.http_port", 8000);
	user_pref("network.proxy.type", 1);
(note that my proxy is not useable by outside people, so the
most interesting ones are the type and perhaps the keep-alive
setting).

Comment 8

[Andreas Franke (gone)]

I still can't reproduce this. From home, I'm using a proxy (Squid), and
build 2000042609 works fine. I also installed junkbuster on one of our
machines at the university, and it's still fine on all mozilla versions
I've tested.
Since the crash is in paint methods, it shouldn't be network related.
Maybe it's the new page created by junkbuster that crashes. If this is
true, then the information from your blocker file will be necessary to 
reproduce this. Can you attach it to this bug? Also could you try to
load the page with a different, non-crashing browser, save it to a local
disk, and see if mozilla also crashes on this local file? (Maybe it's 
necessary to load the complete page into composer and save the it from there,
including all gifs etc.) If so, it  would be most helpful if you could attach
the page or a compressed archive of all the neccessary files to this bug.
Thanks.

Comment 9

[Chris Siebenmann]

Attachment: junkbuster 'blockfile' configuration file

Comment 10

[Chris Siebenmann]

 This is reproduceable in the latest nightly build with my current proxy
configuration (attached above).
 It happens for a local version of the HTML, but the raw HTML calls out
to other servers to load the image, so the proxy is still involved.

 You will have to clear your disk cache in order to test it; if the disk
cache is loaded with the images (such as by visiting a stored version of
the page with the proxy off) later loads of the stored page will be fine.

Comment 11

[Chris Siebenmann]

 I've found a minimal test case.

 The problem appears to be when an image has both a 'lowsrc=...' and a
'src=...' tag, and the SRC version of the image but not the LOWSRC one
is blocked by the proxy. (It has to be blocked, as opposed to just not
being there.)

 I'll attach the minimal test case (you need to block 'deja.com/ads' in
the junkbuster config) here.

Comment 12

[Chris Siebenmann]

Attachment: minimal HTML example to cause SEGV with deja.com/ads blocked

Comment 13

[Kevin McCluskey (gone)]

Reassinging to rods, lowsrc..src implementor.

Comment 14

[rods (gone)]

waqar, this works fine on NT could check it out on Linux?

Comment 15

[Andreas Franke (gone)]

Attachment: screenshot of above testpage, using IJ/2.0.2, blocking web2.deja.com/ads

Comment 16

[Andreas Franke (gone)]

I still can't reproduce this (see above screenshot).

[e-mail]: Can you guess what else
could be the reason that I'm still not crashing? It's a direct
connection here, but that should not make any difference.
What should the screenshot be like in case that mozilla behaves 
as it should?

Comment 17

[waqar]

Accepting

Comment 18

[Chris Siebenmann]

 I can't see what would be different about our configurations
that would cause my Mozillas to crash consistently and yours
not to. If you give me an IP number or the like, I can fiddle
my junkbuster configuration so that you can work through it
so you're using my exact configuration/etc there.

Comment 19

[Andreas Franke (gone)]

Oops, it seems that my last comment got lost due to some database
connectivity problems yesterday. So here we go once again:

I finally could reproduce the crash on http://www.deja.com/home_ps.shtml
several times (about 4 times out of ~20 tries) using Chris' junkbuster.
I took two full stack traces; one of them was really identical to the first 
attachment of this bug, and had was nearly identical (#0-#36 were the same).

Thus the good news is that this bug is real... ;-)

Comment 20

[waqar]

This bug has been marked "future" because the original netscape engineer working
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way
-- please attach your concern to the bug for reconsideration.

Comment 21

[Chris Siebenmann]

 This bug continues to pretty much block me from using DejaNews with
Mozilla. Since I had the time, I built a Mozilla version (pulled from
CVS today) without '--disable-debug', and captured a 'where full' gdb
stack backtrace, which I will make an attachment of.

Comment 22

[Chris Siebenmann]

Attachment: gdb 'where full' and 'where' backtraces on a debugging version

Comment 23

[Mike]

M16 has been out for a while now, these bugs target milestones need to be 
updated.

Comment 24

[waqar]

2/23/2000 build it works fine for me. I can visit the deja page and the test
case as well without crashing.

Comment 25

[Chris Petersen]

Verified works for me in the July 11th build.

Comment 26

[Chris Siebenmann]

 I'm afraid that this bug is still there for me with a CVS pull from today.
Same stack backtrace. Thus: reopening the bug.

Comment 27

[Andreas Franke (gone)]

Reassigning (back) to [e-mail] based on CVS blame, cc previous owner.

This comment should summarize all the information necessary to fix this bug.
Especially note that my attachments made between 4/26 and 4/29 are irrelevant to
this bug, please ignore them.

From the full stack trace (attachment 06 [details]/17/00 16:53), you can see that the
crash happens in nsImageFrame.cpp, line 606, while dereferencing image which is
null. lowImage is non-null however, and that's why we get into the else branch:
 
 585                    if (nsnull == image && nsnull == lowImage) {
 586 troy     1.6         // No image yet, or image load failed. 
                          // Draw the alt-text and an icon
 587                      // indicating the status
 588 nisheeth 1.113       if (NS_FRAME_PAINT_LAYER_BACKGROUND == aWhichLayer &&
 589                          !mInitialLoadCompleted) {
 590 kipp     1.38          DisplayAltFeedback(aPresContext, aRenderingContext,
 591                                           mImageLoader.GetLoadImageFailed()
 592                                           ? NS_ICON_BROKEN_IMAGE
 593                                           : NS_ICON_LOADING_IMAGE);
 594                      }
 595 kipp     1.1       }
 596 kipp     1.71      else {
 597 nisheeth 1.113       mInitialLoadCompleted = PR_TRUE;
 598 kipp     1.73        if (NS_FRAME_PAINT_LAYER_FOREGROUND == aWhichLayer) {
 599 kipp     1.71          // Now render the image into our content area 
                            // (the area inside the
 600                        // borders and padding)
 601                        nsRect inner;
 602 tbogard  1.101         GetInnerArea(aPresContext, inner);
 603 kipp     1.71          if (mImageLoader.GetLoadImageFailed()) {
 604                          float p2t;
 605 tbogard  1.101           aPresContext->GetScaledPixelsToTwips(&p2t);
 606 rods     1.117           inner.width  =                
                                    NSIntPixelsToTwips(image->GetWidth(), p2t);
 607 kipp     1.71            inner.height = 
                                    NSIntPixelsToTwips(image->GetHeight(), p2t);
 608                        }
 609 rods     1.117 
 610                        if (lowImage != nsnull && lowSrcLinesLoaded > 0) {
 611                          //inner.height = lowSrcLinesLoaded;
 612                          aRenderingContext.DrawImage(lowImage, inner);
 613                        }
 614                
 615                        if (image != nsnull && imgSrcLinesLoaded > 0) {
 616                          //inner.height = imgSrcLinesLoaded;
 617                          aRenderingContext.DrawImage(image, inner);
 618                        }
 619 kipp     1.71        }

It seems to me that a non-null lowImage together with a null image is a valid
situation at this point (see e.g. line 615), but it inevitably leads to a crash.

Maybe you could fix this even without a testcase? Chris (the original reporter)
could test your patch and verify that it fixes the crash for him.

Comment 28

[Chris Siebenmann]

 Armed with Andreas Franke's analysis (thank you, Andreas!) I've made a
simplistic, obvious patch to try to work around the problem, which I will
glue in as an attachment. A patched Mozilla no longer crashes, and seems
to otherwise behave normally. I don't know if what I'm doing in the patch
is itself proper, or if there is a better alternative; it should obviously
be reviewed by someone more familiar with the code.

Comment 29

[Chris Siebenmann]

Attachment: obvious patch to nsImageFrame.cpp to fix things up

Comment 30

[rods (gone)]

fix is from submitted patch, looks like the right thing to, low risk

Comment 31

[Jay Patel [:jay]]

Putting on [nsbeta2-] radar. Not critical to beta2. This is in a debug build 
renominate if you see this in the commercial builds.  Also, if the fix is 
from an outside contributor, talk to watterson about the a checkin.

Comment 32

[rods (gone)]

fixed

Comment 33

[Chris Petersen]

Fixed in the July 26th build.

Comment 34

[Jan Carpenter]

Reopening
This bug is a topcrasher, added topcrash keyword.  
Added [@ nsImageFrame::Paint] for tracking.  

Here are some URLs & Comments that might help repro this crash:

    (28432441) URL: 
http://rigaut.home.cern.ch/rigaut/about.html
     (28617667) Comments: search mail/news messages.
Open message in a standalone window.  crashed.


Here is a recent stack trace:

         nsImageFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsContainerFrame::PaintChildren()  
         nsHTMLContainerFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsBlockFrame::PaintChildren()  
         nsBlockFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsContainerFrame::PaintChildren()  
         nsTableCellFrame::Paint()  
         nsTableRowFrame::PaintChildren()  
         nsTableRowFrame::Paint()  
         nsTableRowGroupFrame::PaintChildren()  
         nsTableRowGroupFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsContainerFrame::PaintChildren()  
         nsTableFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsTableOuterFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsBlockFrame::PaintChildren()  
         nsBlockFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsBlockFrame::PaintChildren()  
         nsBlockFrame::Paint()  
         nsContainerFrame::PaintChild()  
         nsContainerFrame::PaintChildren()  
         nsHTMLContainerFrame::Paint()  
         PresShell::Paint()  
         nsView::Paint()  
         nsViewManager::RenderDisplayListElement()  
         nsViewManager::RenderViews()  
         nsViewManager::Refresh()  
         nsViewManager::DispatchEvent()  
         HandleEvent()  
         nsWidget::DispatchEvent()  
         nsWidget::DispatchWindowEvent()  
         nsWindow::DoPaint()  
         nsWindow::Update()  
         nsWindow::Update()  
         nsViewManager::Composite()  
         nsViewManager::UpdateViewAfterScroll()  
         nsScrollPortView::Scroll()  
         nsScrollPortView::ScrollTo()  
         nsScrollPortView::ScrollByLines()  
         nsEventStateManager::DoWheelScroll()  
         nsEventStateManager::PostHandleEvent()  
         PresShell::HandleEventInternal()  
         PresShell::HandleEvent()  
         nsView::HandleEvent()  
         nsView::HandleEvent()  
         nsView::HandleEvent()  
         nsViewManager::DispatchEvent()  
         HandleEvent()  
         nsWidget::DispatchEvent()  
         nsWidget::DispatchWindowEvent()  
         nsWidget::OnButtonPressSignal()  
         nsWindow::HandleGDKEvent()  
         dispatch_superwin_event()  
         handle_gdk_event()  
         libgdk-1.2.so.0 + 0x17d00 (0x40622d00)  
    nsImageFrame::Paint f534b632

Comment 35

[Jay Patel [:jay]]

Could this be a dup of 74017?  Adding sairuh to cc list since she seems to know 
more about this crash.

Comment 36

[Jan Carpenter]

*** Bug 76249 has been marked as a duplicate of this bug. ***

Comment 37

[Jan Carpenter]

Here are more URLs & Comments that might help repro this crash:
     (29135392) URL: http://www.amd.com/products/cpg/bin/miniport_480_winme.exe
     (29135309) URL: http://www.amd.com/products/cpg/bin/miniport_480_winme.exe
     (28892233) Comments: Crash when deleting emails
     (28891764) Comments: crash while deleting emails

Comment 38

[sairuh (rarely reading bugmail)]

i think this is different from bug 74017, since 74017 is particular to the
recent libpr0n landing. cc'ing terri and pav to see what they think.

Comment 39

[Asa Dotzler [:asa]]

<Hixie> Should this really be a reopened bug? It seems like it would be a new
bug to me.

Comment 40

[Jay Patel [:jay]]

Putting this bug back to Verified Fixed.  No need to log another bug for the 
latest nsImageFrame::Paint crash, since this crash hasn't happened since build 
2001041322.

Comment 41

[Jay Patel [:jay]]

Ok, this is now back to it's status on 07/26/00.  If the crash mentioned in any 
comments later than that date comes up again, we'll log another bug.