https://bugs.winehq.org/show_bug.cgi?id=57011
Bug ID: 57011 Summary: GameStudio 3D program freezes on fflush since wine-9.5 Product: Wine Version: 9.13 Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: dark.shadow4@web.de Distribution: ---
Created attachment 76855 --> https://bugs.winehq.org/attachment.cgi?id=76855 Test script for 3D Gamestudio
Originally reported at https://forum.winehq.org/viewtopic.php?p=144395
This is a regression between wine-9.4 and wine-9.5.
The program can be downloaded from http://www.opserver.de/down/gstudio8_setup.exe Install the free version in a 32bit WINEPREFIX. Run SED.exe. Click Project->New Project. Enter a name, save it somewhere and press no to the next question. Click File->New, enter the test code from the attachment and save it somewhere. Press the run button in the script editor.
The newly opened game freezes and the terminal spams lines like
0284:err:sync:RtlpWaitForCriticalSection section 0031FE30 #0001 wait timed out in thread 0284, blocked by 2620054, retrying (60 sec)
For further tests you can just open the save script.
https://bugs.winehq.org/show_bug.cgi?id=57011
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |http://www.opserver.de/down | |/gstudio8_setup.exe Keywords| |download, regression
https://bugs.winehq.org/show_bug.cgi?id=57011
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Regression SHA1| |2745228b14d138ea2c7f631c212 | |440ecf7b8f453 CC| |pgofman@codeweavers.com
--- Comment #1 from Fabian Maurer dark.shadow4@web.de --- Bisected to
commit 2745228b14d138ea2c7f631c212440ecf7b8f453 (HEAD) Author: Paul Gofman pgofman@codeweavers.com Date: Fri Mar 22 13:58:11 2024 -0600
ntdll: Don't use debug info presence to detect critical section global status.
Reverting fixes the issue.
@Paul Gofman, would you mind taking a look?
https://bugs.winehq.org/show_bug.cgi?id=57011
--- Comment #2 from Paul Gofman pgofman@codeweavers.com --- I've debugged this and it looks like the attached script bug (not even the app's one) and this script executes without errors on Windows by chance only.
fhandle_n = file_open_write ("bubu.txt"); xhandle_n = fhandle_n; diag_var("\nfhandle_n: %6.3f",fhandle_n);
file_str_write(fhandle_n, "bubu\n\r"); diag("\ncomes"); diag_var("\nfhandle_n: %6.3f",fhandle_n); fflush(xhandle_n); // stucks here..
Here the engine executing the script calls msvcrt.fflush(pointer). msvcrt.fflush() assumes it is FILE structure but it is not. It is some internal engine's structure returned by file_open_write(), no msvcrt / else MS CRT functions are called to execute those other engine's function, the engine works with kernel32.CreateFile() etc. directly.
msvcrt has the logic to lock with EnterCriticalSection. If FILE structure was created by msvcrt itself it finds the critical section in the own data, if it was not then it assumes CRITICAL_SECTION follows FILE structure (that matches Windows behaviour). That data (along with the other FILE fields) fflush() receives are nonsensual. It is only a matter of random that it doesn't crash in msvcrt trying to do actual fflush (probably not entirely on random, a bit more on that below). But the random data for "critical section" are so that after the blamed commit our implementation decides first that it should wait (depends on CRITICAL_SECTION.LockCount) and then that the WaitSemaphore is semaphore which yields observed behaviour. I tested that on Windows under similar conditions EnterCriticalSection doesn't lockup but throws an exception. Doing so in Wine (I sent the patch for that upstream: https://gitlab.winehq.org/wine/wine/-/merge_requests/6175) results in the message box pop up when trying to execute this script.
On Windows there is no such message box, this script in its exact form executes silently. But it is only a matter of semi-random data on stack (the address passed to fflush points to stack and apparently contains some unrelated other data). This random data happens to be a bit different on Windows with LockCount such as it doesn't tries to wait on this "would be" critical section, only updates critical section's field (probably corrupting unrelated data on stack).
Note that in this repro script fhandle_n is copied to xhandle_n before file_str_write and fflush. Changing that to a straightforward way (just fflush(fhandle_n)) causes access violation in this fflush() both on Windows and Wine and results in a message box complaining about invalid access. That's probably why the script author "worked around" this issue by copying that to another object, but fflush() still doesn't work and just doesn't do anything as best, it gets not a msvcrt's FILE strcuture. So I think the current behaviour with my MR is better than on Windows in a sense that it doesn't result in a silent successful execution of a broken script. But that can change anytime of course because depends on something unrelated in stack data.
So this is a script bug and the app's script engine just doesn't perform any sort of validation on what passed to native (msvcrt in this case) functions, so what happens if the script does that is random. I sent the MR upstream (https://gitlab.winehq.org/wine/wine/-/merge_requests/6175) because that looks like a correct change to me and makes EnterCriticalSection sort of more robust and behaving like on Windows. But broadly it seems to me there is nothing to fix here and this change also doesn't guarantee that such things work anyhow predictable on such script bugs (that's basically the same on Windows, even if in the present case the semi-random app's on-stack data are a bit different resulting in a different behaviour).
Unless there will be some considerations why this would be wrong, I'd probably suggest closing this bug as invalid regardless of whether my patch goes in or not.
https://bugs.winehq.org/show_bug.cgi?id=57011
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |INVALID Status|NEW |RESOLVED
--- Comment #3 from Fabian Maurer dark.shadow4@web.de --- Thanks for your time, that's an in-depth explanation! I agree that this is inherently broken and there's nothing we can reasonably do about it.
Marking INVALID.
https://bugs.winehq.org/show_bug.cgi?id=57011
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|GameStudio 3D program |GameStudio 3D program |freezes on fflush since |freezes on fflush since |wine-9.5 |wine-9.5 because it passes | |invalid data Keywords|regression |
https://bugs.winehq.org/show_bug.cgi?id=57011
--- Comment #4 from Paul Gofman pgofman@codeweavers.com --- The mentioned critical section change is upstream as bf7d403ad89b624c90784afb3ffd3e5ba5559ae5. So right now it will probably show message box attempting to execute the sample script instead of hanging. But again, going forward there is no warranty that the script will work the same way. If some random change which will alter unrelaed stack contents will result in some other value for critical section data where it won't decide to wait for that as semaphore but will use WaitOnAddress() instead then it will hang again.
https://bugs.winehq.org/show_bug.cgi?id=57011
--- Comment #5 from Austin English austinenglish@gmail.com --- Closing.
https://bugs.winehq.org/show_bug.cgi?id=57011
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #6 from Austin English austinenglish@gmail.com --- Actually closing.
-
WineHQ Bugzilla