77442 - Trunk crash (or hang) with linux scaled images [@ libc.so.6 - DoScale - DrawScaledImageNN]

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[John Morrison]

With either a new or existing profile, using today's verification comm. build 
on Linux, I either crash (stack below) or hang when I go to home.netscape.com
(and likely other pages). 

I believe this is what has been afflicting 'coffee' the tinderbox since 11pm
last night (orangeness due to crash running page loading test). 

I think this is also the same as one or both of the blockers from this morning
(they shouldn't have been downgraded -- mcafee, alecf, a tinderbox also say they 
can't run today's builds on Linux).

Start mozilla with '-url about:blank' -- loads OK
go to http://www.mozilla.org/ (e.g., a simple page) -- loads OK
go to http://www.google.com/ (another simple page) -- loads OK
go to http://home.netscape.com/ -- crash or hang




libc.so.6 + 0x5f117 (0x4022a117) 
libX11.so.6 + 0x28bf7 (0x406f6bf7) 
libX11.so.6 + 0x29385 (0x406f7385) 
libX11.so.6 + 0x2949e (0x406f749e) 
libX11.so.6 + 0x29850 (0x406f7850) 
DoScale() 
DrawScaledImageNN() 
nsImageGTK::DrawScaled() 
nsImageGTK::Draw() 
nsRenderingContextImpl::DrawScaledImage() 
nsImageFrame::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() 
CanvasFrame::Paint() 
PresShell::Paint() 
nsView::Paint() 
nsViewManager::RenderDisplayListElement() 
nsViewManager::RenderViews() 
nsViewManager::Refresh() 
nsViewManager::DispatchEvent() 
HandleEvent() 
nsWidget::DispatchEvent() 
nsWidget::DispatchWindowEvent() 
nsWindow::DoPaint() 
nsWindow::Update() 
nsWindow::UpdateIdle() 
libglib-1.2.so.0 + 0x10ba9 (0x406b0ba9) 
libglib-1.2.so.0 + 0xfbe6 (0x406afbe6) 
libglib-1.2.so.0 + 0x101a1 (0x406b01a1) 
libglib-1.2.so.0 + 0x10341 (0x406b0341) 
libgtk-1.2.so.0 + 0x8c209 (0x405d7209) 
nsAppShell::Run() 
nsAppShellService::Run() 
main1() 
main() 
libc.so.6 + 0x181eb (0x401e31eb)

Comment 1

[John Morrison]

-> blocker

Comment 2

[tor]

Works for me.

Anyone have a debug build and can provide a useful stack?

Comment 3

[Chris McAfee]

smoketest, I think this is why coffee is orange.

Comment 4

[Chris McAfee]

Here's the same stack, fails on first URL in the pageloader test:

#0  0x40369129 in ?? () from /lib/libc.so.6
#1  0x40368fba in ?? () from /lib/libc.so.6
#2  0x402c1a65 in PR_Free (ptr=0x88ec438) at prmem.c:66
#3  0x410343cf in DoScale (aDisplay=0x80b5768, aDest=46138510, aGC=0x899eaa8, 
    aSrc=46138472, aSrcWidth=1, aSrcHeight=1, aSX=0, aSY=0, aSWidth=1, 
    aSHeight=1, aDX=2, aDY=406, aDWidth=1, aDHeight=5) at scale.c:134
#4  0x410344b4 in DrawScaledImageNN (display=0x80b5768, aDest=0x87dcff0, 
    aGC=0x88bb620, aSrc=0x8a2ec48, aSrcMask=0x89a1350, aSrcWidth=1, 
    aSrcHeight=1, aSX=0, aSY=0, aSWidth=1, aSHeight=1, aDX=2, aDY=406, 
    aDWidth=1, aDHeight=5) at scale.c:174
#5  0x41041a9b in nsImageGTK::DrawScaled (this=0x88e9820, aContext=@0x89a09f0, 
    aSurface=0x8280518, aSX=0, aSY=0, aSWidth=1, aSHeight=1, aDX=2, aDY=406, 
    aDWidth=1, aDHeight=5) at nsImageGTK.cpp:499
