otya128 / winevdm

16-bit Windows (Windows 1.x, 2.x, 3.0, 3.1, etc.) on 64-bit Windows
GNU General Public License v2.0
2.67k stars 152 forks source link

Roland U20 sound editor U20EDIT.EXE crashes when requesting preset data #864

Open AnttiHuovilainen opened 3 years ago

AnttiHuovilainen commented 3 years ago

Describe the bug

A freeware Windows 3 sound editor (U20EDIT) for the Roland U-20/220 synthesizer crashes near the end of the sysex dump when current preset data is requested. This happens every time when using otvdmw.exe but only on the second attempt when starting with otvdm.exe.

The timing of the crash can be changed slightly by going to the program midi preferences and changing midi device to something else and back to the original (maybe related to uninitialized memory?). Sometimes it also crashes on the first try.

The application seems to be built with Visual Basic 3.0 and uses a third party MIDI1.VBX for the midi communication. Unfortunately it requires the hardware synthesizer to run far enough to crash.

Environment (please complete the following information):

otvdm.ini is default except for "PeekMessageSleep=2" setting.

Procedure I've attached a zip with the required executable, vbx and ini files. Also a trace log with the recommended settings is included. U20EDIT_crash.zip u20edit.log

cracyc commented 3 years ago

Notes: The sysex call is started here.

3068:Call KERNEL.15: GLOBALALLOC(2002,00000010) ret=1987:22b8 ds=198f
     AX=00f0 BX=2bb1 CX=00f0 DX=1b37 SI=29ce DI=29ce ES=1b37 EFL=00003246
3068:Ret  KERNEL.15: GLOBALALLOC() retval=none ret=1987:22b8 ds=198f
     AX=1e86 BX=1e86 CX=1e86 DX=1b37 SI=29ce DI=29ce ES=0000 EFL=00003246
3068:Call KERNEL.18: GLOBALLOCK(1e86) ret=1987:22c1 ds=198f
3068:Ret  KERNEL.18: GLOBALLOCK() retval=1e870000 ret=1987:22c1 ds=198f
3068:Call KERNEL.15: GLOBALALLOC(2002,0000001c) ret=1987:22d3 ds=198f
     AX=0000 BX=1e86 CX=1e87 DX=1e87 SI=29ce DI=29ce ES=0000 EFL=00003206
3068:Ret  KERNEL.15: GLOBALALLOC() retval=none ret=1987:22d3 ds=198f
     AX=1eae BX=1eae CX=1eae DX=1e87 SI=29ce DI=29ce ES=0000 EFL=00003206
3068:Call KERNEL.18: GLOBALLOCK(1eae) ret=1987:22dc ds=198f
3068:Ret  KERNEL.18: GLOBALLOCK() retval=1eaf0000 ret=1987:22dc ds=198f
3068:Call KERNEL.15: GLOBALALLOC(2002,0000000d) ret=1987:2313 ds=198f
     AX=000d BX=0096 CX=000d DX=0000 SI=000d DI=0000 ES=1767 EFL=00003246
3068:Ret  KERNEL.15: GLOBALALLOC() retval=none ret=1987:2313 ds=198f
     AX=1eb6 BX=1eb6 CX=1eb6 DX=0000 SI=000d DI=0000 ES=0000 EFL=00003246
3068:Call KERNEL.18: GLOBALLOCK(1eb6) ret=1987:231c ds=198f
3068:Ret  KERNEL.18: GLOBALLOCK() retval=1eb70000 ret=1987:231c ds=198f
3068:Call MMSYSTEM.206: MIDIOUTPREPAREHEADER(0073,1eaf0000,001c) ret=1987:23ac ds=198f
2118:3068:trace:mmsys:midiOutPrepareHeader16 811: (0073, 1eaf0000, 28)
2118:3068:trace:winmm:MMSYSTDRV_Message 1129: Calling message(msg=5 p1=0x0677fe04 p2=0x0000001c)
3068:Ret  MMSYSTEM.206: MIDIOUTPREPAREHEADER() retval=00000000 ret=1987:23ac ds=198f

