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

Fabian Maurer [email protected] 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

--- Comment #2 from Paul Gofman [email protected] --- 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 [email protected] 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 #5 from Austin English [email protected] --- Closing.