[Bug 40297] New: Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Bug ID: 40297
           Summary: Multiple app crashes in macdrv_EnumDisplayMonitors
           Product: Wine
           Version: 1.9.5
          Hardware: x86
                OS: Mac OS X
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
          Assignee: [e-mail]
          Reporter: [e-mail]

Created attachment 53935
  --> https://bugs.winehq.org/attachment.cgi?id=53935
Deathsmiles crash backtrace

The steam games Mushihimesama and Deathsmiles both crash on start with the
attached logs and backtrace (attachments are for Deathsmiles). This games are
both by the same developer so they are likely tickling the same bug.

I've done some additional testing and this is what I've found:

It crashes with all the pre-packaged version of wine for OSX, going back to
1.9.0
It crashes if I use brew to install wine (brew install wine --devel)
It crashes if I compile my own wine from a clean source tree; using the latest
XCode under OSX 10.11.3 (SDK 10.11)

As a final test I cobbled together a build environment that's somewhat close to
the WineSkin engine build environment. This uses gcc-4.2 and the OSX 10.6 SDK.

When compiled this way it DOES NOT crash, and everything works great.

I also took this build environment and used clang instead of gcc-4.2 (so the
SDK was still 10.6); the resulting wine CRASHES in the same way as the others.

It seems clang may be the issue, or at least some factor in allowing other
issues to cause a crash?

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #1 from Zakk  ---
Created attachment 53936
  --> https://bugs.winehq.org/attachment.cgi?id=53936
Deathsmiles console output

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

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

--- Comment #2 from Ken Thomases  ---
Created attachment 53937
  --> https://bugs.winehq.org/attachment.cgi?id=53937
Use an assembly wrapper to call EnumDisplayMonitors callback

I suspect (with very little supporting evidence) that the issue is that the
games are passing a non-stdcall callback function to EnumDisplayMonitors(). 
That results in the stack being messed up after it returns.

It's similar to the issue fixed by
https://source.winehq.org/git/wine.git/?a=commit;h=b08c400fe2924bf7137e350c3673b11d3a6378c2
and the fix will probably be similar.  I've attached a patch based on that. 
Give it a try.

Another thing to try: test with the X11 driver.  That would presumably require
a similar patch.  Set the following registry setting:

[HKEY_CURRENT_USER\Software\Wine\Drivers]
"Graphics"="x11,mac"

You will also need to make sure that Wine will find the X11 libraries at load
time.  That may entail setting the DYLD_FALLBACK_LIBRARY_PATH environment
variable which is complicated by El Capitan's System Integrity Protection
(SIP).  Ask if you encounter trouble.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #3 from Zakk  ---
Applied the patch, both games launch and work now.

I tried with the X11 driver and they launch and work fine.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

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

--- Comment #4 from Ken Thomases  ---
Submitted: http://source.winehq.org/patches/data/120243

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #5 from Dmitry Timoshkov  ---
(In reply to Ken Thomases from comment #4)
> Submitted: http://source.winehq.org/patches/data/120243

This version mis-aligns the stack.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #6 from Ken Thomases  ---
(In reply to Dmitry Timoshkov from comment #5)
> (In reply to Ken Thomases from comment #4)
> > Submitted: http://source.winehq.org/patches/data/120243
>
> This version mis-aligns the stack.

Yeah, you're right.  I'll work up a new patch tomorrow.  Thanks.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #7 from Dmitry Timoshkov  ---
(In reply to Ken Thomases from comment #6)
> > > Submitted: http://source.winehq.org/patches/data/120243
> >
> > This version mis-aligns the stack.
>
> Yeah, you're right.  I'll work up a new patch tomorrow.  Thanks.

wine-staging has a similar patch for winex11, it aligns the stack properly:
https://github.com/wine-compholio/wine-staging/blob/master/patches/winex11-M...

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #8 from Ken Thomases  ---
(In reply to Dmitry Timoshkov from comment #7)
> wine-staging has a similar patch for winex11, it aligns the stack properly:
> https://github.com/wine-compholio/wine-staging/blob/master/patches/winex11-
> MONITORENUMPROC/0001-winex11.drv-Use-assembler-wrapper-to-call-MONITORENU.
> patch

Thanks again.  I guess I'll do that %ecx thing, too.  Do you know why it hasn't
been submitted upstream?

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #9 from Dmitry Timoshkov  ---
(In reply to Ken Thomases from comment #8)
> > wine-staging has a similar patch for winex11, it aligns the stack properly:
> > https://github.com/wine-compholio/wine-staging/blob/master/patches/winex11-
> > MONITORENUMPROC/0001-winex11.drv-Use-assembler-wrapper-to-call-MONITORENU.
> > patch
>
> Thanks again.  I guess I'll do that %ecx thing, too.  Do you know why it
> hasn't been submitted upstream?

I'd guess because of not adequate reaction and/or unwillingness of winehq
maintainer to deal with things like
https://www.winehq.org/pipermail/wine-devel/2016-March/112130.html

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

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

--- Comment #10 from Ken Thomases  ---
Created attachment 53962
  --> https://bugs.winehq.org/attachment.cgi?id=53962
Use an assembly wrapper to call EnumDisplayMonitors callback

Please test this new patch.  It fixes the stack misalignment issue.  Also, it
is implemented in user32 rather than the Mac driver so that it fixes the issue
regardless of driver.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #11 from Zakk  ---
Just compiled and tested with new patch. Looks good, crash is resolved.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

--- Comment #12 from Ken Thomases  ---
(In reply to Zakk from comment #11)
> Just compiled and tested with new patch. Looks good, crash is resolved.

Thanks!  I've submitted it.  https://source.winehq.org/patches/data/120558

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
      Fixed by SHA1|                            |474196c746e95cf84137a85512d
                   |                            |01a0e622c5c15

--- Comment #13 from Ken Thomases  ---
The fix has been accepted.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Alexandre Julliard  changed:

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

--- Comment #14 from Alexandre Julliard  ---
Closing bugs fixed in 1.9.7.

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
   Target Milestone|---                         |1.8.x

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Ken Thomases  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugs.winehq.org/sho
                   |                            |w_bug.cgi?id=24421

[Bug 40297] Multiple app crashes in macdrv_EnumDisplayMonitors

https://bugs.winehq.org/show_bug.cgi?id=40297

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|1.8.x                       |---

--- Comment #15 from Michael Stefaniuc  ---
Removing 1.8.x milestone from bugs included in 1.8.3.