From the midi1.vbx source (https://web.archive.org/web/20050619075800/http://home.modemss.brisnet.org.au/~mlevoi/mabrymidi.zip)

                    if( (msg & 0xff) == 0xf0 )
                    {
                        HANDLE      hMidiHdr;
                        LPMIDIHDR   lpMidiHdr;
                        HANDLE      hData;
                        LPVOID      lpData;
                        LPVOID      lpBuffer;
                        USHORT      lenData;
                        HANDLE      hParms;
                        LPLONGPARMS lpLongParms;
badcode:                        
                        hParms =
                            GlobalAlloc( GMEM_MOVEABLE | GMEM_SHARE, sizeof(LONGPARMS) );
                        lpLongParms = GlobalLock( hParms );

                        hMidiHdr =
                            GlobalAlloc( GMEM_MOVEABLE | GMEM_SHARE, sizeof(MIDIHDR) );
                        lpMidiHdr = GlobalLock( hMidiHdr );
                        _fmemset(lpMidiHdr, 0, sizeof(MIDIHDR) );

                        lenData = VBGetHlstrLen( lpctl->hlstrBuffer );

                        hData = GlobalAlloc( GMEM_MOVEABLE | GMEM_SHARE, lenData );
                        lpData = GlobalLock( hData );

                        lpctl = (LPMIDIOUT)VBDerefControl( hctl );

                        lpBuffer = VBDerefHlstr( lpctl->hlstrBuffer );
                        _fmemcpy( lpData, lpBuffer, lenData );                                          
//                      VBGetHlstr( lpctl->hlstrBuffer, lpData, lenData );
                        lpctl = (LPMIDIOUT)VBDerefControl( hctl );

            lpMidiHdr->lpData = lpData;
            lpMidiHdr->dwBufferLength = lenData;
                        lpMidiHdr->dwUser = (LONG)lpLongParms;

                        midiOutPrepareHeader( (HMIDIOUT)lpctl->hMidiDevice, lpMidiHdr, sizeof(MIDIHDR) );

                        lpLongParms->hwndQueueOwner = hwnd;
                        lpLongParms->hMidiHdr = hMidiHdr;
                        lpLongParms->lpMidiHdr = lpMidiHdr;
                        lpLongParms->hData = hData;
                        lpLongParms->lpData = lpData;
                        lpLongParms->hLongParms = hParms;   

Then

2118:3068:trace:message:DispatchMessage16 3374: (0x0069) [03c9] wp=0000 lp=1eaf0000
3068:CallTo16(func=1417:0000,ds=120f,0069,03c9,0000,1eaf,0000) ss:sp=120f:6f20 ax=120f bx=0006 cx=0000 dx=0000 si=0069 di=0000 bp=6f4a es=120f fs=0053
3068:Call USER.129: GETCLASSWORD(0069,ffee) ret=1417:0041 ds=120f
3068:Ret  USER.129: GETCLASSWORD() retval=00020006 ret=1417:0041 ds=120f
3068:Call USER.135: GETWINDOWLONG(0069,0000) ret=1417:004a ds=120f
3068:Ret  USER.135: GETWINDOWLONG() retval=1b3729ce ret=1417:004a ds=120f
3068:Call KERNEL.19: GLOBALUNLOCK(1eb7) ret=1987:293b ds=198f
3068:Ret  KERNEL.19: GLOBALUNLOCK() retval=1eb70000 ret=1987:293b ds=198f
3068:Call KERNEL.17: GLOBALFREE(1eb7) ret=1987:294d ds=198f
2118:3068:trace:selector:SELECTOR_FreeBlock 170: (1eb7,1)
        case MOM_DONE:
        {
            LPLONGPARMS lplp = (LPLONGPARMS)lp;
            HANDLE      hlp = lplp->hLongParms;

            GlobalUnlock( lplp->hMidiHdr );
            GlobalFree( lplp->hMidiHdr );
            GlobalUnlock( lplp->hData );
            GlobalFree( lplp->hData );
            GlobalUnlock( hlp );
            GlobalFree( hlp );

What's passed to globalunlock is not a handle so lpLongParms is getting corrupted somewhere.

Edit: Wait, it passes the wrong pointer...

void FAR PASCAL MidiOutFunc( HMIDIOUT hMidiOut, WORD wMsg, DWORD dwInstance, DWORD dwParam1, DWORD dwParam2 )
{
    if( wMsg == MOM_DONE )
    {
        LPMIDIHDR   lpMidiHdr = (LPMIDIHDR)dwParam1;
        LPLONGPARMS lpLongParms = (LPLONGPARMS)(lpMidiHdr->dwUser);

        midiOutUnprepareHeader( hMidiOut, lpMidiHdr, sizeof(MIDIHDR) );     
        PostMessage( lpLongParms->hwndQueueOwner, MOM_DONE, 0, dwParam1 );
    }
    return;
}

It's passing in lParam dwParam1 but the above code expects lpLongParms.

cracyc commented 3 years ago

Maybe if there were a sysex file with exactly what the synth is supposed to return then I could try to feed that back to the software.