Memory Corruption Bug Options

[JRob]

I've been looking at the entity handle code, and I think it is bugged and causing crashes.

http://sourceforge.net/p/rcbot2/code/HEAD/...t_ehandle.h#l59

First of all, it should null m_pEnt if it is free or the serial number changed. It shoudn't just leave it until another entity reuses it. even though the chances might be low.

Next, there is a bunch of places where these MyEHandle aren't checked for null (the wrapped entity, not the MyEHandle itself). The first thing that crashed for me was here

http://sourceforge.net/p/rcbot2/code/HEAD/...rtress.cpp#l671

and there are probably a lot more. I fixed CBotFortress :: setVisible though.

CODE

bool CBotFortress :: setVisible ( edict_t *pEntity, bool bVisible )
{
    bool bValid = CBot::setVisible(pEntity,bVisible);

    // check for people to heal
    if ( m_iClass == TF_CLASS_MEDIC )
    {
        if ( bValid && bVisible )
        {
            if (CBotGlobals::isPlayer(pEntity) ) // player
            {
                CBotWeapon *pMedigun = m_pWeapons->getWeapon(CWeapons::getWeapon(TF2_WEAPON_MEDIGUN));
                bool bIsSpy = CClassInterface::getTF2Class(pEntity)==TF_CLASS_SPY;
                int iDisguise = 0;

                if ( bIsSpy )
                {
                    CClassInterface::getTF2SpyDisguised(pEntity,&iDisguise,NULL,NULL,NULL);
                }

                if ( pMedigun && pMedigun->hasWeapon() &&
                    (  // Heal my team member or a spy if I think he is on my team
                       (CBotGlobals::getTeam(pEntity) == getTeam()) ||
                       ((bIsSpy&&!thinkSpyIsEnemy(pEntity,(TF_Class)iDisguise)))
                    ) )
                {
                    Vector vPlayer = CBotGlobals::entityOrigin(pEntity);

                    if ( distanceFrom(vPlayer) <= CWaypointLocations::REACHABLE_RANGE )
                    {
                        float fFactor;

                        if ( (fFactor = getHealFactor(pEntity)) > 0 )
                        {
                            if ( m_pHeal.get() != NULL )
                            {
                                if ( m_pHeal != pEntity )
                                {
                                    if ( fFactor > m_fHealFactor )
                                    {            
                                        m_pHeal = pEntity;
                                        m_fHealFactor = fFactor;
                                        updateCondition(CONDITION_SEE_HEAL);
                                    }
                                }
                                else
                                {
                                    // not healing -- what am I doing?
                                    if ( !m_pSchedules->hasSchedule(SCHED_HEAL) )
                                    {
                                        // not healing -- what am I doing?
                                        m_pSchedules->freeMemory();
                                        m_pSchedules->addFront(new CBotTF2HealSched(m_pHeal));
                                    }
                                }
                            }
                            else
                            {                                
                                m_fHealFactor = fFactor;
                                m_pHeal = pEntity;
                                updateCondition(CONDITION_SEE_HEAL);

                                if ( !m_pSchedules->hasSchedule(SCHED_HEAL) )
                                {
                                    // not healing -- what am I doing?
                                    m_pSchedules->freeMemory();
                                    m_pSchedules->addFront(new CBotTF2HealSched(m_pHeal));
                                }
                            }
                        }
                    }
                }
            }
        }
        else if ( m_pHeal == pEntity )
        {
            m_pHeal = NULL;
            removeCondition(CONDITION_SEE_HEAL);
        }
    }
    //else if ( m_iClass == TF_CLASS_SPY ) // Fix
    //{
        // Look for nearest sentry to sap!!!
    if ( bValid && bVisible )
    {
        if ( CTeamFortress2Mod::isSentry(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( (m_iClass!=TF_CLASS_ENGINEER)||!CClassInterface::isObjectCarried(pEntity) )
            {
                if ( !m_pNearestEnemySentry || m_pNearestEnemySentry.notValid() || ((pEntity != m_pNearestEnemySentry) && (distanceFrom(pEntity) < distanceFrom(m_pNearestEnemySentry)) ))
                {
                    m_pNearestEnemySentry = pEntity;
                }
            }
        }
        else if ( CTeamFortress2Mod::isTeleporter(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( !m_pNearestEnemyTeleporter || m_pNearestEnemyTeleporter.notValid() || ((pEntity != m_pNearestEnemyTeleporter)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestEnemyTeleporter))))
            {
                m_pNearestEnemyTeleporter = pEntity;
            }
        }
        else if ( CTeamFortress2Mod::isDispenser(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( !m_pNearestEnemyDisp || m_pNearestEnemyDisp.notValid() || ((pEntity != m_pNearestEnemyDisp)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestEnemyDisp))))
            {
                m_pNearestEnemyDisp = pEntity;
            }
        }
        else if ( CTeamFortress2Mod::isHurtfulPipeGrenade(pEntity,m_pEdict) )
        {
            if ( !m_pNearestPipeGren || m_pNearestPipeGren.notValid() || ((pEntity != m_pNearestPipeGren)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestPipeGren))))
            {
                m_pNearestPipeGren = pEntity;
            }
        }
    }
    else if ( pEntity == m_pNearestEnemySentry )
    {
        m_pNearestEnemySentry = NULL;
    }
    else if ( pEntity == m_pNearestEnemyTeleporter )
    {
        m_pNearestEnemyTeleporter = NULL;
    }
    else if ( pEntity == m_pNearestEnemyDisp )
    {
        m_pNearestEnemyDisp = NULL;
    }
    else if ( pEntity == m_pNearestPipeGren )
    {
        m_pNearestPipeGren = NULL;
    }

    //}

    // Check for nearest Dispenser for health/ammo & flag
    if ( bValid && bVisible && !(CClassInterface::getEffects(pEntity)&EF_NODRAW) ) // EF_NODRAW == invisible
    {
        if ( m_pFlag != pEntity && CTeamFortress2Mod::isFlag(pEntity,getTeam()) )
            m_pFlag = pEntity;
        else if ( (m_pNearestAllySentry != pEntity) && CTeamFortress2Mod::isSentry(pEntity,getTeam()) )
        {
            if ( !m_pNearestAllySentry || m_pNearestAllySentry.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestAllySentry)))
                m_pNearestAllySentry = pEntity;
        }
        else if ( (m_pNearestDisp != pEntity) && CTeamFortress2Mod::isDispenser(pEntity,getTeam()) )
        {
            if ( !m_pNearestDisp || m_pNearestDisp.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestDisp)) )
                m_pNearestDisp = pEntity;
        }
        else if ( (pEntity != m_pNearestTeleEntrance) && CTeamFortress2Mod::isTeleporterEntrance(pEntity,getTeam()) )
        {
            if ( !m_pNearestTeleEntrance || m_pNearestTeleEntrance.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestTeleEntrance)))
                m_pNearestTeleEntrance = pEntity;
        }
        else if ( (pEntity != m_pAmmo) && CTeamFortress2Mod::isAmmo(pEntity) )
        {
            static float fDistance;

            fDistance = distanceFrom(pEntity);

            if ( fDistance <= 200 )
            {

                if ( !m_pAmmo || m_pAmmo.notValid() || (fDistance < distanceFrom(m_pAmmo)))
                    m_pAmmo = pEntity;
            }
            
        }
        else if ( (pEntity != m_pHealthkit) && CTeamFortress2Mod::isHealthKit(pEntity) )
        {
            static float fDistance;

            fDistance = distanceFrom(pEntity);

            if ( fDistance <= 200 )
            {
                if ( !m_pHealthkit || m_pHealthkit.notValid() || (fDistance < distanceFrom(m_pHealthkit)))
                    m_pHealthkit = pEntity;
            }
        }
    }
    else
    {
        if ( pEntity == m_pFlag.get_old() )
            m_pFlag = NULL;
        else if ( pEntity == m_pNearestDisp.get_old() )
            m_pNearestDisp = NULL;
        else if ( pEntity == m_pAmmo.get_old() )
            m_pAmmo = NULL;
        else if ( pEntity == m_pHealthkit.get_old() )
            m_pHealthkit = NULL;
        else if ( pEntity == m_pHeal.get_old() )
            m_pHeal = NULL;
        else if ( pEntity == m_pNearestPipeGren.get_old() )
            m_pNearestPipeGren = NULL;
    }

    return bValid;
}

