FLHook SVN discussion

Schmackbolzen

Hehe, no need to be

Huor

Just wanted to mention that this stack corruption is also in the DefenseModule

And in swprintf routines its better to call _countof instead of sizeof. sizeof returns the wrong value for wide character strings and the possibility of memory corruption might occur too.

Schmackbolzen

I think he just forgot. The calls are mostly correct and use wsclen stuff, but on a few occasions it is indeed sizeof, which gives double the count. Shouldn’t be that bad, though, because of the formatted texts being short in those cases (just had a glance at it).

Huor

Well sizeof can be used for char, for wchar its better to use _countof. wsclen is just the length of the string but not the size of the buffer that holds these strings

And this is not only for Cannons plugin, that is also in the base FlHook version and in some other plugins too

Schmackbolzen

I am not sure I follow you. The base FLHook does not even use the overloaded call for swprintf where you also pass the buffer length. There are only a few calls which use sizeof and I agree that they are problematic.

FriendlyFire

Schmack, I’ve added you to the dev group. You can now patch things yourself.

Huor, you don’t have a Forge account just yet, but if you do make one I can add you too.

DJ.DBFuz

will the patches be available to the public aswell ?

Alex.

It’s in SVN, DJ.DBFuz: http://svn.the-starport.net/flhookplugin/

DJ.DBFuz

Oh thank you
I was beein blinde like usual ^^

Now I only need a “Howto add PlayerBases to your mod” guide

Schmackbolzen

@FF Thanks, I will correct it when I find the time. Maybe I add some stuff I recently reversed, too.

Huor

FriendlyFire wrote:
…Huor, you don’t have a Forge account just yet, but if you do make one I can add you too.

Now i remember i already have an account - dunno since months, but it does not let me in. Yesterday i tried again but it refuses - even with a new password. So there is something wrong. Maybe someone might have a look into this?!

Thanks in advance.

FriendlyFire

I’ve checked and I do not see a user named “huor” so I think you don’t have an account.

w0dk4

There is “Huor” with last connection(login?) of yesterday actually.

/edit: added you as developer

Huor

Just to come back to topic. Its something i am currently stumbled.
In some of the function calls FlHook uses a variable argument list to handle multiple arguments. However such a variable argument list must be started with

va_start ()

call. So far so good, but to clean the stack it is required to have the

va_end ()

as well, which i searched for, is missing in every call. I am not sure what happens if its left, but i learned that both should be used together. Whether i am not sure if thats important - i corrected it now in my version.

Schmackbolzen

Well, if you look at the documentation for the msvc compiler there only is stated that the pointer is set to NULL. Nowhere is explicitly written that you have to call it. The example with void testit(…) doesn’t. The one after that does.

If you look on www.cplusplus.com they do state this. Whether this causes errors - no idea. Doesn’t seem to, though. With gcc etc this might be different.

Huor

Just want to mention that the last added revision from w0dka (236) seems to miss the pushad and popad commands within the asm code [in dynamic_missions/Hooks.cpp]

Schmack already pointed that out in the base plugin code where it was corrected already. Just thought to mention it.

Schmackbolzen

Yeah, I see what you mean. I’d call it risky, since the registers after the calls from the asm code could contain different values than the msvc compiler assumes while compiling, leading to unexpected behaviour.

w0dk4

Any function you call will clean up and preserve registers itself. A function that does not do that is generally buggy/faulty.
Thats why you dont need pushads/pops around “call xy”.

As regards __asm blocks, msdn has this to say:

When using __asm to write assembly language in C/C++ functions, you don’t need to preserve the EAX, EBX, ECX, EDX, ESI, or EDI registers.

You should preserve other registers you use (such as DS, SS, SP, BP, and flags registers) for the scope of the __asm block. You should preserve the ESP and EBP registers unless you have some reason to change them (to switch stacks, for example).

Schmackbolzen

Yes. this sounds all nice, however since there also is stated:

However, using these registers will affect code quality because the register allocator cannot use them to store values across __asm blocks. In addition, by using EBX, ESI or EDI in inline assembly code, you force the compiler to save and restore those registers in the function prologue and epilogue.

The compiler seems to analyse the inline asm code, which ofc does not work when doing external calls. For example the second function uses edi, but the compiler never saves it before and after the asm block (i’ve checked with IDA), but reads it later.

Notice, that I still am not quite sure how to interpret the text exactly. I’ve checked the generated assembly code of such blocks some time ago and came to the conclusion that it is not quite clear how save it is, since it is very time consuming to check all the cases.

w0dk4

A function will preserve all registers except for eax (and esp, eip obviously) so I dont understand what the problem is with edi?