Critical errors in CryEngine V code

Introduction

mkdir ~/projects && cd ~/projects
git clone https://github.com/CRYTEK/CRYENGINE.git
cd CRYENGINE/
git checkout main
chmod +x ./download_sdks.py
./download_sdks.py
pvs-studio-analyzer trace -- \
sh ./cry_waf.sh build_linux_x64_clang_profile -p gamesdk
pvs-studio-analyzer analyze \
-l /path/to/PVS-Studio.lic \
-o ~/projects/CRYENGINE/cryengine.log \
-r ~/projects/CRYENGINE/ \
-C clang++-3.8 -C clang-3.8 \
-e ~/projects/CRYENGINE/Code/SDKs \
-j4
plog-converter -a GA:1,2 -t tasklist \
-o ~/projects/CRYENGINE/cryengine_ga.tasks \
~/projects/CRYENGINE/cryengine.log

A strange Active() function

void SetActive(bool bActive)
{
if (bActive == bActive)
return;
m_bActive = bActive;
OnResetState();
}
class CFeatureCollision : public CParticleFeature
{
public:
CRY_PFX2_DECLARE_FEATURE
public:
CFeatureCollision();
....
bool IsActive() const { return m_terrain ||
m_staticObjects ||
m_staticObjects; }
....
bool m_terrain;
bool m_staticObjects;
bool m_dynamicObjects;
};

Code above has no bugs

static bool CompactBoneVertices(....,
DynArray<uint16>& outArrIndices, ....) // <= uint16
{
....
outArrIndices.resize(3 * inFaceCount, -1);
int outVertexCount = 0;
for (int i = 0; i < verts.size(); ++i)
{
....
outArrIndices[....] = outVertexCount - 1;
}
// Making sure that the code above has no bugs // <= LOL
for (int i = 0; i < outArrIndices.size(); ++i)
{
if (outArrIndices[i] < 0) // <= LOL
{
return false;
}
}

return true;
}

Memory handling errors

void CGeomCacheRenderNode::Render(....)
{
....
CREGeomCache* pCREGeomCache = iter->second.m_pRenderElement;
....
uint8 hashableData[] =
{
0, 0, 0, 0, 0, 0, 0, 0,
(uint8)std::distance(pCREGeomCache->....->begin(), &meshData),
(uint8)std::distance(meshData....->....begin(), &chunk),
(uint8)std::distance(meshData.m_instances.begin(), &instance)
};
memcpy(hashableData, pCREGeomCache, sizeof(pCREGeomCache));
....
}
void
CClipVolumeManager::GetMemoryUsage(class ICrySizer* pSizer) const
{
pSizer->AddObject(this, sizeof(this));
for (size_t i = 0; i < m_ClipVolumes.size(); ++i)
pSizer->AddObject(m_ClipVolumes[i].m_pVolume);
}
vector<unique_ptr<CFullscreenPass>> m_jitteredDepthPassArray;void CClipVolumesStage::PrepareVolumetricFog()
{
....
for (int32 i = 0; i < m_jitteredDepthPassArray.size(); ++i)
{
m_jitteredDepthPassArray[i].release();
}
m_jitteredDepthPassArray.resize(depth); for (int32 i = 0; i < depth; ++i)
{
m_jitteredDepthPassArray[i] = CryMakeUnique<....>();
m_jitteredDepthPassArray[i]->SetViewport(viewport);
m_jitteredDepthPassArray[i]->SetFlags(....);
}
....
}
std::unique_ptr<Foo> up(new Foo());
Foo* fp = up.release();
delete fp;
void COctreeNode::LoadSingleObject(....)
{
....
float* pAuxDataDst = pObj->GetAuxSerializationDataPtr(....);
const float* pAuxDataSrc = StepData<float>(....);
memcpy(pAuxDataDst, pAuxDataDst, min(....) * sizeof(float));
....
}

Strange code

void CAttrWriter::PackFloatInSemiConstType(float val, ....)
{
uint32 type = PFSC_VAL;
if (val == 0 || val == -0) // <=
type = PFSC_0;
else if (val == 1)
type = PFSC_1;
else if (val == -1)
type = PFSC_N1;
....
}
if (val == 0.0f || val == -0.0f)
type = PFSC_0;
int CArticulatedEntity::Step(float time_interval)
{
....
for (j=0;j<3;j++) if (!(m_joints[i].flags & angle0_locked<<j)&&
isneg(m_joints[i].limits[0][j]-m_joints[i].qext[j]) +
isneg(m_joints[i].qext[j]-m_joints[i].limits[1][j]) +
isneg(m_joints[i].limits[1][j]-m_joints[i].limits[1][j]) < 2)
{
....
}
  • V501 There are identical sub-expressions ‘m_joints[op[1]].limits[1][i]’ to the left and to the right of the ‘-’ operator. articulatedentity.cpp 513
void COPCrysis2FlightFireWeapons::ParseParam(....)
{
....
bool paused;
value.GetValue(paused);
if (paused && (m_State != eFP_PAUSED) &&
(m_State != eFP_PAUSED_OVERRIDE))
{
m_NextState = m_State;
m_State = eFP_PAUSED;
m_PausedTime = 0.0f;
m_PauseOverrideTime = 0.0f;
}
else if (!paused && (m_State == eFP_PAUSED) && // <=
(m_State != eFP_PAUSED_OVERRIDE)) // <=
{
m_State = m_NextState;
m_NextState = eFP_STOP;
m_PausedTime = 0.0f;
m_PauseOverrideTime = 0.0f;
}
....
}
int CTriMesh::Slice(...)
{
....
pmd->pMesh[0]=pmd->pMesh[1] = this; AddRef();AddRef();
for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next); // <=
pmd0->next = pmd;
....
}