Here are the MyEHandles in bot_fortress.cpp. It looks like you are correctly checking some of them.

MyEHandle m_pHeal;
MyEHandle m_pLastHeal;
MyEHandle m_pSentryGun;
MyEHandle m_pDispenser;
MyEHandle m_pTeleEntrance;
MyEHandle m_pTeleExit;

MyEHandle m_pAmmo;
MyEHandle m_pHealthkit;

MyEHandle m_pNearestDisp;
MyEHandle m_pNearestEnemySentry;
MyEHandle m_pNearestAllySentry;
MyEHandle m_pNearestEnemyTeleporter;
MyEHandle m_pNearestEnemyDisp;
MyEHandle m_pNearestTeleEntrance;
MyEHandle m_pNearestPipeGren;

MyEHandle m_pFlag;
MyEHandle m_pPrevSpy;

MyEHandle m_pHealer;

MyEHandle m_pLastEnemySentry;
MyEHandle m_NearestEnemyRocket;
MyEHandle m_NearestEnemyGrenade;


MyEHandle m_pDefendPayloadBomb;
MyEHandle m_pPushPayloadBomb;
MyEHandle m_pRedPayloadBomb;
MyEHandle m_pBluePayloadBomb;

[Cheeseh]

Found another issue here. A null edict_t is being passed to CBotTF2HealSched.

