[Bug 52187] New: wine-mono's WPF tests hang
https://bugs.winehq.org/show_bug.cgi?id=52187 Bug ID: 52187 Summary: wine-mono's WPF tests hang Product: Wine Version: 6.23 Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs(a)winehq.org Reporter: madewokherd(a)gmail.com Distribution: --- The test WineMono.Tests.System.Windows.Media.TextFormatting.TextFormatterTest:IndentTest hangs on current Wine. It seems it gets in a loop where dwritetextanalyzer_AnalyzeLineBreakpoints requests data past the end of the range provided by the source, and the source returns 0-length text. This is our implementation of GetTextAtPosition: https://github.com/madewokherd/wpf/blob/main/src/Microsoft.DotNet.Wpf/src/Di... Print statement debugging shows that the "else" path is being taken in that function, but on the dwrite side checking for a length of 0 works around the hang. Bisecting gave me this: 909f7aa7c21f82114586e98a0a9e940b13bc386a is the first bad commit commit 909f7aa7c21f82114586e98a0a9e940b13bc386a Author: Nikolay Sivov <nsivov(a)codeweavers.com> Date: Thu Dec 2 15:37:19 2021 +0300 dwrite: Use CRT allocation functions. Nothing in that commit should change the behavior. I double-checked the result, and it's consistent. I tried running with WINEDEBUG=warn+heap, both before and after the bad commit, and that didn't change the behavior or print any warnings while the test process was running. Leaving component unknown because it's not clear to me that dwrite is doing anything wrong. -- 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=52187 Esme Povirk <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bunglehead(a)gmail.com, | |madewokherd(a)gmail.com Regression SHA1| |909f7aa7c21f82114586e98a0a9 | |e940b13bc386a URL| |https://dl.winehq.org/wine/ | |wine-mono/7.0.0/wine-mono-7 | |.0.0-tests.zip Keywords| |download, regression, | |source, 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=52187 --- Comment #1 from Esme Povirk <madewokherd(a)gmail.com> --- Changing just buff in dwritetextanalyzer_AnalyzeLineBreakpoints to heap allocation also works around the hang. -- 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=52187 --- Comment #2 from Esme Povirk <madewokherd(a)gmail.com> --- I think the compiler is assuming that text is non-NULL because it was passed into memcpy. -- 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=52187 --- Comment #3 from Esme Povirk <madewokherd(a)gmail.com> --- Found some explanation of this: https://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-p... -- 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=52187 --- Comment #4 from Esme Povirk <madewokherd(a)gmail.com> --- I'm working on a patch, but there are a lot of places memcpy or memmove are used. -- 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=52187 --- Comment #5 from Esme Povirk <madewokherd(a)gmail.com> --- OK, I don't think there are any memcpys or memmoves that could cause this problem other than the functions using GetTextAtPosition, but we have more problems. The loop can end successfully and copy fewer than 'length' characters into the buffer, but it doesn't account for the shorter length, so part of the buffer is used uninitialized. -- 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=52187 --- Comment #6 from Esme Povirk <madewokherd(a)gmail.com> --- I guess the safest way to address that, since we're in code freeze, would be to zero the buffer when we first allocate it. Ideally, I think we should add tests for this case, and possibly the related case of just one of text or len being 0, and see if it works on Windows. But right now wine-mono depends on it working. -- 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=52187 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |dwrite --- Comment #7 from Nikolay Sivov <bunglehead(a)gmail.com> --- Yes, I definitely plan to add some tests for this, at some point. Were you able to confirm if wine-mono calls AnalyzeLineBreakpoints() with length including null terminator? It should just be valuable characters, this way we'll avoid allocation and this inner loop. Again, this is something for further improve it, not necessarily for 7.0. -- 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=52187 --- Comment #8 from Esme Povirk <madewokherd(a)gmail.com> --- No, I never actually looked into why the length is wrong. -- 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=52187 --- Comment #9 from Esme Povirk <madewokherd(a)gmail.com> --- I think it's including one character for the TextEndOfLine. -- 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=52187 --- Comment #10 from Esme Povirk <madewokherd(a)gmail.com> --- One other thing I'm noticing: the loop seems to assume position == 0? I think it should be passing position+read to GetTextAtPosition. -- 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=52187 --- Comment #11 from Nikolay Sivov <bunglehead(a)gmail.com> --- Yes, I think you're right. -- 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=52187 --- Comment #12 from Esme Povirk <madewokherd(a)gmail.com> --- So far I haven't found a way to fix this on the wine-mono side without breaking the tests. FullTextLine is pretty complicated. -- 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=52187 --- Comment #13 from Esme Povirk <madewokherd(a)gmail.com> --- This seems to fix it on the wine-mono side: https://github.com/madewokherd/wpf/commit/74120bddded35b72176c6f464a1903d6e8... -- 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=52187 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|bunglehead(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=52187 Esme Povirk <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |412b36e45a7d086747b6ef42b9b | |9b0e432ac4d77 Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #14 from Esme Povirk <madewokherd(a)gmail.com> --- Fix committed. -- 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=52187 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|wine-mono's WPF tests hang |wine-mono's WPF tests hang | |(broken handling of | |IDWriteTextAnalysisSource | |callbacks in analyzer) -- 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=52187 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #15 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 7.0-rc2. -- 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