About pointers

int CBreakableManager::HandlePhysics_UpdateMeshEvent(....)
{
CEntity* pCEntity = 0;
....
if (pmu && pSrcStatObj && GetSurfaceType(pSrcStatObj))
{
....
if (pEffect)
{
....
if (normal.len2() > 0)
pEffect->Spawn(true, pCEntity->GetSlotWorldTM(...); // <=
}
}
.... if (iForeignData == PHYS_FOREIGN_ID_ENTITY)
{
pCEntity = (CEntity*)pForeignData;
if (!pCEntity || !pCEntity->GetPhysicalProxy())
return 1;
}
....
}
void CAudioNode::Animate(SAnimContext& animContext)
{
....
const bool bMuted = gEnv->IsEditor() && (pTrack->GetFlags() &
IAnimTrack::eAnimTrackFlags_Muted);
if (!pTrack || pTrack->GetNumKeys() == 0 ||
pTrack->GetFlags() & IAnimTrack::eAnimTrackFlags_Disabled)
{
continue;
}
....
}
// Safe memory helpers
#define SAFE_RELEASE(p){ if (p) { (p)->Release(); (p) = NULL; } }
void CObjManager::UnloadVegetationModels(bool bDeleteAll)
{
....
SVegetationSpriteLightInfo& rLightInfo = ....;
if (rLightInfo.m_pDynTexture)
SAFE_RELEASE(rLightInfo.m_pDynTexture);
....
}
  • V571 Recurring check. The ‘if (m_pSectorGroups)’ condition was already verified in line 48. PartitionGrid.cpp 50
class CLvlRes_finalstep : public CLvlRes_base
{
....
for (;; )
{
if (*p == '/' || *p == '\\' || *p == 0)
{
char cOldChar = *p;
*p = 0; // create zero termination
_finddata_t fd;
bool bOk = FindFile(szFilePath, szFile, fd); if (bOk)
assert(strlen(szFile) == strlen(fd.name));
*p = cOldChar; // get back the old separator if (!bOk)
return;
memcpy((void*)szFile, fd.name, strlen(fd.name)); // <= if (*p == 0)
break;
++p;
szFile = p;
}
else ++p;
}
....
}

Problems with a comma

bool CTacticalPointSystem::Parse(....) const
{
string sInput(sSpec);
const int MAXWORDS = 8;
string sWords[MAXWORDS];
int iC = 0, iWord = 0;
for (; iWord < MAXWORDS; !sWords[iWord].empty(), iWord++) // <=
{
sWords[iWord] = sInput.Tokenize("_", iC);
}
....
}
for (; iWord < MAXWORDS && !sWords[iWord].empty(); iWord++) {...}
void CHommingSwarmProjectile::HandleEvent(....)
{
....
explodeDesc.normal = -pCollision->n,pCollision->vloc[0];
....
}

Suspicious conditions

//! Find last single character.
// \return -1 if not found, distance from beginning otherwise.
template<class T>
inline typename CryStringT<T>::....::rfind(....) const
{
const_str str;
if (pos == npos)
{
// find last single character
str = _strrchr(m_str, ch);
// return -1 if not found, distance from beginning otherwise
return (str == NULL) ?
(size_type) - 1 : (size_type)(str - m_str);
}
else
{
if (pos == npos)
{
pos = length();
}
if (pos > length())
{
return npos;
}
value_type tmp = m_str[pos + 1];
m_str[pos + 1] = 0;
str = _strrchr(m_str, ch);
m_str[pos + 1] = tmp;
}
return (str == NULL) ?
(size_type) - 1 : (size_type)(str - m_str);
}
  • V571 Recurring check. The ‘if (pos == npos)’ condition was already verified in line 1262. CryFixedString.h 1271
bool CScriptTable::AddFunction(const SUserFunctionDesc& fd)
{
....
char sFuncSignature[256];
if (fd.sGlobalName[0] != 0)
cry_sprintf(sFuncSignature, "%s.%s(%s)", fd.sGlobalName,
fd.sFunctionName, fd.sFunctionParams);
else
cry_sprintf(sFuncSignature, "%s.%s(%s)", fd.sGlobalName,
fd.sFunctionName, fd.sFunctionParams);
....
}
  • V523 The ‘then’ statement is equivalent to the ‘else’ statement. BudgetingSystem.cpp 718
  • V523 The ‘then’ statement is equivalent to the ‘else’ statement. D3DShadows.cpp 627
  • V523 The ‘then’ statement is equivalent to the ‘else’ statement. livingentity.cpp 967

Undefined behavior

class CPhysicalEntity;
const int NO_GRID_REG = -1<<14;
const int GRID_REG_PENDING = NO_GRID_REG+1;
const int GRID_REG_LAST = NO_GRID_REG+2;
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘~(TFragSeqStorage(0))’ is negative. UDPDatagramSocket.cpp 757
  • V610 Undefined behavior. Check the shift operator ‘<<’. The right operand (‘cpu’ = [0..1023]) is greater than or equal to the length in bits of the promoted left operand. CryThreadUtil_posix.h 115
  • V610 Undefined behavior. Check the shift operator ‘>>’. The right operand is negative (‘comp’ = [-1..3]). ShaderComponents.cpp 399
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. trimesh.cpp 4126
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. trimesh.cpp 4559
  • V610 Unspecified behavior. Check the shift operator ‘>>’. The left operand ‘-NRAYS’ is negative. trimesh.cpp 4618
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. tetrlattice.cpp 324
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. tetrlattice.cpp 350
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. tetrlattice.cpp 617
  • V610 Undefined behavior. Check the shift operator ‘<<’. The left operand ‘-1’ is negative. tetrlattice.cpp 622
boolCOperatorQueue::Prepare(....)
{
++m_current &= 1;
m_ops[m_current].clear();
return true;
}
  • V567 Undefined behavior. The ‘m_commandBufferIndex’ variable is modified while being used twice between sequence points. XConsole.cpp 180
  • V567 Undefined behavior. The ‘itail’ variable is modified while being used twice between sequence points. trimesh.cpp 3119
  • V567 Undefined behavior. The ‘ihead’ variable is modified while being used twice between sequence points. trimesh.cpp 3126
  • V567 Undefined behavior. The ‘ivtx’ variable is modified while being used twice between sequence points. boolean3d.cpp 957
  • V567 Undefined behavior. The ‘ivtx’ variable is modified while being used twice between sequence points. boolean3d.cpp 965
  • V567 Undefined behavior. The ‘ivtx’ variable is modified while being used twice between sequence points. boolean3d.cpp 983
  • V567 Undefined behavior. The ‘m_iNextAnimIndex’ variable is modified while being used twice between sequence points. HitDeathReactionsDefs.cpp 192

Questions for the developers

void CNetContext::EnableBackgroundPassthrough(bool enable)
{
SCOPED_GLOBAL_LOCK;
// THIS IS A TEMPORARY HACK TO MAKE THE GAME PLAY NICELY,
// ASK peter@crytek WHY IT'S STILL HERE
enable = false;
....
}
....
// please ask me when you want to change [tetsuji]
....
// please ask me when you want to change [dejan]
....
//if there are problems with this function, ask Ivo
uint32 numAnims =
pCharacter->GetISkeletonAnim()->GetNumAnimsInFIFO(layer);
if (numAnims)
return pH->EndFunction(true);
....
//ask Ivo for details
//if (pCharacter->GetCurAnimation() &&
// pCharacter->GetCurAnimation()[0] != '\0')
// return pH->EndFunction(pCharacter->GetCurAnimation());
....
/////////////////////////////////////////////////////////////////
// Strange, !do not remove... ask Timur for the meaning of this.
/////////////////////////////////////////////////////////////////
if (m_nStrangeRatio > 32767)
{
gEnv->pScriptSystem->SetGCFrequency(-1); // lets get nasty.
}
/////////////////////////////////////////////////////////////////
// Strange, !do not remove... ask Timur for the meaning of this.
/////////////////////////////////////////////////////////////////
if (m_nStrangeRatio > 1000)
{
if (m_pProcess && (m_pProcess->GetFlags() & PROC_3DENGINE))
m_nStrangeRatio += cry_random(1, 11);
}
/////////////////////////////////////////////////////////////////
....
// tank specific:
// avoid steering input around 0.5 (ask Anton)
....
CryWarning(VALIDATOR_MODULE_EDITOR, VALIDATOR_WARNING,
"....: Wrong edited item. Ask AlexL to fix this.");
....
// If this renders black ask McJohn what's wrong.
glGenerateMipmap(GL_TEXTURE_2D);
....

Conclusion

--

--

--

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

Love podcasts or audiobooks? Learn on the go with our new app.

Recommended from Medium

Project: Build a Python GUI Calculator

Functional Calculator

JWT authentication and role-based authorization in Springboot

Technology and Management Risks that can cause Software Outsourcing to fail

Terraform and azure

Python tricks #1 List with condition

Arduino Tutorial Series: LED Blinking

What You Need to Know Before Choosing a Web Host

Backup is not dead, but it’s time to reinvent it.

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Unicorn Developer

Unicorn Developer

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

More from Medium

Dynamic Library In C

F#: Documenting Domain with Unions and Pattern Matching

MuditaOS: Will your alarm clock go off? Part I

Functional programming — Flow