Eric Pouech wrote: > > I've been trying to solve some issues with MS Media Player > the Favorite menu doesn't appear correctly > it's an owner drawn menu, and the bug (a huge menu being displayed) > comes from bogus values used while painting > I tried to investigate it a bit, and I must confess I'm quite puzzled > by the outcome: > - from a regression point of view, but has been introduced here > >http://cvs.winehq.com/patch.py?root=/opt/cvs-commit&logs=/opt/cvs-commit/CVSROOT/winecommitlog&id=970458426348898677621037 > - I've had a look at the patch but didn't find anything wrong (except > for the attached patch, but it doesn't help) > - running the program with -debugmsg +relay no longer displays the > issue (in fact, turning relay debugging only for channel advapi32 > also solves the problem) > - looked at function signatures (WINAPI mismatch), but didn't help > either well, after some further investigation, hair ripping, lots of coffee... I found the culprit. it's a gcc bug :-(( We use: DWORD WINAPI RegCloseKey( HKEY hkey ) { if (!hkey || hkey >= 0x80000000) return ERROR_SUCCESS; return RtlNtStatusToDosError( NtClose( hkey ) ); } which produces: 0x2ab301f8 <RegCloseKey>: push %ebp 0x2ab301f9 <RegCloseKey+1>: mov %esp,%ebp 0x2ab301fb <RegCloseKey+3>: sub $0x14,%esp 0x2ab301fe <RegCloseKey+6>: push %ebx 0x2ab301ff <RegCloseKey+7>: call 0x2ab30204 <RegCloseKey+12> 0x2ab30204 <RegCloseKey+12>: pop %ebx 0x2ab30205 <RegCloseKey+13>: add $0x71e54,%ebx 0x2ab3020b <RegCloseKey+19>: mov 0x8(%ebp),%edx 0x2ab3020e <RegCloseKey+22>: lea 0xffffffff(%edx),%eax 0x2ab30211 <RegCloseKey+25>: cmp $0x7ffffffe,%eax 0x2ab30216 <RegCloseKey+30>: ja 0x2ab30230 <RegCloseKey+56> 0x2ab30218 <RegCloseKey+32>: add $0xfffffff4,%esp 0x2ab3021b <RegCloseKey+35>: add $0xfffffff4,%esp 0x2ab3021e <RegCloseKey+38>: push %edx 0x2ab3021f <RegCloseKey+39>: call 0x2aae2750 <_init+15000> 0x2ab30224 <RegCloseKey+44>: push %eax 0x2ab30225 <RegCloseKey+45>: call 0x2aae0960 <_init+7336> 0x2ab3022a <RegCloseKey+50>: jmp 0x2ab30232 <RegCloseKey+58> 0x2ab3022c <RegCloseKey+52>: lea 0x0(%esi,1),%esi 0x2ab30230 <RegCloseKey+56>: xor %eax,%eax 0x2ab30232 <RegCloseKey+58>: mov 0xffffffe8(%ebp),%ebx 0x2ab30235 <RegCloseKey+61>: mov %ebp,%esp 0x2ab30237 <RegCloseKey+63>: pop %ebp 0x2ab30238 <RegCloseKey+64>: ret $0x4 if we write instead DWORD WINAPI RegCloseKey( HKEY hkey ) { NTSTATUS ret; if (!hkey || hkey >= 0x80000000) return ERROR_SUCCESS; ret = NtClose(hkey); return RtlNtStatusToDosError( ret ); } we get: 0x2ab301f8 <RegCloseKey>: push %ebp 0x2ab301f9 <RegCloseKey+1>: mov %esp,%ebp 0x2ab301fb <RegCloseKey+3>: sub $0x14,%esp 0x2ab301fe <RegCloseKey+6>: push %ebx 0x2ab301ff <RegCloseKey+7>: call 0x2ab30204 <RegCloseKey+12> 0x2ab30204 <RegCloseKey+12>: pop %ebx 0x2ab30205 <RegCloseKey+13>: add $0x71e54,%ebx 0x2ab3020b <RegCloseKey+19>: mov 0x8(%ebp),%edx 0x2ab3020e <RegCloseKey+22>: lea 0xffffffff(%edx),%eax 0x2ab30211 <RegCloseKey+25>: cmp $0x7ffffffe,%eax 0x2ab30216 <RegCloseKey+30>: ja 0x2ab30230 <RegCloseKey+56> 0x2ab30218 <RegCloseKey+32>: add $0xfffffff4,%esp 0x2ab3021b <RegCloseKey+35>: push %edx 0x2ab3021c <RegCloseKey+36>: call 0x2aae2750 <_init+15000> 0x2ab30221 <RegCloseKey+41>: push %eax 0x2ab30222 <RegCloseKey+42>: call 0x2aae0960 <_init+7336> 0x2ab30227 <RegCloseKey+47>: jmp 0x2ab30232 <RegCloseKey+58> 0x2ab30229 <RegCloseKey+49>: lea 0x0(%esi,1),%esi 0x2ab30230 <RegCloseKey+56>: xor %eax,%eax 0x2ab30232 <RegCloseKey+58>: mov 0xffffffe8(%ebp),%ebx 0x2ab30235 <RegCloseKey+61>: mov %ebp,%esp 0x2ab30237 <RegCloseKey+63>: pop %ebp 0x2ab30238 <RegCloseKey+64>: ret $0x4 the only notable difference (NTSTATUS ret get optimized in register variable %eax) is twice the "add $0xfffffff4,%esp" in the buggy case this explain the stack trashing (-12 on stack can give bad results) and also why it was working with +relay (relay adds another call level, hence the stack trashing happened on the relay code, not on the caller...) I'm using gcc 2.95.1. Could folks with some other GCC versions check what they get there ? The simple fix would be not to nest the two calls and use an intermediate variable, but let's wait for the testings on other GCC version... A+ -- --------------- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle
