[Bug 14868] New: Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Summary: Initial crash when running GSpot
           Product: Wine
           Version: 1.0.0
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=15418)
 --> (http://bugs.winehq.org/attachment.cgi?id=15418)
GSpot crash log (wine GSpot.exe 2> gspot_crash.bt.txt)

I wasn't able to run the GSpot media analysis tool, available from this site:

http://www.headbands.com/gspot/v26x/index.htm [v 2.70a]

There was a wine crash very soon after the program started, I've attached the
output to this report.

Allan Højgaard has mentioned a crash on the GSpot page of appdb:

http://appdb.winehq.org/objectManager.php?sClass=version&iId=6924

Not sure about the component, just picked kerne32 because it came up on the
first line of the backtrace.

Note: I'm using the Debian variant of wine (Version: 1.0.0-1).

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://www.headbands.com/gsp
                   |                            |ot/v26x/index.htm
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |download

--- Comment #1 from Vitaliy Margolen   2008-08-15 00:07:09 ---
Confirming with latest git.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Louis Lenders  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
           Keywords|                            |regression

--- Comment #2 from Louis Lenders   2008-08-15 01:53:25 ---
Could you do a regression test please as described here
:http://wiki.winehq.org/RegressionTesting?highlight=(regression)

The application runs fine for me in wine-0.9.58, but crashes in wine-1.0

so that's enough narrowed down to get going i hope

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #3 from Louis Lenders   2008-08-15 07:09:56 ---
Created an attachment (id=15426)
 --> (http://bugs.winehq.org/attachment.cgi?id=15426)
reversed patch makes app start fine for me

well, just did it already, but hit bisect bad instead of good accidentily.
However, this must be the patch :

http://www.winehq.org/pipermail/wine-cvs/2008-April/042654.html

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Louis Lenders  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #15426|0                           |1
        is obsolete|                            |

--- Comment #4 from Louis Lenders   2008-08-15 08:26:13 ---
(From update of attachment 15426)
Dang, did something wrong. Forget the the whole comment above. It must be a
different patch that broke it. Sorry for the noise

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Louis Lenders  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Louis Lenders   2008-08-15 08:31:54 ---
This is the patch that broke the app:
http://www.winehq.org/pipermail/wine-cvs/2008-April/042671.html

Added author of patch

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #6 from Louis Lenders   2008-08-15 09:00:57 ---
just for completeness, here's the minimal patch to get the app running again:

diff --git a/dlls/gdi32/dib.c b/dlls/gdi32/dib.c
index 440857d..3b66492 100644
--- a/dlls/gdi32/dib.c
+++ b/dlls/gdi32/dib.c
@@ -651,6 +651,7 @@ INT WINAPI GetDIBits(
                 info->bmiHeader.biBitCount = 32;
                 break;
             default:
+                info->bmiHeader.biCompression = BI_RGB;
                 info->bmiHeader.biBitCount = bmp->bitmap.bmBitsPixel;
                 break;
             }

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #7 from David Hall (gringer)   2008-08-15 20:29:31 ---
Created an attachment (id=15441)
 --> (http://bugs.winehq.org/attachment.cgi?id=15441)
bisect log for crash

I got the following patch come up when I did a bisect:

$ git bisect good
baceb8d07e0ae08a00aaaac39c15b14f872c9d97 is first bad commit
commit baceb8d07e0ae08a00aaaac39c15b14f872c9d97
Author: Alexandre Julliard 
Date:   Mon Apr 21 11:57:30 2008 +0200

gdi32: Return bitmask for 32-bit bitmaps too in GetDIBits.

:040000 040000 d6fcd429195e9e571009766fe8339707a1856dea
051e98e9137a8972e7a47d10a134adf5d2e6dcfa M dlls

[full log attached]

I tried to be as careful as possible, doing 'rm -rf ~/.wine' before each test,
and a full 'CC="ccache gcc" ./configure --verbose && make clean && make' after
changing the bisect.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #8 from David Hall (gringer)   2008-08-15 21:05:57 ---
Louis Lenders wrote:
> just for completeness, here's the minimal patch to get the app running again:

I'm not convinced that is a good idea, as the default case for compression is
covered by the line setting the compression further up in the code:

+            info->bmiHeader.biCompression = (bmp->bitmap.bmBitsPixel > 8) ?
BI_BITFIELDS : BI_RGB;

[from 'git show baceb8d07e0ae08a00aaaac39c15b14f872c9d97']

I view that line as saying, "if the bits per pixel are greater than 8, set the
compression to BI_BITFIELDS. Otherwise, set it to BI_RGB."

I view the the minimal patch you have as saying, "If the bits per pixel are not
15 or 24, set the compression to BI_RGB." That happens after that first line in
the git patch, and seems to somewhat negate the effect of the ternary operator.

As I'm not too familiar with how the DIB format works, I hesitate to suggest
another solution, but I think changing the line about the default compression
before that case statement would be a better idea.

Note that the specification on MSDN mentions that BI_BITFIELDS "... is valid
when used with 16- and 32-bpp bitmaps."

reference: BITMAPINFOHEADER [MSDN]
http://msdn.microsoft.com/en-us/library/ms532290(VS.85).aspx

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #9 from David Hall (gringer)   2008-08-15 22:26:25 ---
sorry, didn't notice your comment #5, but it's nice to know that we ended up
with the same patch.

I did a quick trace to work out why it was crashing, by adding a FIXME to show
the number of bytes per pixel:

diff --git a/dlls/gdi32/dib.c b/dlls/gdi32/dib.c
index 440857d..31e003a 100644
--- a/dlls/gdi32/dib.c
+++ b/dlls/gdi32/dib.c
@@ -641,6 +641,7 @@ INT WINAPI GetDIBits(
                 DIB_GetDIBImageBytes( bmp->bitmap.bmWidth,
                                       bmp->bitmap.bmHeight,
                                       bmp->bitmap.bmBitsPixel );
+            FIXME("Bits per pixel: %d\n", bmp->bitmap.bmBitsPixel);
             info->bmiHeader.biCompression = (bmp->bitmap.bmBitsPixel > 8) ?
BI_BITFIELDS : BI_RGB;
             switch(bmp->bitmap.bmBitsPixel)
             {

Running the program demonstrates that the bitmap is encoded as 32 bits per
pixel:

$ ~/install/git/wine-git/wine GSpot.exe
ALSA lib seq_hw.c:457:(snd_seq_hw_open) open /dev/snd/seq failed: No such file
or directory
fixme:bitmap:GetDIBits Bits per pixel: 32
wine: Unhandled page fault on read access to 0x0000ff00 at address 0xff00
(thread 0009), starting debugger...

Taking into account that your patch fixes this (I have confirmed this) means
that the compression expected is BI_BITFIELDS, while the actual compression is
BI_RGB (i.e. none / uncompressed).

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #10 from David Hall (gringer)   2008-08-15 23:19:19 ---
I have noticed a fixme scattered through ./dlls/gdi32/dib.c:

$ grep -n 'FIXME: BI_BITFIELDS not supported yet' ./dlls/gdi32/dib.c
869:                    /* FIXME: BI_BITFIELDS not supported yet */
876:                            /* FIXME: BI_BITFIELDS not supported yet */
937:                            /* FIXME: BI_BITFIELDS not supported yet */
988:                    /* FIXME: BI_BITFIELDS not supported yet */
997:                            /* FIXME: BI_BITFIELDS not supported yet */
1030:                            /* FIXME: BI_BITFIELDS not supported yet */

would it make sense just making sure the output is never compressed (which is
what your suggested patch does)?

an alternative patch:

diff --git a/dlls/gdi32/dib.c b/dlls/gdi32/dib.c
index 440857d..cd48703 100644
--- a/dlls/gdi32/dib.c
+++ b/dlls/gdi32/dib.c
@@ -641,7 +641,7 @@ INT WINAPI GetDIBits(
                 DIB_GetDIBImageBytes( bmp->bitmap.bmWidth,
                                       bmp->bitmap.bmHeight,
                                       bmp->bitmap.bmBitsPixel );
-            info->bmiHeader.biCompression = (bmp->bitmap.bmBitsPixel > 8) ?
BI_BITFIELDS : BI_RGB;
+            info->bmiHeader.biCompression = BI_RGB; /* FIXME: BI_BITFIELDS not
supported yet */
             switch(bmp->bitmap.bmBitsPixel)
             {
             case 15:

I'm not quite clear on this code, so the non-compression patches could still be
problematic. It's hard for me to tell if this is the compression of the bitmap
being copied *to*, or the compression of the bitmap being copied *from*.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

--- Comment #11 from Dmitry Timoshkov   2008-08-15 23:56:08 ---
The problem might be that the new code writes color masks when there is no
enough space for them, and corrupts the stack. Specifying BI_RGB avoids
that.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Forest  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Mark hughes  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from Mark hughes   2009-06-11 06:54:45 ---
Bug still exists - 1.1.23

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

NSLW  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
                   |                            |m

--- Comment #13 from NSLW   2009-11-01 02:28:31 ---
Still present in wine-1.1.32-260-gf222a16

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

NSLW  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|[e-mail] |
                   |m                           |

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Vincent Povirk  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|-unknown                    |gdi32

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

A Wine user  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #14 from A Wine user   2010-04-09 10:51:22 ---
> baceb8d07e0ae08a00aaaac39c15b14f872c9d97 is first bad commit

This commit has also caused Paint It! to crash when trying to use some special
effects. The minimal patch from comment #6 stops these crashes, so it's
probably the same problem.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #15 from Alexandre Julliard   2010-05-27 11:40:49 ---
Should be fixed by 7f19bda6ec10036f6202d7eeb64c3691082af847.

[Bug 14868] Initial crash when running GSpot

http://bugs.winehq.org/show_bug.cgi?id=14868

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #16 from Alexandre Julliard   2010-05-28 12:50:37 ---
Closing bugs fixed in 1.2-rc2.