Memory Corruption Bug |
Memory Corruption Bug |
JRob |
Sep 20 2015, 11:33 PM
Post
#1
|
Advanced Member Group: Members Posts: 52 Joined: 30-April 11 Member No.: 1,970 |
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 |
Nov 20 2015, 08:12 AM
Post
#2
|
Admin Group: Admin Posts: 3,055 Joined: 11-September 03 From: uk Member No.: 1 |
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 |
Nov 21 2015, 09:03 AM
Post
#3
|
Admin Group: Admin Posts: 3,055 Joined: 11-September 03 From: uk Member No.: 1 |
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 |
Nov 25 2015, 01:24 AM
Post
#4
|
Advanced Member Group: Members Posts: 52 Joined: 30-April 11 Member No.: 1,970 |
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? |
Cheeseh |
Nov 26 2015, 10:20 AM
Post
#5
|
Admin Group: Admin Posts: 3,055 Joined: 11-September 03 From: uk Member No.: 1 |
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? this ehandle code comes from HL1 not HL2 so there may be some differences, Apparently hl2 invalid serial number is -1 (INVALID_EHANDLE_INDEX) so I might change it to that. the problem with changing the edict to NULL is, in that in the code above I can assign an ehandle to an edict expecting the ehandle to have a valid edict, although once it goes into the assign code within the ehandle it will nullify the ehandle, therefore making the expected non null ehandle null, causing all sorts of problems. It doesn't matter whether or not its null as it will always return null anyway if its invalid, but the previous invalid edict will still be stored albeit inaccessible. |
Lo-Fi Version | Time is now: 29th March 2024 - 11:50 AM |