[Bug 22146] New: 64bit compatibility issue with Wine's MCI string command parser
http://bugs.winehq.org/show_bug.cgi?id=22146 Summary: 64bit compatibility issue with Wine's MCI string command parser Product: Wine Version: 1.0.0 Platform: x86-64 OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: winmm&mci AssignedTo: wine-bugs(a)winehq.org ReportedBy: hoehle(a)users.sourceforge.net Created an attachment (id=27005) --> (http://bugs.winehq.org/attachment.cgi?id=27005) work in progress MCI parser that knows about 64bit structures Hi, Wine's MCI string command parser is one reason why test.winehq.org shows failures with 64bit machines. I've created this issue so everybody can discuss and contribute patches until evolution leads to a correct one. The attached patch fixes part of what's currently in git, but it is still incomplete. Therefore I don't plan to submit it. I've run tests on WTB's 64bit machines that proove that MCI_STATUS_PARMS and MCI_GETDEVCAPS indeeed process differently sized dwReturn types on x64: DWORD_PTR and DWORD. That's what's missing from my patch. A solution that sounds ideal is to define something like MCI_INTEGER64 and change the resource files mciavi_res.rc and winmm_res.rc. Given that new type, the MCI string parser would compute the correct offset. I'd be very pleased if somebody could conform that MS does it this way and tell us the value of this constant. Another solution path is to special-case MCI_STATUS_PARMS in the parser, as it uses the only 64bit construct I knew until 5 minutes ago (beside dwCallback of course) that's not a LP(W)STR. HWND seems to be 32bit. 5 minutes later: What about HDC in MCI_DGV_UPDATE_PARMS? Wait, wtypes.h says void*! The MCI string command parser needs to handle this type as well: MCI_INTEGER as currently used in mciav_res.rc is not adequate on 64bit. In the meantime, I'll submit a patch that adds MCI_HDC and a few other missing defines I know to the includes. Regards, Jörg Höhle -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Jörg Höhle <hoehle(a)users.sourceforge.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |win64 -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #1 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-03-24 04:23:40 --- Dmitry Timoshkov tells me: "HWND is defined in windef.h ... a generic pointer." Perhaps I looked in some other place like wtypes.h and was mislead. Well, we have those options A) Define and use MCI_INTEGER64, and B) Special-case MCI_STATUS' integer return type C) ? My plan is to produce 2 patches along the B route. That's "good enough" and much better than what's currently in git (i.e. it might work :). It can and should be superseded by an "A route" patch as soon as somebody shows evidence that native knows about such a special return type, either from documentation or from a resource dump of a 64bit winmm.dll or mciav32.dll if that's legal. The second patch would handle MCI_HWND, MCI_HDC and MCI_HPAL types. Perhaps MS changed MCI_STATUS' return type to DWORD_PTR precisely because some status command may return handles, e.g. MCI_DGV_STATUS_HWND or MCI_DGV_STATUS_HPAL? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|1.0.0 |1.1.41 --- Comment #2 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2010-03-24 04:30:50 --- Obviously your Wine version is not 1.0.0 -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Jörg Höhle <hoehle(a)users.sourceforge.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27005|0 |1 is obsolete| | --- Comment #3 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-03-24 18:14:35 --- Created an attachment (id=27027) --> (http://bugs.winehq.org/attachment.cgi?id=27027) patch: use 64bit compatible structures with the MCI. It would be nice if somebody with x64-64 could provide feedback on the patch. What it does: - MCI_STATUS and MCI_GETDEVCAPS aka. "capability" ought to work in simple cases, e.g. not "status x palette handle", to demonstrate that (some) offsets are correctly computed. What it does not: - conversion of return values >FFFFFFFF (e.g. handles) - handle MCI_HWND etc. "window x handle xyz" "open parent xyz" etc. - Hence "window x text hello" is broken. BTW, did you know that the MCI string command parser is covered by a US patent from IBM? I couldn't believe my eyes. Some sentence therein is 1:1 what's in the MCI SDK. http://www.freepatentsonline.com/6397263.html -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #4 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-03-24 18:20:17 --- Created an attachment (id=27028) --> (http://bugs.winehq.org/attachment.cgi?id=27028) test case involving MCI_STATUS and MCI_GETDEVCAPS aka. capability These tests pass on WTB's 64 bit MS boxes. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Eric Pouech <eric.pouech(a)orange.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric.pouech(a)orange.fr --- Comment #5 from Eric Pouech <eric.pouech(a)orange.fr> 2010-03-28 03:54:43 --- Actually, we should check if the content of the MCI table in winmm.dll has been changed on 64bit platforms (ie if we get something like you suggest about 32 and 64 bit integers described in this table). -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Wylda <wylda(a)volny.cz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wylda(a)volny.cz --- Comment #6 from Wylda <wylda(a)volny.cz> 2010-03-28 04:17:32 --- (In reply to comment #0)
Hi,
Wine's MCI string command parser is one reason why test.winehq.org shows failures with 64bit machines.
Hi, actually my experience is a bit different. The only test which does not fail is 64bit with ie6. See: http://test.winehq.org/data/719d904e351fa8fd1c7d1ebfbbae809ef27aa47c/index_X...
It would be nice if somebody with x64-64 could provide feedback on the patch.
If you would melt that into wine test exe, i could give it a try on 64bit and give feedback. Don't know if this is you are looking for ;) I mean i could try that on WinXP x64 (but i could NOT on 64bit linux/wine). -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #7 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-03-29 03:41:22 --- Eric wrote:
check if the content of the MCI table in winmm.dll has been changed on 64bit platforms I'd be very pleased if somebody would supply that data. Because a new MCI_INTEGER64 type would make the parser more regular and simplify my patch.
Wylda wrote:
If you would melt that into wine test exe Go to WTB job 1156 before it gets deleted, you can download both patch and exe. https://winetestbot.geldorp.nl/JobDetails.pl?Key=1156
actually my experience is a bit different. Your results are crazy indeed. Perhaps MS auto-detects a 32bit driver and sends plain old DWORD-sized objects instead?!? That also hints in the direction of additional MCI_INTEGER64 types. Austin English machine's shows exactly the crash that one expects from a 32/64bit pointer mismatch: http://test.winehq.org/data/f7cc8f695b954bf98bd34bb7adccc09378349a1a/wine_ae...
-- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #8 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-03-29 11:58:05 --- Wylda confused me. - Native machines pass the tests, validating the current mmsystem and digitalv.h structure definitions to some extent. Wylda's XP results are not surprising at all. - Wine64 machines crash, because winmm/mci.c is wrong for 64bit. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Jörg Höhle <hoehle(a)users.sourceforge.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27027|0 |1 is obsolete| | --- Comment #9 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-04-15 07:05:33 --- Created an attachment (id=27369) --> (http://bugs.winehq.org/attachment.cgi?id=27369) patch: For MCI parsing, use 64bit compatible structures. Here's the current version of the first patch. It could be simplified a little by what I put in git meanwhile. It's the first of three patches that should allow MCI to work on Wine64. 2. Fix offset calculation in MCI_CONSTANT (not yet written). 3. Introduce MCI_HWND etc. and other such DWORD_PTR sized objects (done). (not mentioning tests). The advantage of that approach is that a 4th one could be added anytime after we learn whether or not MS introduced new constants for 64bit objects in 64bit MCI drivers: there would be few things to change. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 Jerome Leclanche <adys.wh(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |adys.wh(a)gmail.com --- Comment #10 from Jerome Leclanche <adys.wh(a)gmail.com> 2010-06-30 01:34:11 --- Update on this bug? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #11 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-06-30 09:47:44 --- The state is unchanged since my comment #3 and comment #9: 64bit support is broken for apps that use MCI string commands. I somehow convinced myself that the 3-4 patches are not trivial and therefore not suitable for integration during code freeze, hence I invested my time somewhere else meanwhile (testing apps rather than writing patches). Am I wrong because 64bit support is the one goal for 1.2? André Hentschel was so kind to validate on a 64bit system the patches that I wrote so far, but they are still incomplete: - nr.2 from comment #9 still not written - nr.3 still contains one #ifdef WIN64 TODO - nr.4 missing extensive tests that specifically target the structures that matter (e.g. those involving DWORD_PTR: MCI_HWND, MCI_HDC etc.) & validate nothing was broken by the rewrite. So far I only wrote tests for MCI_STATUS (and MCI_GETDEVCAPS or MCI_INFO IIRC). MCI_HWND, MCI_HDC and MCI_HPAL mostly occur with the mciavi and mciquartz modules, for which there are currently no tests at all. Such tests could be written by a volunteer independently on the 64bit state. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #12 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-08-26 14:44:16 --- Created an attachment (id=30414) --> (http://bugs.winehq.org/attachment.cgi?id=30414) Use MCI_HWND & MCI_HDC in resource files This patch introduces MCI_HWND, MCI_HDC and MCI_PALETTE in the resource files. The MCI parser still need be fixed to handle them. In particular, the parser performs incorrect offset calculations around the MCI_HWND element in the L"handle\0", 0x00010000L, MCI_CONSTANT, L"default\0", 0x00000000L, MCI_HWND, L"\0", 0x00000000L, MCI_END_CONSTANT, construct in MCI_DGV_WINDOWS_PARMS. Obviously, the contents of the CONSTANT block must be scanned to determine the offset of the union. Don't try "window x text foobar" prior to that! I'd be glad if Octavian would jump in. I don't find time for a rewrite but I've written many parser (unpublished) tests that show differences between native and Wine. My conclusion is that native's parser cannot be implemented as the simple loop across the resources that Wine uses. Yet I believe Wine's loop is enough for what apps require. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #13 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2010-10-02 02:25:26 CDT --- W.r.t. patch numbering in comment #9, Alexandre reworked patch 1 for 1.3.4. test.winehq shows that it's enough to make current MCI tests pass. Good news & thanks, Alexandre. patch 2 (correct offset computations) is still not written - any volunteer? patch 3 is attached to comment #12 (however it predates the time when Maarten copied mciavi_res.rc into mciqtz) As I wrote to Octavian Voicu, a good testcase for patch 2 is open foo.avi alias a window a text "hello world" info a window text => "hello world" (oops, MCIAVI_mciInfo does not yet implement MCI_DGV_INFO_TEXT) Writing a reasonable set of mciavi tests is still in my queue, but don't hold your breath. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #14 from Jerome Leclanche <adys.wh(a)gmail.com> 2011-02-09 22:52:16 CST --- (In reply to comment #13)
W.r.t. patch numbering in comment #9, Alexandre reworked patch 1 for 1.3.4. test.winehq shows that it's enough to make current MCI tests pass. Good news & thanks, Alexandre. patch 2 (correct offset computations) is still not written - any volunteer? patch 3 is attached to comment #12 (however it predates the time when Maarten copied mciavi_res.rc into mciqtz)
As I wrote to Octavian Voicu, a good testcase for patch 2 is open foo.avi alias a window a text "hello world" info a window text => "hello world" (oops, MCIAVI_mciInfo does not yet implement MCI_DGV_INFO_TEXT)
Writing a reasonable set of mciavi tests is still in my queue, but don't hold your breath.
Saw some patches going in during October. Status on this bug? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #15 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2011-02-10 04:28:53 CST --- Comment #13 still applies. Anybody is welcome to rewrite the part of the parser that incorrectly computes the offsets, aka. MCI_ParseOptArgs. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 André H. <nerv(a)dawncrow.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nerv(a)dawncrow.de -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 joaopa <jeremielapuree(a)yahoo.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremielapuree(a)yahoo.fr --- Comment #16 from joaopa <jeremielapuree(a)yahoo.fr> 2012-11-03 14:34:00 CDT --- Still a bug in current wine? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=22146 --- Comment #17 from Jörg Höhle <hoehle(a)users.sourceforge.net> 2012-11-05 04:38:45 CST --- Git history shows that nothing changed: http://source.winehq.org/git/wine.git/history/HEAD:/dlls/winmm/mci.c Anybody - preferably a user of a 64bit system - is invited to try and fix 64bit MCI parser support, see comment #13. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=22146 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, testcase -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=22146 super_man(a)post.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |super_man(a)post.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org