#6  0x41041b76 in nsImageGTK::Draw (this=0x88e9820, aContext=@0x89a09f0, 
    aSurface=0x8280518, aSX=0, aSY=0, aSWidth=1, aSHeight=1, aDX=2, aDY=406, 
    aDWidth=1, aDHeight=5) at nsImageGTK.cpp:541
#7  0x40042859 in ?? ()
   from /builds/mcafee/cmonkey/mozilla/dist/bin/libgkgfx.so
#8  0x41b5edc7 in ?? ()
   from /builds/mcafee/cmonkey/mozilla/dist/bin/components/libgklayout.so

Comment 5

[tor]

CCing syd, as he wrote the code where it's crashing.

Does this help?

Index: scale.c
===================================================================
RCS file: /cvsroot/mozilla/gfx/src/gtk/scale.c,v
retrieving revision 1.1
diff -u -r1.1 scale.c
--- scale.c     2001/04/24 05:46:46     1.1
+++ scale.c     2001/04/25 02:29:19
@@ -70,8 +70,8 @@
     for (i = 0; i < newHeight; i++) {
       ysrc = (PRInt16) (i * factorY);
       sptr = img->data + ysrc * rowsize;
-      memcpy(dptr, sptr, rowsize);
-      dptr += rowsize;
+      memcpy(dptr, sptr, PR_MIN(rowsize,newImg->bytes_per_line));
+      dptr += newImg->bytes_per_line;
     }
   } else {
     for (i = 0; i < newWidth; i++) {

Comment 6

[tor]

Attachment: better patch to add bulletproofing and alloc the right amount of imgdata

Comment 7

[Chris McAfee]

patch still crashes, rh62.

Comment 8

[tor]

Which patch did you try?  Stack still the same?

Comment 9

[Chris McAfee]

I tried the first patch.  trying the 2nd now.

Comment 10

[Chris McAfee]

2nd patch, e.g. the first attachment, seems to work!
Can we check this in?  r=mcafee.

Comment 11

[Stuart Parmenter]

looks fine to me too.  need an sr=

Comment 12

[Mike Shaver (:shaver emeritus)]

+  if (!newImg) {
+    return newImg;
+  }

You know that newImg is null there, so it'd be clearer to just return null
explicitly:

  if (!newImg) {
    return NULL;
  }

+  if (!data) {
+    XDestroyImage(newImg);
+    return (XImage *) NULL;
+  }

Why do you need the cast?  If NULL is defined to |(void *)0|, then C lets you
just return it. (And if you're in C++, you should use NS_REINTERPRET_CAST, or
just return |0|.)

+      memcpy(dptr, sptr, PR_MIN(rowsize,newImg->bytes_per_line));

The arguments to PR_MIN want a bit of personal space, I think.

Fix those nitty bits, and sr=shaver.

Comment 13

[Asa Dotzler [:asa]]

fixing milestone (please excuse the trespass)

Comment 14

[tor]

Attachment: patch with shaver's nits fixed (plus including string.h for memcpy)

Comment 15

[Asa Dotzler [:asa]]

great! a= [e-mail] for checkin to 0.9

Comment 16

[tor]

Checked in.

Comment 17

[Brendan Eich [:brendan]]

Sorry I rubberstamped the scale.c code -- super-reviewer burnout, what can I
say?  Thanks to tor for doing the job right, after crash evidence pointed out a
problem.

/be

Comment 18

[tor]

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

Comment 19

[tor]

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

Comment 20

[Tracy Walker [:tracy]]

verified fixed on linix commercial build 2001-04-26-05-trunk

Comment 21

[Jay Patel [:jay]]

adding topcrash keyword and Trunk [@ libc.so.6 - DoScale - DrawScaledImageNN] to 
summary for tracking, since this is/was a topcrasher with recent Trunk builds.