https://bugs.winehq.org/show_bug.cgi?id=57758
Bug ID: 57758 Summary: Starcraft Brood War regression on Wine 10 Product: Wine Version: 10.0 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: blocker Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: louis@qdcec.co.za Distribution: ---
Running on Fedora 40 using packages wine-10.0-1.fc40.x86_64 and wine-10.0-0.7rc4.fc40.x86_64 I am unable to successfully run a previously "platinum" running package: StarCraft / Brood Wars by Blizzard. If I downgrade to wine-9.1-1.fc40.x86_64 the application works once again.
I also have a very simple tool that decodes Cisco level 7 ascii passwords - useful when I forget what I assigned an account - that works on version 9 but not 10.
(I've tried to file this regression a few days ago, but cannot find it anywhere on "My Bugs" or anywhere on this site. If I have inadvertently created a duplicate, please forgive me.)
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #1 from Louis louis@qdcec.co.za --- Further detail:
When running wine-9.1-1.fc40.x86_64, StarCraft/Brood War runs perfectly.
Once wine upgraded to wine-10.0-1.fc40.x86_64 the application no longer can run.
It attempts to start and then hits a black screen, with a pop up stating: Program Error A program on your system has crashed, but WineDbg was unable to attach to the process to obtain a backtrace.
It tries to open the debugger, but this just shows: Wine Debugger Can't attach process 0020: error 5
When running from the terminal, it also shows this error:
0024:fixme:ntdll:NtQuerySystemInformation info_class SYSTEM_PERFORMANCE_INFORMATION wine: Unhandled page fault on read access to 00000000 at address 00000000 (thread 0024), starting debugger...
I'm running / calling the program with: wine "/home/louis/.wine/drive_c/Program Files/Starcraft/StarCraft.exe"
https://bugs.winehq.org/show_bug.cgi?id=57758
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|blocker |normal Keywords| |regression
--- Comment #2 from Austin English austinenglish@gmail.com --- Please run a regression test: https://gitlab.winehq.org/wine/wine/-/wikis/Regression-Testing
(In reply to Louis from comment #0)
I also have a very simple tool that decodes Cisco level 7 ascii passwords - useful when I forget what I assigned an account - that works on version 9 but not 10.
Please open a separate bug for that (a regression test would be helpful for that as well).
https://bugs.winehq.org/show_bug.cgi?id=57758
Alex Henrie alexhenrie24@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |ntdll CC| |alexhenrie24@gmail.com, | |michael@fds-team.de Ever confirmed|0 |1 Regression SHA1| |6303163af0c8e1c245e45fe2efe | |a8f664431d26c Status|UNCONFIRMED |NEW
--- Comment #3 from Alex Henrie alexhenrie24@gmail.com --- I can confirm that StarCraft no longer launches. `git bisect` says:
6303163af0c8e1c245e45fe2efea8f664431d26c is the first bad commit commit 6303163af0c8e1c245e45fe2efea8f664431d26c Author: Michael Müller michael@fds-team.de AuthorDate: Mon Apr 3 05:56:19 2017 +0200 Commit: Alexandre Julliard julliard@winehq.org CommitDate: Mon Nov 11 20:38:22 2024 +0100
ntdll: Use HashLinks when searching for a dll using the basename.
dlls/ntdll/loader.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
https://bugs.winehq.org/show_bug.cgi?id=57758
Zeb Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pgofman@codeweavers.com
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #4 from Paul Gofman pgofman@codeweavers.com --- How can I get / purchase this game? Doesn't look like StarCraft / Brood Wars exist in Battle.net, where can I get it? Or is it some game mode within some Startcraft version in Battle.net?
Could you please attach the compressed full output with WINEDEBUG=+loaddll,+pid,+timestamp,+seh,+unwind,+threadname,+module log from the failing run?
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #5 from Paul Gofman pgofman@codeweavers.com --- FWIW I tried Starcraft with Battle.net, Expansion version with the current Wine and the game launches for me with current Wine (after working around some Battle.net issues).
So to move this forward all the details from the previous comment are needed, including exact game version, how that was installed and run, Wine version and the log with those channels.
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #6 from Alex Henrie alexhenrie24@gmail.com --- Created attachment 78042 --> https://bugs.winehq.org/attachment.cgi?id=78042 WINEDEBUG=+loaddll,+pid,+timestamp,+seh,+unwind,+threadname,+module
Here are the logs you requested. You need actual StarCraft and StarCraft Brood War CDs from 1998 and the 1.16.1 patch from http://ftp.blizzard.com/pub/broodwar/patches/PC/BW-1161.exe
The crash does not happen on StarCraft 1.05 or 1.17.0.
https://bugs.winehq.org/show_bug.cgi?id=57758
Alex Henrie alexhenrie24@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|10.0 |9.22 Summary|Starcraft Brood War |Starcraft Brood War 1.16.1 |regression on Wine 10 |does not launch on Wine 10 URL| |http://ftp.blizzard.com/pub | |/broodwar/patches/PC/BW-116 | |1.exe
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #7 from Paul Gofman pgofman@codeweavers.com --- Thanks for the info! I studied the logs, and I don't see any clue how the commit can be related to the observed crash in log. The uses of find_basename_module() which was changed through the blamed commit are pretty limited and those cases should always leave traces in logs. I studied probably each one of them and each of those seem to provide expected result. If the previous related commit was blamed I could suspect that the game is doing something behind the scenes with introduced hash links which would obviously not reflected in logs but that can't explain the attribution to the blamed commit.
So one guess is maybe there is something regressed it on top and on current Wine it crashes earlier due to a different reason? So, the additional questions:
1. Does the game work if the blamed commit is reverted on top of current Wine? If it does, could you please attach the same logs with the commit reverted on top?
2. Can we add +system,+win,+x11drv,+event,+msg,+ddraw,+d3d,warn+heap logs to the current set for reproduction case (that would be WINEDEBUG=+loaddll,+pid,+timestamp,+seh,+unwind,+threadname,+module,+ddraw,+system,+win,+x11drv,+event,+msg,+ddraw,+d3d,warn+heap)? Note that warn+heap is not just a logging channel but enables heap debugging; if it changes something with reproduction details that is interesting info by itself but also worth running with that one excluded.
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #8 from Paul Gofman pgofman@codeweavers.com --- Created attachment 78047 --> https://bugs.winehq.org/attachment.cgi?id=78047 PoC patch (not a fix really)
So I managed to get the game and reproduced the problem.
Reverting blamed commit indeed fixes the crash, although the only effect of the patch is that it changes which bytes are modified on the stack, and those bytes are later used uninitialized by the game.
I am attaching the patch which clearly illustrates that and "fixes" the issue (but please don't treat that as an attempt to fix it, that is absolutely insane and I am attaching it purely for illustration purposes).
The actual crash in the generated code. The game generates some code specific to probably screen size or some other parameters on the fly and then it crashes jumping to NULL address. The generation of code blocks is performed based on some string translation encoding that, like "4 D=S", which generates instructions (which patterns later possibly combined). Such string is passed to function at address 0x150314B0 in storm.dll on stack (previously strcpy'ed from the game data). That function ends up parsing it beyond the end terminator, not entirely sure due to it expects it double NULL terminated or it is a bug in the function itself, but it parses it beyond zero terminator reading leftover unitialized stack data. The fatal error or success depends on what is there. It is tolerant to the wide range of garbage on stack, but if there happens to be 'S' before zero it generates the broken code pattern. I confirmed with debugger on Windows that:
- There is also garbage, so it is not like we are using more stack than Windows, just the bytes are different and on Windows (like in Wine previously) the problematic condition of 'S' before 0 is not met. - Setting these leftover stack data on Windows in debugger to satisfy the problematic condition yields the same crash.
So in my case in that stack garbage, when the game is installed at default location C:/Program Files/Starcraft , there is a part of this path here. Renaming Starcraft so it doesn't contain 'S' (e. g., to "Qtarcraft") works around the issue here. But that should not be necessarily the case with Wine build differently, e. g., with a different version of gcc; while such renaming also has good chances to help. Using a shorter path (placing the game in C:/q instead) doesn't help: then in the leftover stack data contains some string mapping data with characters 01-FF and then it still hits 'S' before 0.
So that so far looks not sanely fixable to me. It works by chance on Windows, worked by chance on Wine and maybe will work again eventually once something else in the functions on the way of GetModuleHandle() randomly changes.
If that was, e. g., about not touching that stack from Wine (using less of the stack in some functions) there maybe could be a valid fix, but this is not the case: we are not using much of stack on that path and there is also garbage on Windows, just without 'S'.
Maybe a workaround that has some chance of working is renaming game's Starcraft directory to anything with the same size without 'S'.
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #9 from Alex Henrie alexhenrie24@gmail.com --- Created attachment 78048 --> https://bugs.winehq.org/attachment.cgi?id=78048 Case sensitive hack
Interesting, thanks for the analysis!
The attached patch also fixes the crash, and it doesn't break the HashLinks tests in dlls/kernel32/tests/module.c. Are we certain that HashLinks is supposed to be case-insensitive?
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #10 from Alex Henrie alexhenrie24@gmail.com --- Looking at it more closely, I see that hash_table and hash_basename are entirely internal to Wine. The only way that case sensitivity might matter here is if the list or hash table has two references to the same module with different capitalizations and searches are supposed to prefer exact matches over case-insensitive matches. But I don't think it's actually possible for a module to appear in the list twice.
I have to agree with you that this is probably just an uninitialized memory bug in the game itself. It might have even been fixed properly between version 1.16.1 and version 1.17.0. The best we can do is to tell Wine users to install the 1.17.0 patch from http://ftp.blizzard.com/pub/broodwar/patches/PC/BW-1170.exe to play StarCraft.
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #11 from Paul Gofman pgofman@codeweavers.com --- Created attachment 78051 --> https://bugs.winehq.org/attachment.cgi?id=78051 ad-hoc test illustrating that case conversion during hashing is required
The attached patch also fixes the crash, and it doesn't break the HashLinks tests in dlls/kernel32/tests/module.c. Are we certain that HashLinks is supposed to be case-insensitive?
Basically, since the search for module is case insensitive I am afraid hashing has to be case-insensitive too, no way around. It is not immediately trivial to confirm in test that there is a difference because for the typical cases the difference in codes between capical and low case letter is 32. And so is HASH_MAP_SIZE (hash_basename returns hash % HASH_MAP_SIZE), so one character effectively affects hash as (*hash = *hash * 65599 + string->Buffer[i]) % 32, and if the two characters' codes differ in 32 the result is the same for them (that can be proved mathematically). So for the typical case (and always for ascii module names) there is no difference in result whether we convert case in RtlHashUnicodeString() or not. However, it is not always the case, e. g., Czech letters Ž and ž differ in 1. So the very ad-hoc and dirty attached test illustrates that case insensitive call to RtlHashUnicodeString() is correct; the test fails on Wine with your patch but succeeds as is and on Windows for me.
But this is not too much related to the present bug anyway. I hope my PoC patch attached to Comment #8 confirms beyond any doubt that the effect of blamed commit is not a wrong functional change but the effect of stack data modification (to be later used erroneously). What is harder to prove beyond any doubt is that, blamed commit aside, the difference in stack data and buggy game behaviour is not triggered by some other, unrelated Wine bug or fixable difference with Windows. However, I tried to build a confidence in that in a way explained in the rest of Comment #8.
So of course random changes on GetModuleHandleA can tweak the reproduction on and off depending on which bytes it ends up modifying on its local stack and how. But we don't have a control of that in C really, nor we can guarantee the exact stack leftover garbage matching Windows. If we were to workaround this game bug reliably the only possible way is something between the line of the patch in Comment #8 (with some cleanup, like sanitizing asm names, reducing extra stack allocation, removing check for return address etc.).
I don't think this bug can be closed, whenever something reliably works on Windows, even if that is a game bug and works by chance, we consider this a bug. But I doubt it can be fixed, while also may start working due to other random changes or switching compiler version (and then randomly stop working again...)
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #12 from Paul Gofman pgofman@codeweavers.com ---
What is harder to prove beyond any doubt is that, blamed commit aside, ...
Just to note, not like that proof was my goal, I was actually trying to find a way to reliably workaround this game problem with some sensible and otherwise useful change elsewhere which could go upstream, it is just that I so far don't see any.
https://bugs.winehq.org/show_bug.cgi?id=57758
--- Comment #13 from Louis louis@qdcec.co.za --- (In reply to Alex Henrie from comment #10)
It might have even been fixed properly between version 1.16.1 and version 1.17.0. The best we can do is to tell Wine users to install the 1.17.0 patch from http://ftp.blizzard.com/pub/broodwar/patches/PC/BW-1170.exe to play StarCraft.
I was indeed on patch 1.16 and updating to patch 1.17 has fixed this bug for me. I'm happy with that as a workaround solution. (Sadly Blizzard's FTP site is no longer browseable, so thank you for providing the URL to the patch, which still works).
-
WineHQ Bugzilla