Bug 260612

Summary: emulators/wine-devel: Fix multiple bugs including crash
Product: Ports & Packages Reporter: Ivan Rozhuk <rozhuk.im>
Component: Individual Port(s)Assignee: Damjan Jovanovic <damjan.jov>
Status: Closed FIXED    
Severity: Affects Some People CC: Alexander88207, damjan.jov, dmenelkir, gerald, iwtcex, rozhuk.im, se
Priority: --- Keywords: crash, needs-qa
Version: LatestFlags: bugzilla: maintainer-feedback? (damjan.jov)
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch rozhuk.im: maintainer-approval?

Description Ivan Rozhuk 2021-12-22 14:34:19 UTC
Created attachment 230316 [details]
patch

1. Only once unpack staging patch.
2. Fix plist: some files was marked as staging.
3. Add wine patch to proper stack size calculation

2+3 fix genshin impact crashes for me.
Comment 1 Ivan Rozhuk 2021-12-22 14:43:39 UTC
Created attachment 230317 [details]
patch
Comment 2 Alex S 2021-12-22 19:49:17 UTC
(In reply to Ivan Rozhuk from comment #0)

> 3. Add wine patch to proper stack size calculation

What's the chain of reasoning here?
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2021-12-22 21:30:20 UTC
^Triage: If quarterly is affected, please set merge-quarterly to ?
Comment 4 Ivan Rozhuk 2021-12-22 22:27:00 UTC
Created attachment 230324 [details]
patch

wine without stage patches:
1 mb - some times crash with stack overflow
2 mb - always crash on early start
4 mb - see no crash
Comment 5 Ivan Rozhuk 2021-12-22 22:36:26 UTC
(In reply to Alex S from comment #2)

genshin impact crash on start with "stack overflow" messages.

I found old patch for set stack size: https://wine-devel.winehq.narkive.com/3F4LSB42/stack-size-allocations
then found place at actual code where stack allocate and patch it.

May be this is not best solution, but it works for me and IMHO should not add new bugs.
I have plan to upstream this patch in next few days.

Now I have only sound issues with some games, but this is different issue that I do not debug yet.
Comment 6 Alex S 2021-12-23 01:36:28 UTC
(In reply to Ivan Rozhuk from comment #5)

Hmm... The adjustment probably doesn't hurt, but this is a bit weird. Why doesn't Linux Wine crash then?

(In reply to Ivan Rozhuk from comment #4)

> wine without stage patches:
> 1 mb - some times crash with stack overflow
> 2 mb - always crash on early start
> 4 mb - see no crash

2 MB worse than 1 MB? If you say so...
Comment 7 Ivan Rozhuk 2021-12-23 06:17:29 UTC
(In reply to Alex S from comment #6)

Not so many users try to run GI on wine.
Also FreeBSD and linux has some difference with memory allocation and how other apps (like xlib) builded.

Worse on wine without staging patches.


I do not insist to include this patch and can continue use it as private patch.
Comment 8 Stefan Eßer freebsd_committer freebsd_triage 2022-01-05 11:31:47 UTC
Gerald Pfeifer has asked me to test this patch and to commit it to wine-devel on success.
Comment 9 commit-hook freebsd_committer freebsd_triage 2022-01-13 07:33:45 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=e676927ad796e5935553ef417e16b14cc800c4a9

commit e676927ad796e5935553ef417e16b14cc800c4a9
Author:     Damjan Jovanovic <damjan.jov@gmail.com>
AuthorDate: 2022-01-13 07:29:20 +0000
Commit:     Gerald Pfeifer <gerald@FreeBSD.org>
CommitDate: 2022-01-13 07:32:15 +0000

    emulators/wine-devel: Update to 7.0-rc5

    The fifth milestone of the soon to be released Wine 7.0 with
    30 bug fixes.

    Also do not extract the wine-staging files twice, symlink them
    instead. [1]

    PR:             260612 [1]
    Submitted by:   Ivan Rozhuk <rozhuk.im@gmail.com> [1]
    Approved by:    maintainer (= author)

 emulators/wine-devel/Makefile |  6 ++++--
 emulators/wine-devel/distinfo | 10 +++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)
Comment 10 Gerald Pfeifer freebsd_committer freebsd_triage 2022-01-13 07:40:17 UTC
(In reply to Stefan Eßer from comment #8)
> Gerald Pfeifer has asked me to test this patch and to commit it to
> wine-devel on success.

Just the pkg-plist part (in that context) :-)

(In reply to Ivan Rozhuk from comment #5)
> May be this is not best solution, but it works for me and IMHO should
> not add new bugs. I have plan to upstream this patch in next few days.

Thank you for this report and the patches, Ivan.

As you can see, two of the three aspects have been committed now. The
third you mentioned you plan on upstreaming, so shall we mark this PR
as closed?
Comment 11 Ivan Rozhuk 2022-01-13 07:54:38 UTC
I do not see wine-devel/pkg-plist in commit, did it missed?

patch-dlls_ntdll_unix_virtual.c - will open new PR or report to upstream.
Comment 12 Gerald Pfeifer freebsd_committer freebsd_triage 2022-01-13 08:04:59 UTC
(In reply to Ivan Rozhuk from comment #11)
> I do not see wine-devel/pkg-plist in commit, did it missed?

se@ added it to the previous commit according to comment #8, just
didn't mention this PR in the commit message:

    commit ee7c59732f043936c4f56f7dd25ed5bc9d00954c
    Author: Damjan Jovanovic <damjan.jov@gmail.com>
    Date:   Fri Jan 7 22:59:24 2022 +0100

    emulators/wine-devel: update to 7.0-rc4
    
    The 4th milestone of the soon to be released Wine 7.0 with 38 bug
    fixes, and another 22 bug fixes from the (skipped) RC3 release.
    
    In addition to the patch provided by Damjan, part of the PLIST change
    in PR 260612 has been applied to install some files that used to be
    dependent on the STAGING option.

> patch-dlls_ntdll_unix_virtual.c - will open new PR or report to upstream.

I recommend pushing such things upstrea, both for expertise and longer
term maintainability.

If you want to keep it on the FreeBSD side, we can keep this PR open
which already has context and Alex' feedback.
Comment 13 Ivan Rozhuk 2022-01-13 08:21:59 UTC
Created attachment 230975 [details]
patch

I recheck my git and found few things from plist that has not commited yet.

Yes, please leave it open.
Comment 14 Gerald Pfeifer freebsd_committer freebsd_triage 2022-01-23 00:02:57 UTC
(In reply to Ivan Rozhuk from comment #13)
> I recheck my git and found few things from plist that has not commited yet.

As for those pkg-plist changes, Stefan did not miss those, rather in
his testing those were not actually necessary/available.

Are you building in a clean environment such as Poudriere or on a regular
system? If the latter, I'm guessing here's something installed there which
influence what is created.


Damjan, how about that other change, the one to the code?
Comment 15 Damjan Jovanovic 2022-01-23 03:30:09 UTC
(In reply to Ivan Rozhuk from comment #13)

I don't see any win32k.sys / win32k.sys.so in my 32 bit build.

It appears to only be added by wine-staging's
patches/winedevice-Default_Drivers/0001-win32k.sys-Add-stub-driver.patch
Comment 16 Damjan Jovanovic 2022-01-23 04:22:37 UTC
(In reply to Gerald Pfeifer from comment #14)

How on earth do you measure stack usage by library to begin with? I mean in theory you could put a breakpoint somewhere, and then sum up the stack frames whose subsequent return address is inside libX11's executable memory ranges, but how do you do that in practice?
Comment 17 Gerald Pfeifer freebsd_committer freebsd_triage 2022-01-28 07:38:28 UTC
Damjan, does that as maintainer you want to close this PR and not
pursue this remaining bit of the patch?

What next step(s) do you suggest)?
Comment 18 Damjan Jovanovic 2022-03-18 17:54:54 UTC
(In reply to Gerald Pfeifer from comment #17)

The double extraction of staging patches was fixed, the win32k.sys patch was wrong (comment 15), and the stack usage patch is getting upstreamed, so I think we're done here. Closing fixed.

Thank you for your contribution.