Shouldn't these task functions check if the edict is valid? Also these look like callbacks. Shouldn't they check if the edict is still valid? Shouldn't they all be using EHandles?

#0 0xe6788a88 in CBaseEdict::GetIServerEntity (this=0x0) at ../sdk/hl2sdk-ob-valve/public/edict.h:238
#1 0xe678931c in CClassInterfaceValue::getBool (this=0xffbc4d14, edict=0x0, defaultvalue=244, bIsEdict=false) at bot_getprop.h:196
#2 0xe68224a8 in CFindPathTask::debugString (this=0xb8648d30, string=0x0) at bot_task.cpp:2046
#3 0xe68144b1 in CBotTF2HealSched::CBotTF2HealSched (this=0xb3de6f40, pHeal=0x0) at bot_schedule.cpp:121
#4 0xe67c6eda in CBotFortress::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:596
#5 0xe67d25bb in CBotTF2::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:3945
#6 0xe683a141 in CBotVisibles::updateVisibles (this=0xb2c8c880) at bot_visibles.cpp:316
#7 0xe679771d in CBot::think (this=0xeb37f000) at bot.cpp:957
#8 0xe679fcf7 in CBots::botThink () at bot.cpp:3548
#9 0xe680a4b2 in RCBotPluginMeta::Hook_GameFrame (this=0xe68d8ce0 <g_RCBotPluginMeta>, simulating=true) at bot_plugin_meta.cpp:1413

I don't want to check every function. Some functions should just expect a non NULL pointer. if there is a crash then that's a sign of something going wrong in the code above. I don't know why it's going into DebugString function, it seems to be going wrong in there

I thought I mentioned this

https://sourceforge.net/p/rcbot2/code/480/t...2_mod.cpp#l1542

this needs to be ENTINDEX(pOwner) - 1

cheers

[Cheeseh]

I don't want to check every function. Some functions should just expect a non NULL pointer. if there is a crash then that's a sign of something going wrong in the code above. I don't know why it's going into DebugString function, it seems to be going wrong in there
cheers

weel the medic crash was caused by the change we made to the ehandle by making the edit NULL . the best idea is not to change it. I changed entityIsValid and the setvisible bValid Boolean and it's not crashing now

[JRob]

weel the medic crash was caused by the change we made to the ehandle by making the edit NULL . the best idea is not to change it. I changed entityIsValid and the setvisible bValid Boolean and it's not crashing now

I don't think you should remove the ehandle changing it to null, because in that case you are returning null anyway if the serial number doesn't match.

In the rare chance that it becomes valid in the future, you will be accessing the wrong entity.

Also do you know for sure that the serial number can't be 0?