Devilution: Fix decompilation of "msgcmd.cpp"

[ghost]

This entire file got hosed in the dumping process, mainly because the types are unknown. I'm putting this here so I remember to fix it later:

.text:00408A0B                 call    msgcmd_send_chat
.text:00408A10                 mov     gbGameLoopStartup, esi
.text:00408A16                 call    DrawAndBlit
.text:00408A1B loc_408A1B:
.text:00408A1B                 cmp     gbRunGame, esi
.text:00408A21                 jnz     loc_40896B

.text:0043F8CC                 push    esi
.text:0043F8CD                 call    msgcmd_delete_server_cmd_W
.text:0043F8D2 loc_43F8D2: 
.text:0043F8D2                                         ; msgcmd_send_chat+20↑j
.text:0043F8D2                 pop     esi // should be returning a value from __thiscall??



md5-0fc80f1372be3d1d2993a251ee769bf8



.text:0043FA98 ; void __fastcall msgcmd_cleanup_extern_msg(ServerCommand **extern_msgs)
(...)
.text:0043FAB1                 mov     [edx], esi
.text:0043FAB3                 mov     eax, [ecx]
.text:0043FAB5                 mov     edx, [ecx+4]
.text:0043FAB8                 mov     [eax+4], edx
.text:0043FABB                 and     dword ptr [ecx], 0 // overwrites ESP
.text:0043FABE                 and     dword ptr [ecx+4], 0 // ESP+4
.text:0043FAC2 loc_43FAC2:
.text:0043FAC2                 pop     esi // wtf is this???
.text:0043FAC3                 retn

[heinermann]

wtf is this??? it pushes esi at the beginning of the function and pops it at the end... :/

[mewmew]

wtf is this??? it pushes esi at the beginning of the function and pops it at the end... :/

This is a convention used to store caller registers. The push in the beginning of the function (the function prologue) stores the value of the esi register, and the pop at the end of the function (the function epilogue) restores the original value of the esi register. This makes it possible to use the esi register within the function, and still retain the value of the esi register as expected by the caller function.

[ghost]

Well yeah, I know that :P The problem is that ESI appears to be passed as an argument for some functions, and being used as a return value for others. This messes up the decompiler since it assumes EAX as return and fastcall convention.

It shouldn't be hard to fix, especially since we can use PvPGn to emulate battle.net to test chat commands. That should also help fill in the remaining struct fields. Timed messages are also broken, but the code never appears to be executed anyway.

[mewmew]

Well yeah, I know that :P

Hehe, yea. I felt I stated something rather basic after posting. Oh well :)

[heinermann]

Which function uses esi as a return value?

[sunverwerth]

https://reverseengineering.stackexchange.com/questions/2673/what-x86-calling-convention-passes-first-parameter-via-esi

TLDR: Probably link time optimization

Edit: This might also concern #111

[squidcc]

ChatCmd::extern_msgs might be a union. Complex types as parameters to fastcall functions aren't passed in registers, they get pushed on the stack like normal. This is what msgcmd_delete_server_cmd_W expects because it cleans it up when returning ("retn 4").

[squidcc]

Had another look and I now think these might be __thiscall (i.e. c++ member) functions. This would make sense since the SMemAlloc/SMemFree calls are using -2 (SLOG_OBJECT) for their logline argument.

[squidcc]

The more I look, the uglier it gets.
Those SMemAlloc and SMemFree calls with the ".?AUEXTERNMESSAGE@@" tag? They come from a const type_info class, at offset 0x49F070. That means in the original code there must be a call to typeid(X) where X is an instance of struct EXTERNMESSAGE. Now here's the weird thing: the string .?AUEXTERNMESSAGE@@ would normally be accessed by calling the type_info::raw_name() function, but instead it's being accessed directly. I double-checked the <typeinfo> header for VC6 and char _m_d_name[] is definitely a private member of class type_info, so I think the only way this could happen is if they redefined type_info locally (or something equally dumb e.g. (const char*)&typeid(sgChat_Cmd)+8) - which means we will probably have to use the same ugly hack to reproduce it exactly and break compatibility with compilers other than MSVC.

[sskras]

Now what's the summary?