PlaneShift

Development => Development Deliberation => Topic started by: esben on December 27, 2004, 11:08:30 pm

Title: [patch] mathscript.cpp bugs re casts, both implicit and explicit
Post by: esben on December 27, 2004, 11:08:30 pm

Hi,

there is bugs in mathscript. Firstly, there is two instances of casting size_t to unsigned int, which is wrong. As this unsigned int is later compared to (size_t)-1 to exit a loop, MatchScript::MatchScript will loop endlessly on platforms where sizeof(unsigned int) < sizeof(size_t) --- e.g. amd64.

I have posted a patch for this  here: http://www.mosehansen.dk/mathscript.patch

There is also some very dubious code, like this:

    iScriptableVar *obj;
// ---cut....
 return (double)(int)obj;

Again, on 64-bit platforms, the above is even less sensible than on 32-bit platforms. I\'ll try to fix this some other day, if I doesn\'t forget.

Feel free to contact me by Jabber at mesbenh@jabber.dk, or by email --- see http://www.mosehansen.dk/about.html

Happy coding!
Esben M Hansen

Title:
Post by: Xordan on December 28, 2004, 07:07:13 pm

This patch causes the game to segfault on loading world. My client is in release, so I havn\'t any very useful output :) Here\'s what I have though:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 16384 (LWP 31225)]
0x0000000000414ee0 in csString::operator char const* ()
(gdb) bt
#0  0x0000000000414ee0 in csString::operator char const* ()
#1  0x0000000000455622 in GEMClientActor::GetGuildName ()
#2  0x0000000000452f4e in psEntityLabels::MakeRowsText ()
#3  0x0000000000453d53 in psEntityLabels::CreateLabelOfObject ()
#4  0x00000000004548af in psEntityLabels::OnObjectArrived ()
#5  0x0000000000458312 in psCelClient::HandleActor ()
#6  0x0000000000458c53 in psCelClient::HandleMessage ()
#7  0x00000000005008a3 in MsgHandler::Publish ()
#8  0x00000000004df4b4 in psClientMsgHandler::HandleEvent ()
#9  0x00000000004df75d in psClientMsgHandler::EventHandler::HandleEvent ()
#10 0x00000000005d2e80 in csEventQueue::Notify ()
#11 0x00000000005d2ed3 in csEventQueue::Process ()
#12 0x00000000005ea130 in csDefaultRunLoop ()
#13 0x00000000004344ac in main ()

Title:
Post by: esben on December 29, 2004, 11:51:05 am

This must be something that my patch uncovered, I\'d think. Basically, without it, any 64-bit machine should just loop infinitely unless a ; (I believe) happened in byte 2^32-1. Also, the crash you show me seems unrelated, jugding from the backtrace.  This is from a few hours study, though, so maybe I ovrelooked something. What type of system is your crash from?

I\'ll take another whack at getting the beast to run on amd64 when I can... but for today and tommorrow I am busy with familiy stuff :rolleyes:

Title:
Post by: Xordan on December 29, 2004, 03:08:40 pm

Running on gentoo x86_64.

Title: the code I'm more concerned about...
Post by: dimaq on January 10, 2005, 10:58:47 pm

there are several places like this:

double MathScriptEngine::GetArmorVSWeaponResistance(const double *parms)
{
    psItem *weapon = (psItem *)(int)parms[0];
    psItem *armor  = (psItem *)(int)parms[1];
   
    // if no armor return 1
    if (!armor)
        return 1.0F;

    return weapon->GetArmorVSWeaponResistance(armor->GetCurrentStats());
}

now, my understanding is that the (int) cast should actually return a value corresponding to the double source numerically.

another option would be to have a reinterpret_cast.

either way, int is hardly useful on 64-bit platform.
luckily it seems this is used by the server only :)