s1lentq / ReGameDLL_CS

:hammer: Reverse-engineered gamedll (CS 1.6 / CZero)
GNU General Public License v3.0
578 stars 198 forks source link

Optimizations and Amxmodx's hamsandwich #17

Closed IgnacioFDM closed 6 years ago

IgnacioFDM commented 8 years ago

Is it normal that some Hamsandwich hooks don't trigger on some situations (but consistently) when ReGameDLL is compiled with MSVC optimizations enabled (as it is in Release configuration), and trigger correctly with optimizations disabled?

    RegisterHam(Ham_Weapon_Reload, "weapon_xm1014", "fw_WeaponReload_xm1014")

With MSVC /Od it triggers not only after pressing R, but also each time a shell comes into the shotgun (as it should). With /O2, it doesn't trigger every time a shell comes in.

Is there some workaround to this, so that it's not needed to entirely disable optimizations?

IgnacioFDM commented 8 years ago

At least in this particular scenario, it correctly triggers if I remove /GL (Whole program optimization)

IgnacioFDM commented 8 years ago

So, looking at the generated assembly, what's happening is pretty obvious. VC++ is removing the virtual function lookup, and inlining code when CXM1014::Reload is called from one place, but not from another.

I wonder if with whole program optimization disabled, other virtual function lookups that one would hook from hamsandwich are being replaced by static calls. I would assume yes.

s1lentq commented 8 years ago

I was unable to find a solution with the settings of visual c++ compiler, so in my opinion the only solution is to change the code to make the visual c++ compiler to think differently.

IgnacioFDM commented 8 years ago

One simple hack I can think of, would be creating dummy classes that extend anything you could hook with hamsandwich. This way, the compiler can never skip virtual table lookup if a class is derived any further.

Edit: Furthermore, if you have whole program optimizations enabled, I think maybe the compiler can detect even though a class is derived, if the virtual functions are never overriden it can skip vtable lookup. So you would need to disable WPO (but with it disabled, you are now guaranteed that compiler never optimizes away vtable lookups I think), or override all virtual functions in addition.

s1lentq commented 8 years ago

This will not be enough, since these dummy classes will be marked as NOXREF.

The easiest way to do the cast before calling (reinterpret_cast<CBasePlayerWeapon *>(this))->Reload();

IgnacioFDM commented 8 years ago

Although one would need to replace every call in the entire codebase, which would take a lot of time (and the resulting code would be pretty ugly).

s1lentq commented 8 years ago

@IgnacioFDM please let me know if the issue persists

IgnacioFDM commented 8 years ago

This fixes it. There are surely a lot more cases other than weaponxm1014 (and weapon*) where this happens. It would be needed to do the same with many (most) classes, which would be quite a big effort.

Off the top of my head:

player func* info* grenade weaponbox armoury_entity weapon_shield

navmesh commented 8 years ago

Hi, thank you for hard working 1.i want to report the "csdm spawn when bomb planted" problem with zbot after the C4 planted, if the zbot respawned,they seems don't know that they are respawned, they kept standing there and do nothing (although he's still in navigation mesh area)

  1. there is also a problem with defusing the c4 while being shooted, if the bot get hit in defusing C4, he cancel defusing and return fire,after killing the enemy (or enemy disappear) he get back to defuse C4 From now he can't defuse it ,he just start defusing than cancel, then start, and cancel again ... Thanks again