[Bug 48877] New: [regression] Melodyne crashes when using the Pitch tool
https://bugs.winehq.org/show_bug.cgi?id=48877 Bug ID: 48877 Summary: [regression] Melodyne crashes when using the Pitch tool Product: Wine Version: unspecified Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: gdiplus Assignee: wine-bugs(a)winehq.org Reporter: marcan(a)marcansoft.com Distribution: --- Created attachment 66814 --> https://bugs.winehq.org/attachment.cgi?id=66814 Partial fix Tested on git master. Regression was introduced by commit 9bc6f004ce, but that commit is not the root cause. The assertion `assert( obj->numRects <= nb_points / 2 );` would fail, but the code actually crashes earlier due to memory corruption as obj->rects overflows. In the normal case of nb_points == 0, ET->ymin..ymax are 2147483647..-2147483648, which makes the code iterate over zero scanlines as ymax < ymin. However, in the crash case, ymin..ymax are -2147483648..378 (note the negative ymin), so it tries to iterate over two billion scanlines and adds rects to the obj until it crashes. This happens because create_polypolygon_region gets passed an insane polygon with a -2147483648,-2147483648 vertex. The actual bug is in GdipWidenPath, which produces NaN float points, which get converted to that insane int after rounding. And this happens because Melodyne is passing in a degenerate path with the first two points at the same position (added debugging trace to dump the path points): format: "index: [type] X, Y" 00df:err:gdiplus:GdipWidenPath 0: [0] 4.000000, 4.000946 00df:err:gdiplus:GdipWidenPath 1: [1] 4.000000, 4.000946 00df:err:gdiplus:GdipWidenPath 2: [3] 4.000000, 4.000946 00df:err:gdiplus:GdipWidenPath 3: [3] 4.000000, 4.000000 00df:err:gdiplus:GdipWidenPath 4: [3] 4.000000, 4.000000 The code goes GdipWidenPath -> widen_open_figure -> widen_cap, where the segment length is computed as 0 and it ends up dividing by zero. I'm not sure what Windows does here. add_bevel_point handles the special case by just placing a point coincident with the path, ignoring the pen width. The attached patch generalizes that to widen_cap as a whole. It fixes the Melodyne crash for me, but I'm not sure if it's the correct behavior, and degenerate segments will probably cause problems in other code paths (e.g. add_anchor seems to have the same bug). Maybe a better solution would be to just remove coincident points from the path before widening (being careful of cases where the path ends up with one point after this). Someone with more experience with the GDI code should look at this, and perhaps test it on Windows to see how it behaves. -- 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=48877 Esme <madewokherd(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd(a)gmail.com, | |whydoubt(a)gmail.com --- Comment #1 from Esme <madewokherd(a)gmail.com> --- We might as well add a test to find out what Windows does in this case. -- 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=48877 --- Comment #2 from Jeff Smith <whydoubt(a)gmail.com> --- (In reply to Esme from comment #1)
We might as well add a test to find out what Windows does in this case.
I had looked into this a bit already, and have some related patches in my gdiplus queue that still need a lot of cleaning-up. In most cases, a repeated point is skipped in Windows. If the entire path reduces to a single point, status 3 (OutOfMemory) is returned (and the path is not altered), just as when a single point is actually provided. There are a few cases however, that are peculiar and need further investigation. -- 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=48877 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |focht(a)gmx.net URL| |https://web.archive.org/web | |/20200411100340/http://s3-e | |u-west-1.amazonaws.com/asse | |ts.celemony.com/Demos/Melod | |yne.4.2.4.001-Demo.zip Keywords| |download Version|unspecified |5.5 -- 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=48877 --- Comment #3 from Jeff Smith <whydoubt(a)gmail.com> --- While wine does need to deal with that initial zero-length line better, I have a suspicion that the problematic path is being created that way due to another bug in gdiplus. I haven't been able to reproduce the problem you describe, so I wonder if you would be able to set WINEDEBUG=+gdiplus and send the output to a log file. Then attach the log file to this bug. -- 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=48877 --- Comment #4 from Hector Martin <marcan(a)marcansoft.com> --- Created attachment 66895 --> https://bugs.winehq.org/attachment.cgi?id=66895 gdiplus trace To clarify, you need to actually load some audio into Melodyne and then click on the pitch tool button. The crash doesn't happen if all you have is a blank canvas. Apparently it also doesn't happen with literally any audio either, though I only realized this just now (a simple sine beep is not enough). I'm attaching a test clip that reproduces the issue. I'm on Melodyne Assistant 4.2.4.001 (it should hopefully repro the same on the trial version, though I'm using a licensed copy). This is the backtrace: Unhandled exception: page fault on read access to 0xffffffffffffffff in 64-bit code (0x00007f56c17ac8fb). Register dump: rip:00007f56c17ac8fb rsp:00000000002284c0 rbp:00000000002288e0 eflags:00010207 ( R- -- I - -P-C) rax:8029cc607fffff60 rbx:0000000006ef5ca0 rcx:000000000000004f rdx:8029cc607fffff60 rsi:0000000006ff8cf0 rdi:0000000000228850 r8:0000000006ff8820 r9:0000000000000008 r10:0000000000000000 r11:0000000006ff8840 r12:00000000063ceda0 r13:00000000065b0530 r14:0000000000000009 r15:0000000000229610 Stack dump: 0x00000000002284c0: 0000000000000000 0000000100000002 0x00000000002284d0: 0000000007003890 0000000006ff8b20 0x00000000002284e0: 0000000006ff8be0 0000000006ff8be0 0x00000000002284f0: 0000000000000000 0000000000228500 0x0000000000228500: 0000000006ff8ca0 0000000006ff8c20 0x0000000000228510: 000000000000017a 0000000000228520 0x0000000000228520: 0000000006ff8c60 0000000006ff8c60 0x0000000000228530: 000000000000017b 0000000000000000 0x0000000000228540: 0000000000228540 0000000000228540 0x0000000000228550: 0000000080000000 00000000002284e0 0x0000000000228560: 0000000000000000 0000000000000000 0x0000000000228570: 0000000000000000 0000000000000000 Backtrace: =>0 0x00007f56c17ac8fb create_polypolygon_region+0x741() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\region.c:2700] in gdi32 (0x00000000002288e0) 1 0x00007f56c17acb8c CreatePolyPolygonRgn+0x7b() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\region.c:2770] in gdi32 (0x00000000002289a0) 2 0x00007f56c179c98b path_to_region+0x1a0() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\path.c:356] in gdi32 (0x0000000000228a10) 3 0x00007f56c179da4c PathToRegion+0xfe() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\path.c:705] in gdi32 (0x0000000000228b00) 4 0x00007f56c17a5035 nulldrv_SelectClipPath+0x2a() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\path.c:2059] in gdi32 (0x0000000000228b40) 5 0x00007f56c179dcc7 SelectClipPath+0xbd() [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\path.c:748] in gdi32 (0x0000000000228c40) 6 0x00007f56bf5fe96e brush_fill_path+0x60() [Z:\home\marcan\software\wine\build64\dlls\gdiplus\..\..\..\dlls\gdiplus\graphics.c:1080] in gdiplus (0x0000000000228ce0) 7 0x00007f56bf609fab GDI32_GdipFillPath+0x139() [Z:\home\marcan\software\wine\build64\dlls\gdiplus\..\..\..\dlls\gdiplus\graphics.c:4239] in gdiplus (0x0000000000228d30) 8 0x00007f56bf60a182 GdipFillPath+0x135() [Z:\home\marcan\software\wine\build64\dlls\gdiplus\..\..\..\dlls\gdiplus\graphics.c:4292] in gdiplus (0x0000000000228db0) 9 0x00007f56bf608eca SOFTWARE_GdipDrawPath+0x358() [Z:\home\marcan\software\wine\build64\dlls\gdiplus\..\..\..\dlls\gdiplus\graphics.c:3939] in gdiplus (0x0000000000228e40) 10 0x00007f56bf60902a GdipDrawPath+0x13f() [Z:\home\marcan\software\wine\build64\dlls\gdiplus\..\..\..\dlls\gdiplus\graphics.c:3966] in gdiplus (0x0000000000228eb0) 11 0x000000018159a17d EntryPoint+0xfff6b3b4() in melodynecore-4.2.4.001 (0x0000000000228f40) 12 0x0000000180fa2c2c EntryPoint+0xff973e63() in melodynecore-4.2.4.001 (0x0000000000228fd9) 0x00007f56c17ac8fb create_polypolygon_region+0x741 [Z:\home\marcan\software\wine\build64\dlls\gdi32\..\..\..\dlls\gdi32\region.c:2700] in gdi32: movq (%rax),%rax 2700 LIST_FOR_EACH_ENTRY( active, &AET, struct edge_table_entry, entry ) And I'm attaching the log from an untouched wine master (well, as of a few days ago), though AFAICT tracing gdiplus doesn't log the actual path points without extra patching so I'm not sure how useful it will be. -- 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=48877 --- Comment #5 from Hector Martin <marcan(a)marcansoft.com> --- Created attachment 66896 --> https://bugs.winehq.org/attachment.cgi?id=66896 audio clip for repro -- 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=48877 --- Comment #6 from Jeff Smith <whydoubt(a)gmail.com> --- About 60 lines from the end, I found something that matches what I suspected. 00a1:trace:gdiplus:GdipStartPathFigure (0000000006EF5CD0) 00a1:trace:gdiplus:GdipAddPathLine2 (0000000006EF5CD0, 0000000000228A10, 1) 00a1:trace:gdiplus:GdipGetPathLastPoint (0000000006EF5CD0, 0000000000228A10) 00a1:trace:gdiplus:GdipAddPathBeziers (0000000006EF5CD0, 0000000000228A10, 4) We can fairly assume that the first bezier point is identical to the point added in GdipAddPathLine2. GdipAddPathBeziers is _supposed_ to recognize this and not duplicate the point. I have a patch series that fixes this, and the equivalent bug in several other GdipAddPath* functions, that I will post to wine-devel shortly. -- 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=48877 --- Comment #7 from Jeff Smith <whydoubt(a)gmail.com> --- My patches related to this issue were merged into git master a few days ago. Can you please retest? -- 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=48877 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|[regression] Melodyne |Melodyne crashes when using |crashes when using the |the Pitch tool |Pitch tool | Keywords| |regression --- Comment #8 from Gijs Vermeulen <gijsvrm(a)gmail.com> --- Can this be retested? I tried myself, but it seems there's no edit in the trial. I can load the audio clip, but can't do anything with it. -- 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=48877 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fgouget(a)codeweavers.com Keywords| |patch -- 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=48877 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Status|UNCONFIRMED |NEEDINFO --- Comment #9 from Nikolay Sivov <bunglehead(a)gmail.com> --- Hector, please retest with wine 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.
https://bugs.winehq.org/show_bug.cgi?id=48877 Alex Henrie <alexhenrie24(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |RESOLVED CC| |alexhenrie24(a)gmail.com Resolution|--- |FIXED --- Comment #10 from Alex Henrie <alexhenrie24(a)gmail.com> --- I'm marking this bug as fixed because Melodyne 4 can no longer be activated to unlock the pitch tool, Melodyne 5's pitch tool does not crash, and we have good reasons to believe that the problem was resolved in Wine 5.7 by Jeff's gdiplus changes. -- 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=48877 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #11 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 10.0-rc4. -- 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