[Bug 29635] New: Starcraft 2 Read Access Violation Regression possibly related to wine_cp_mbstowc
http://bugs.winehq.org/show_bug.cgi?id=29635
Bug #: 29635 Summary: Starcraft 2 Read Access Violation Regression possibly related to wine_cp_mbstowc Product: Wine Version: 1.3.36 Platform: x86-64 OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown AssignedTo: wine-bugs@winehq.org ReportedBy: cobaltjacket@gmail.com Classification: Unclassified
Steps by which I can reproduce: 1. Create a clean wine prefix. 2. Link (or move) a functional Starcraft 2 directory into "C:\Program Files" 3. Run Starcraft 2. Note: This will seem to work correctly, it loads and even can get into a game. I believe that it may crash immediately afterward. That crash may be unrelated, so I don't want to deal with it right now. 4. Close Starcraft. 5. Run it again. 6. Crash before load.
The Starcraft Error Reporter says:
ACCESS_VIOLATION (0xC0000005) occurred at 0023:F7620108. The memory at '0x04F9100F' could not be read.
Later on in that same window "wine_cp_mbstowc" is mentioned (under "Manual Stack Trace"):
Manual Stack Trace (ID:72 Stack:0x00000000 + 0)
F7642108 wine_cp_mbstowcs+632
I can provide the full text of this if you would like.
This behavior definitely does NOT occur in 1.3.29 (which for other reasons is the last version I was using that worked). I do not believe, however, that this behavior was present in 1.3.35. Cursory testing discovered this problem in 1.3.36, though I performed these more careful steps with 1.3.37. For this reason, I believe the first version with this regression is 1.3.36.
Attached is a WINEDEBUG=+relay,+seh,+tid log of the crash behavior. I have replaced my name in the log file by "myname".
A note: downgrading to 1.3.29, the game runs again. By this, I mean:
7. Downgrade to 1.3.29 8. The game runs perfectly. (There is no "reinstall" step: the same exact wine prefix and game directory are used.)
Repeating the upgrade to 1.3.37 causes the behavior to return. For this reason, I don't think that the game directory is somehow damaged.
http://bugs.winehq.org/show_bug.cgi?id=29635
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression Summary|Starcraft 2 Read Access |Starcraft 2: Read Access |Violation Regression |Violation |possibly related to | |wine_cp_mbstowc |
--- Comment #1 from Austin English austinenglish@gmail.com 2012-01-17 12:04:45 CST --- Please run a regression test: http://wiki.winehq.org/RegressionTesting
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #2 from cobaltjacket@gmail.com 2012-01-17 12:07:50 CST --- Created attachment 38399 --> http://bugs.winehq.org/attachment.cgi?id=38399 WINEDEBUG=+relay,+seh,+tid
gzip-ed log file
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #3 from cobaltjacket@gmail.com 2012-01-17 20:14:53 CST --- Regression Test:
768300c8aa2f977d14910610b8646582f9b376c9 is the first bad commit commit 768300c8aa2f977d14910610b8646582f9b376c9 Author: Hans Leidekker hans@codeweavers.com Date: Fri Nov 18 10:36:04 2011 +0100
winhttp: Implement WinHttpGetProxyForUrl.
:040000 040000 02aeb826b2b0c9adf7c810cd2fb41c4e6135d0ab a05449fc827f7944e6f58838f1b353748360a4b1 M dlls :040000 040000 76367a7aa6cf66954b9693342aa86ed565e736a9 4a06c92dc1752656e43963edeb0fe846d806e375 M include
======
I'm a little concerned that there might be more than one bug causing the symptoms. Some of the crashes occur on the first load, and other occur after the first load. On the other hand, it could be random. If there was an issue with this commit, but you know that it was fixed, could you please direct me to that correcting commit? I will see if the issue was resolved in that commit (and then bisect from there).
http://bugs.winehq.org/show_bug.cgi?id=29635
Dmitry Timoshkov dmitry@baikal.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hans@meelstraat.net Regression SHA1| |768300c8aa2f977d14910610b86 | |46582f9b376c9
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #4 from Hans Leidekker hans@meelstraat.net 2012-01-18 02:21:25 CST --- (In reply to comment #3)
Regression Test:
768300c8aa2f977d14910610b8646582f9b376c9 is the first bad commit commit 768300c8aa2f977d14910610b8646582f9b376c9 Author: Hans Leidekker hans@codeweavers.com Date: Fri Nov 18 10:36:04 2011 +0100
winhttp: Implement WinHttpGetProxyForUrl.:040000 040000 02aeb826b2b0c9adf7c810cd2fb41c4e6135d0ab a05449fc827f7944e6f58838f1b353748360a4b1 M dlls :040000 040000 76367a7aa6cf66954b9693342aa86ed565e736a9 4a06c92dc1752656e43963edeb0fe846d806e375 M include
======
I'm a little concerned that there might be more than one bug causing the symptoms. Some of the crashes occur on the first load, and other occur after the first load. On the other hand, it could be random. If there was an issue with this commit, but you know that it was fixed, could you please direct me to that correcting commit? I will see if the issue was resolved in that commit (and then bisect from there).
Does the crash still occur if you revert this commit?
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #5 from cobaltjacket@gmail.com 2012-01-19 02:06:57 CST --- I have almost no experience with git. I did
% git checkout master % git revert 768300c8aa2f977d14910610b8646582f9b376c9 error: could not revert 768300c... winhttp: Implement WinHttpGetProxyForUrl. hint: after resolving the conflicts, mark the corrected paths hint: with 'git add <paths>' or 'git rm <paths>' hint: and commit the result with 'git commit' % cat .git/MERGE_MSG Revert "winhttp: Implement WinHttpGetProxyForUrl."
This reverts commit 768300c8aa2f977d14910610b8646582f9b376c9.
Conflicts:
dlls/winhttp/session.c
(unsurprisingly, since this is such an old and large commit)
I found lines like "<<<<<< head" and "====" and ">>>>> parent ..." . If there is a really nice tutorial that covers this bit of detail about git, I'd appreciate a reference.
I would like to confirm that 1. These three kinds of lines are the only kinds of lines that are added to files. 2. Between "<<<< head" and "===" is what is at HEAD. 3. Between "====" and ">>>> parent of ..." is what was at the parent (immediate predecessor?) of the reverted commit.
If these are the case, then I think it was appropriate (for testing) to just erase all of these lines. That said, I did this and compiled. The error was resolved. The code added in this commit is causing the crash.
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #6 from Hans Leidekker hans@meelstraat.net 2012-01-19 02:22:27 CST --- Please attach a WINEDEBUG=+winhttp,+jscript trace.
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #7 from cobaltjacket@gmail.com 2012-01-19 09:28:36 CST --- Created attachment 38447 --> http://bugs.winehq.org/attachment.cgi?id=38447 WINEDEBUG=+winhttp,+jscript
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #8 from cobaltjacket@gmail.com 2012-01-19 10:26:56 CST --- git diff diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index b4ed568..30844af 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -1949,7 +1949,7 @@ static BSTR download_script( HINTERNET ses, const WCHAR *url ) } len = MultiByteToWideChar( CP_ACP, 0, buffer, offset, NULL, 0 ); if (!(script = SysAllocStringLen( NULL, len ))) goto done; - MultiByteToWideChar( CP_ACP, 0, buffer, offset, script, len ); + MultiByteToWideChar( CP_ACP, 0, buffer, offset, script, 0 ); script[len] = 0;
done:
This change causes the crash to go away. It is obviously NOT a resolution to the problem (I think setting dstlen=0 in wine_cp_mbstowcs might causes the call to just return the length of the string (mbtowc.c:277) and not copy it to *dst, which seems to be the purpose of this line). Hopefully this helps someone who knows more about the mbs/wcs implementation get a start.
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #9 from Hans Leidekker hans@meelstraat.net 2012-01-19 11:34:33 CST --- Created attachment 38450 --> http://bugs.winehq.org/attachment.cgi?id=38450 patch
Does this help? If not, please generate another +winhttp,+jscript trace with the patch applied.
http://bugs.winehq.org/show_bug.cgi?id=29635
--- Comment #10 from cobaltjacket@gmail.com 2012-01-19 16:39:47 CST --- Yep, that works!
Thanks.
http://bugs.winehq.org/show_bug.cgi?id=29635
Hans Leidekker hans@meelstraat.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |f37b94e69a92ae1f5c984f51f7d | |f6110831ab083 Status|UNCONFIRMED |RESOLVED Component|-unknown |winhttp Resolution| |FIXED
--- Comment #11 from Hans Leidekker hans@meelstraat.net 2012-01-20 13:56:13 CST --- Fixed.
http://bugs.winehq.org/show_bug.cgi?id=29635
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #12 from Alexandre Julliard julliard@winehq.org 2012-01-27 14:18:04 CST --- Closing bugs fixed in 1.4-rc1.
-
wine-bugs＠winehq.org