[Bug 51788] New: windowscodecs:wmpformat test_decode() fails in the ar_MA locale
https://bugs.winehq.org/show_bug.cgi?id=51788 Bug ID: 51788 Summary: windowscodecs:wmpformat test_decode() fails in the ar_MA locale Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: windowscodecs Assignee: wine-bugs(a)winehq.org Reporter: fgouget(a)codeweavers.com Distribution: --- Created attachment 70682 --> https://bugs.winehq.org/attachment.cgi?id=70682 Dump the imagedata content for debugging windowscodecs:wmpformat test_decode() fails in the ar_MA.UTF-8 locale: wmpformat.c:149: Test failed: unexpected image data wmpformat.c:149: Test failed: unexpected image data wmpformat.c:149: Test failed: unexpected image data wmpformat.c:149: Test failed: unexpected image data https://test.winehq.org/data/patterns.html#windowscodecs:wmpformat Strangely this also happens in other locales like ar_AE.UTF-8, ar_EG.UTF-8, fa_IR.UTF-8 (Farsi) and ur_IN.UTF-8 (Urdu); but not in other right-to-left locales like he_IL.UTF-8 (Hebrew) or yi_US.UTF-8 (Yiddish). I added code to dump the imagedata content and in all failure cases I got the same value: wmpformat.c:154: imagedata=6db0fc006db0fc006db0fc006db0fc006db0fc6c instead of the expected wmpformat.c:154: imagedata=6db0fc006db0fc006db0fc006db0fc006db0fc00 So it's only the last byte that changes. Also, while these failures are easily reproducible on the TestBot VMs (debiant2 and my own), I cannot reproduce them on my Debian 11 development machine (I have the required locales). In any case a bisect shows that these failures were introduced by the following commit: commit 711ce415c01a5e36bde6bb147b5aa3cedc8b35ed Author: Jacek Caban <jacek(a)codeweavers.com> Date: Thu Sep 2 14:14:25 2021 +0200 gdi32: Store abort proc in DC_ATTR. Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> Signed-off-by: Huw Davies <huw(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> -- 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=51788 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression, source, | |testcase Regression SHA1| |711ce415c01a5e36bde6bb147b5 | |aa3cedc8b35ed -- 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=51788 --- Comment #1 from Esme Povirk <madewokherd(a)gmail.com> --- Is SetAbortProc even called? -- 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=51788 Esme Povirk <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd(a)gmail.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.
https://bugs.winehq.org/show_bug.cgi?id=51788 Jacek Caban <jacek(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|regression | Regression SHA1|711ce415c01a5e36bde6bb147b5 | |aa3cedc8b35ed | CC| |jacek(a)codeweavers.com --- Comment #2 from Jacek Caban <jacek(a)codeweavers.com> --- There was a commit related to mentioned change later: https://source.winehq.org/git/wine.git/commitdiff/fcb6ae29a6ee45a5a0e4e41dd8... But I don't see how this could affect the test. It sounds more likely that there is a memory corruption somewhere and it just changed something about memory layout. In that case, DC_ATTR was later changed to use NtAllocateVirtualMemory directly and win32u generally doesn't use system heap anymore, so the layout is even more different now. I can't reproduce the bug on Test Bot: https://testbot.winehq.org/JobDetails.pl?Key=104197&f101=wow64_ar_MA.report#... François, can you still reproduce it? I'm removing regression keyword, it doesn't seem like a real regression. -- 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=51788 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|windowscodecs:wmpformat |windowscodecs:wmpformat |test_decode() fails in the |test_decode() fails |ar_MA locale |randomly --- Comment #3 from François Gouget <fgouget(a)codeweavers.com> --- You are right: this failure does not happen anymore on the newtb-debiant2-win32-ar-MA test configuration. However I still see the same failure on my (Debian+KDE) test machine (fg-deb64) though it's pretty rare (happened on 2021-10-25 and 2021-11-30). It also happened once on newtb-debiant2-win32-ja-JP on 2021-08-09. I also cannot reproduce the failure on my machine in the ar_MA.UTF-8 locale when compiling Wine from one of the commit ids where it should be systematic (b99d7db835b8..aa629c4c7225). So I think we have two options: * Run the test in Valgrind in the hope that the output is usable and points to the source of the buffer overflow. * Mark the bug as "Works for me" and reopen it if/when the failures happen again, even if intermittently. -- 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=51788 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |WORKSFORME --- Comment #4 from François Gouget <fgouget(a)codeweavers.com> --- windowscodecs:wmpformat does not fail anymore so I am marking this bug as resolved. -- 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=51788 Esme Povirk <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|WORKSFORME |--- --- Comment #5 from Esme Povirk <madewokherd(a)gmail.com> --- This has started failing again and a bisect points to: 9d1beee6c753f1ad864fb11d16598f09c3f0f510 is the first bad commit commit 9d1beee6c753f1ad864fb11d16598f09c3f0f510 Author: Rémi Bernon <rbernon(a)codeweavers.com> Date: Thu May 19 11:30:40 2022 +0200 combase: Use CRT memory allocation functions. Also likely not the real cause of this. -- 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=51788 --- Comment #6 from Esme Povirk <madewokherd(a)gmail.com> --- I'm taking it as a hint that either combase allocations are involved (unlikely) or CRT allocations, which would suggest memory errors in jxrlib itself. -- 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=51788 --- Comment #7 from Esme Povirk <madewokherd(a)gmail.com> --- Never mind, changing the one heap allocation in wmp_decoder_copy_pixels to use HEAP_ZERO_MEMORY fixes it. Looking at jxrlib example code, that doesn't seem to be intended, but I really don't want to dive into that mess and find the real problem. Hopefully failing to write over parts of the user buffer in cases where a channel is 0 is the only memory error, and zeroing that will get us consistency. -- 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=51788 --- Comment #8 from Esme Povirk <madewokherd(a)gmail.com> --- MR created: https://gitlab.winehq.org/wine/wine/-/merge_requests/108 -- 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=51788 Esme Povirk <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution|--- |FIXED --- Comment #9 from Esme Povirk <madewokherd(a)gmail.com> --- Fix merged, failures no longer appearing on test.winehq.org. -- 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=51788 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |5aa05728a5fb998a3c9dd7aebc9 | |e3ab75acc7742 -- 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=51788 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #10 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 7.10. -- 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=51788 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |7.0.x -- 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=51788 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|7.0.x |--- --- Comment #11 from Michael Stefaniuc <mstefani(a)winehq.org> --- Removing the 7.0.x milestone from bug fixes included in 7.0.2. -- 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)
-
WineHQ Bugzilla