期待已久的CryEngine V检查

2016年5月,德国游戏开发公司Crytek决定将其游戏引擎CryEngine V的源代码上传到Github。 该引擎是用C ++编写的,并立即引起了开源开发人员社区和PVS-Studio静态分析器开发人员团队的关注,他们定期扫描开源项目的代码以评估其质量。 许多视频游戏开发工作室使用不同版本的CryEngine创造了许多出色的游戏,现在该引擎已可供更多开发人员使用。 本文概述了PVS-Studio静态分析器在项目中发现的错误。

介绍

CryEngine是由德国公司Crytek于2002年开发的游戏引擎,最初用于第一人称射击游戏《 孤岛惊魂》 。 许多视频游戏开发工作室使用CryEngine的各种许可版本创建了许多出色的游戏: 孤岛惊魂孤岛危机熵宇宙蓝火星WarfaceHomefront:The RevolutionSniper:Ghost WarriorArmored WarfareEvolve ,还有许多其他。 2016年3月,Crytek宣布了其新引擎CryEngine V的发布日期,并在不久后将其源代码上传到了Github。

该项目的源代码已通过PVS-Studio静态分析器6.05版进行了检查。 此工具旨在检测C,C ++和C#程序源代码中的软件错误。 使用静态分析的唯一真实方法是定期在开发人员的计算机和构建服务器上扫描代码。 但是,为了演示PVS-Studio的诊断功能,我们对开源项目进行了一次性检查,然后撰写有关发现的错误的文章。 如果我们喜欢一个项目,我们可能会在几年后再次对其进行扫描。 实际上,此类重复检查与一次性检查相同,因为代码在这段时间内累积了很多更改。

为了进行检查,我们选择了一些简单受欢迎的项目以及读者通过电子邮件建议的项目。 这就是为什么CryEngine V绝对不是我们的分析仪所扫描的游戏引擎中的第一个游戏引擎。 我们已经检查过的其他引擎包括:

  • 虚幻引擎4(第一检查,第二检查,第三检查)
  • 检查戈多引擎
  • 认真检查引擎
  • 检查X射线引擎
  • Xenko引擎检查

我们还检查了CryEngine 3 SDK。

我想特别详细介绍一下虚幻引擎4引擎。 以该项目为例,我们可以详细演示在实际项目上使用静态分析的正确方式,包括从将分析仪集成到项目的阶段到清除警告到以下步骤的整个过程。零,随后控制新代码中的错误消除。 我们与Epic Games公司合作开发了关于Unreal Engine 4项目的工作,为此,我们的团队修复了该引擎的源代码中发现的所有缺陷,并与Epic Games就完成的工作写了一篇联合文章(该文章已发布在Unreal Engine上)。博客)。 Epic Games还购买了PVS-Studio许可证,以便能够自己维护代码质量。 我们也想与Crytek进行这种协作。

分析器报告结构

在本文中,我想回答一些有关警告和误报的常见问题,例如,“误报的比率是多少?”或“为什么在这么大的项目中很少出现错误? ?”

首先,将所有PVS-Studio警告分为三个严重级别: 级别包含最严重的警告,几乎可以肯定是真实错误,而级别包含最不严重的警告或极有可能是误报的警告。 请记住,错误代码并不能将它们牢固地绑定到特定的严重性级别:警告级别之间的分布在很大程度上取决于上下文。

这是“一般分析”模块的警告在CryEngine V项目的严重性级别之间分布的方式:

  • 高:576条警告;
  • 中:814条警告,
  • 低:2942警告。

图1以饼图的形式显示了警告在各个级别上的分布。

图1 —严重性级别之间的警告百分比分布

不可能在文章中包含所有警告说明和关联的代码片段。 我们的文章通常讨论10–40条评论的案例。 一些警告列为清单; 而且大多数都必须接受检查。 在最佳情况下,项目作者在我们通知他们之后,要求提供完整的分析报告以进行深入研究。 令人痛苦的事实是,在大多数情况下,仅高级警告的数量对于一篇文章来说绰绰有余,而CryEngine V也不例外。 图2显示了为此项目发出的高级警告的结构。

图2 —高级警告的结构

让我们仔细看看该图表的各个部分:

  • 文章中描述了(6%)-文章中引用了警告,并附带了代码片段和注释。
  • 以列表形式显示(46%)-警告以列表形式列出。 这些警告与已经讨论的某些错误所引用的模式相同,因此仅给出警告文本。
  • 误报率(8%)-为分析仪的未来改进,我们已经考虑了一定比例的误报率。
  • 其他(40%)-发出的所有其他警告。 这些警告包括我们必须省略的警告,以使文章不会变得太大,非关键警告,或者仅由开发团队成员才能评估其有效性的警告。 正如我们在虚幻引擎4上的工作经验所表明的那样,此类代码仍然“闻起来”,并且这些警告仍然得到修复。

分析结果

烦人的复制粘贴

V501′-‘运算符的左侧和右侧有相同的子表达式:q2.vz — q2.vz entitynode.cpp 93

 布尔 
CompareRotation(const Quat&q1,const Quat&q2,float epsilon)
{
返回(fabs_tpl(q1.vx-q2.vx)<= epsilon)
&&(fabs_tpl(q1.vy-q2.vy)<= epsilon)
&&(fabs_tpl(q2.vz-q2.vz)<= epsilon)// <=
&&(fabs_tpl(q1.w-q2.w)<= epsilon);
}

数字输入错误可能是一种最令人讨厌的错字。 在上面的函数中,分析器检测到可疑表达式(q2.vz-q2.vz) ,其中变量q1q2似乎混合在一起。

V501在||的左侧和右侧有相同的子表达式’(m_eTFSrc == eTF_BC6UH)’。 操作员。 第919章

  //! 纹理格式。 
枚举ETEX_Format:uint8
{
....
eTF_BC4U,//!<3Dc +。
eTF_BC4S,
eTF_BC5U,//!<3Dc。
eTF_BC5S,
eTF_BC6UH,
eTF_BC6SH,
eTF_BC7,
eTF_R9G9B9E5,
....
};
 布尔CTexture :: StreamPrepare(CImageFile * pIM) 
{
....
如果((m_eTFSrc == eTF_R9G9B9E5)||
(m_eTFSrc == eTF_BC6UH)|| // <=
(m_eTFSrc == eTF_BC6UH))// <=
{
m_cMinColor / = m_cMaxColor.a;
m_cMaxColor / = m_cMaxColor.a;
}
....
}

另一种错字处理常量的复制。 在这种情况下,将m_eTFSrc变量与eTF_BC6UH常数进行两次比较。 这些检查中的第二个必须将变量与其他常量进行比较,该常量的名称与复制的常量名称仅一个字符不同,例如eTF_BC6SH

还有两个类似的问题:

  • V501在’||’的左侧和右侧有相同的子表达式’(td.m_eTF == eTF_BC6UH)’ 操作员。 texture.cpp 1214
  • V501在“ |”的左侧和右侧有相同的子表达式“ geom_colltype_solid” 操作员。 附件管理器.cpp 1004

V517检测到使用’if(A){…} else if(A){…}’模式。 存在逻辑错误的可能性。 检查行:266,268。d3dhwshader.cpp 266

  int SD3DShader :: Release(EHWShaderClass eSHClass,int nSize) 
{
....
如果(eSHClass == eHWSC_Pixel)
return((ID3D11PixelShader *)pHandle)-> Release();
否则,如果(eSHClass == eHWSC_Vertex)
返回((ID3D11VertexShader *)pHandle)-> Release();
否则if(eSHClass == eHWSC_Geometry)// <=
return((ID3D11GeometryShader *)pHandle)-> Release(); // <=
否则if(eSHClass == eHWSC_Geometry)// <=
return((ID3D11GeometryShader *)pHandle)-> Release(); // <=
否则如果(eSHClass == eHWSC_Hull)
返回((ID3D11HullShader *)pHandle)-> Release();
否则,如果(eSHClass == eHWSC_Compute)
返回((ID3D11ComputeShader *)pHandle)-> Release();
否则(eSHClass == eHWSC_Domain)
返回((ID3D11DomainShader *)pHandle)-> Release()
....
}

这是一个条件语句级联的延迟复制示例,其中之一保持不变。

V517检测到使用’if(A){…} else if(A){…}’模式。 存在逻辑错误的可能性。 检查行:970,974。environmentalweapon.cpp 970

 无效CEnvironmentalWeapon :: UpdateDebugOutput()const 
{
....
const char * AttackStateName =“ None”;
if(m_currentAttackState&// <=
EAttackStateType_EnactingPrimaryAttack)// <=
{
AttackStateName =“主要攻击”;
}
否则if(m_currentAttackState&// <=
EAttackStateType_EnactingPrimaryAttack)// <=
{
AttackStateName =“带电投掷”;
}
....
}

在前面的示例中,至少有很小的机会会由于生成过多的代码片段副本而导致额外的情况,而程序员只是忘记了删除其中一项检查。 但是,在此代码中,由于相同的条件表达式, attackStateName变量将永远不会采用值“ Charged Throw”。

V519’BlendFactor [2]’变量连续两次被赋值。 也许这是一个错误。 检查行:1265、1266。ccrydxgldevicecontext.cpp 1266

 无效CCryDXGLDeviceContext :: 
OMGetBlendState(....,FLOAT BlendFactor [4],....)
{
CCryDXGLBlendState :: ToInterface(ppBlendState,m_spBlendState);
如果((* ppBlendState)!= NULL)
(* ppBlendState)-> AddRef();
BlendFactor [0] = m_auBlendFactor [0];
BlendFactor [1] = m_auBlendFactor [1];
BlendFactor [2] = m_auBlendFactor [2]; // <=
BlendFactor [2] = m_auBlendFactor [3]; // <=
* pSampleMask = m_uSampleMask;
}

在此函数中,元素索引中的错字会阻止索引值为 ‘3’的元素BlendFactor [3]填充值。 如果分析人员没有发现同一错误片段的两个以上副本,则该片段将只是许多有趣的错别字示例之一:

V519’m_auBlendFactor [2]’变量连续两次被赋值。 也许这是一个错误。 检查行:904、905。ccrydxgldevicecontext.cpp 905

 无效CCryDXGLDeviceContext :: 
OMSetBlendState(.... const FLOAT BlendFactor [4],....)
{
....
m_uSampleMask = SampleMask;
如果(BlendFactor == NULL)
{
m_auBlendFactor [0] = 1.0f;
m_auBlendFactor [1] = 1.0f;
m_auBlendFactor [2] = 1.0f; // <=
m_auBlendFactor [2] = 1.0f; // <=
}
其他
{
m_auBlendFactor [0] = BlendFactor [0];
m_auBlendFactor [1] = BlendFactor [1];
m_auBlendFactor [2] = BlendFactor [2]; // <=
m_auBlendFactor [2] = BlendFactor [3]; // <=
}
  m_pContext-> SetBlendColor(m_auBlendFactor [0], 
m_auBlendFactor [1],
m_auBlendFactor [2],
m_auBlendFactor [3]);
m_pContext-> SetSampleMask(m_uSampleMask);
....
}

这是该片段,其中索引为“ 3”的元素再次被跳过。 我什至甚至想了一会儿就有某种故意的模式,但是当我看到程序员试图在函数末尾访问m_auBlendFactor数组的所有四个元素时,这种想法很快就消失了。 看起来带有错字的相同代码被简单地复制到文件ccrydxgldevicecontext.cpp中

V523’then’语句等效于’else’语句。 d3dshadows.cpp 1410

 无效的CD3D9Renderer :: ConfigShadowTexgen(....) 
{
....
如果((pFr-> m_Flags&DLF_DIRECTIONAL)||
(!(pFr-> bUseHWShadowMap)&&!(pFr-> bHWPCFCompare)))
{
//线性阴影可用于任何类型的方向
//灯光和非硬件点光源
m_cEF.m_TempVecs [2] [Num] = 1.f /(pFr-> fFarDist);
}
其他
{
// hw点光源现在具有非线性深度
m_cEF.m_TempVecs [2] [Num] = 1.f /(pFr-> fFarDist);
}
....
}

为了完成复制粘贴部分,这是另一个有趣的错误。 无论条件表达式产生什么结果,值m_cEF.m_TempVecs [2] [Num]总是由相同的公式计算。 从周围的代码来看,索引是正确的:正是索引为“ 2”的元素必须填充一个值。 只是公式本身在每种情况下都是不同的,程序员忘记更改复制的代码。

初始化麻烦

V546类的成员由其自身初始化:’eConfigMax(eConfigMax)’。 粒子参数1013

  ParticleParams(): 
....
fSphericalApproximation(1.f),
fVolumeThickness(1.0f),
fSoundFXParam(1.f),
eConfigMax(eConfigMax.VeryHigh),// <=
fFadeAtViewCosAngle(0.f)
{}

分析器检测到潜在的错字,该错字导致将类别字段初始化为自己的值。

V603对象已创建,但未被使用。 如果要调用构造函数,则应使用’this-> SRenderingPassInfo :: SRenderingPassInfo(…。)’。 i3dengine.h 2589

  SRenderingPassInfo() 
:pShadowGenMask(空)
,nShadowSide(0)
,nShadowLod(0)
,nShadowFrustumId(0)
,m_bAuxWindow(0)
,m_nRenderStackLevel(0)
,m_eShadowMapRendering(static_cast (SHADOW_MAP_NONE))
,m_bCameraUnderWater(0)
,m_nRenderingFlags(0)
,m_fZoomFactor(0.0f)
,m_pCamera(NULL)
,m_nZoomInProgress(0)
,m_nZoomMode(0)
,m_pJobState(nullptr)
{
threadID nThreadID = 0;
gEnv-> pRenderer-> EF_Query(EFQ_MainThreadList,nThreadID);
m_nThreadID = static_cast (nThreadID);
m_nRenderFrameID = gEnv-> pRenderer-> GetFrameID();
m_nRenderMainFrameID = gEnv-> pRenderer-> GetFrameID(false);
}

SRenderingPassInfo(threadID id)
{
SRenderingPassInfo(); // <=
SetThreadID(id);
}

在此代码中,检测到对构造函数的错误使用。 程序员可能假设在另一个构造函数中以这种方式(没有参数)调用构造函数将初始化类字段,但是这种假设是错误的。

相反,将创建并立即销毁类型为SRenderingPassInfo的新的未命名对象。 因此,类字段将保持未初始化。 解决此错误的一种方法是创建一个单独的初始化函数,然后从不同的构造函数中调用它。

V688’m_cNewGeomMML’局部变量与一个类成员具有相同的名称,这可能导致混乱。 terrain_node.cpp 344

 无效CTerrainNode :: Init(....) 
{
....
m_nOriginX = m_nOriginY = 0; //扇区原点
m_nLastTimeUsed = 0; //基本上是上次渲染
  uint8 m_cNewGeomMML = m_cCurrGeomMML = m_cNewGeomMML_Min .... 
  m_pLeafData = 0; 
  m_nTreeLevel = 0; 
....
}

局部变量cNewGeomMML的名称与类字段的名称一致。 通常这不是错误,但是在这种特殊情况下,与如何初始化其他类字段相比,它确实看起来很奇怪。

V575’memset’函数处理’0’元素。 检查第三个论点。 crythreadutil_win32.h 294

  void EnableFloatExceptions(....) 
{
....
上下文ctx;
memset(&ctx,sizeof(ctx),0); // <=
....
}

此错误是一个非常有趣的错误。 调用memset()函数时,错误地交换了两个参数,这导致调用该函数填充0个字节。 这是函数原型:

  void * memset(void * ptr,int值,size_t num); 

该函数希望接收缓冲区大小作为第三个参数,而缓冲区要填充的值作为第二个参数。

固定版本:

  void EnableFloatExceptions(....) 
{
....
上下文ctx;
memset(&ctx,0,sizeof(ctx));
....
}

V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 command_buffer.cpp 62

 无效的CBuffer :: Execute() 
{
....
QuatT * pJointsTemp = static_cast (
alloca(m_state.m_jointCount * sizeof(QuatT)));
....
}

在项目代码的某些部分, alloca()函数用于为对象数组分配内存。 在上面的示例中,以这种方式分配内存时,不会为类QuatT的对象调用构造函数或析构函数。 此缺陷可能导致处理未初始化的变量和其他错误。

这是此类型其他缺陷的完整列表:

  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 command_buffer.cpp 67
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 姿态匹配.cpp 144
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 第280章
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 第282章
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 scriptbind_entity.cpp 6252
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 第1016章
  • V630’_alloca’函数用于为对象数组分配内存,这些对象数组是包含构造函数的类。 driverd3d.cpp 5859

V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:-1.8f。 posealignerc3.cpp 330

  ILINE bool InitializePoseAlignerPinger(....) 
{
....
chainDesc.offsetMin = Vec3(0.0f,0.0f,bIsMP?-1.8f:-1.8f);
chainDesc.offsetMax = Vec3(0.0f,0.0f,bIsMP?+ 0.75f:+ 1.f);
....
}

发现了三元运算符?:返回一个相同值的片段。 尽管在前面的示例中可能出于美学原因完成了此操作,但在以下片段中这样做的原因尚不清楚。

 浮动predictDelta = inputSpeed <0.0f?  0.1f:0.1f;  // <= 
float dict =角度+预言Delta *(angle-m_prevAngle)/ dt;

此类型其他缺陷的完整列表:

  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:-1.8f。 posealignerc3.cpp 313
  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:-2.f。 posealignerc3.cpp 347
  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:D3D11_RTV_DIMENSION_TEXTURE2DARRAY。 d3dtexture.cpp 2277
  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:255U。 renderer.cpp 3389
  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:D3D12_RESOURCE_STATE_GENERIC_READ。 dx12device.cpp 151
  • V583’?:’运算符,无论其条件表达式如何,始终返回一个相同的值:0.1f。 vehiclemovementstdboat.cpp 720

V570将’runtimeData.entityId’变量分配给它自己。 behaviortreenodes_ai.cpp 1771

 无效ExecuteEnterScript(RuntimeData&runtimeData) 
{
ExecuteScript(m_enterScriptFunction,runtimeData.entityId);
  runtimeData.entityId = runtimeData.entityId;  // <= 
runtimeData.executeExitScriptIfDestructed = true;
}

变量被分配给它自己,看起来不正确。 作者应检查此代码。

操作优先

V502也许’?:’运算符的工作方式与预期的不同。 ‘?:’运算符的优先级低于’+’运算符。 gpuparticlefeaturespawn.cpp 79

  bool HasDuration(){return m_useDuration;  } 
 无效CFeatureSpawnRate :: SpawnParticles(....) 
{
....
SSpawnData&spawn = pRuntime-> GetSpawnData(i);
const float amount = spawn.amount;
const int spawnedBefore = int(spawn.spawned);
const float endTime = spawn.delay +
HasDuration()吗? spawn.duration:fHUGE;
....
}

上面的函数似乎以错误的方式测量时间。 加法运算符的优先级高于三元运算符😕的优先级,因此将值01首先添加到spawn.delay中 ,然后将值spawn.durationfHUGE写入 endTime变量中。 此错误是很常见的错误。 要了解有关从PVS-Studio错误数据库收集的涉及操作优先级的有趣错误模式的更多信息,请参阅我的文章:C / C ++中的逻辑表达式。 专业人士犯的错误。

V634’*’操作的优先级高于'<<'操作的优先级。 表达式中可能应使用括号。 型号cpp 336

 枚举joint_flags 
{
angle0_locked = 1,
....
};
 布尔CDefaultSkeleton :: SetupPhysicalProxies(....) 
{
....
for(int j = 0; ....; j ++)
{
//锁定0个极限范围的轴
m_arrModelJoints [i] ....标志| =(....)* angle0_locked << j;
}
....
}

这是另一个非常有趣的错误,与乘法和按位移位运算的优先级有关。 后者的优先级较低,因此在每次迭代时整个表达式都乘以1(因为angle0_locked常数的值为1),这看起来很奇怪。

这是程序员必须希望代码看起来像的样子:

  m_arrModelJoints [i] ....标志| =(....)*(angle0_locked << j); 

以下文件包含35个涉及移位操作优先级的可疑片段的列表:CryEngine5_V634.txt。

未定义的行为

未定义行为是执行以某种编程语言编写的计算机代码的结果,该代码取决于多种随机因素,例如内存状态或触发的中断。 换句话说,该结果不是语言规范所规定的。 在您的程序中发生这种情况被认为是错误。 即使它可以在某些编译器上成功执行,也不能保证它是跨平台的,并且可能在另一台计算机,操作系统甚至同一编译器的其他设置上失败。

V610未定义的行为。 检查移位运算符“ <<”。 左操作数“ -1”为负。 物理占位符.h 25

  #ifndef physicalplaceholder_h 
#define physicalplaceholder_h
#pragma一次
....
const int NO_GRID_REG = -1 << 14;
const int GRID_REG_PENDING = NO_GRID_REG + 1;
....

在现代C ++标准下,负值的左移是未定义的行为。 分析器在CryEngine V的代码中发现了更多类似的问题:

  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数'〜(TFragSeqStorage(0))'为负。 udpdatagramsocket.cpp 757
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数“ -1”为负。 tetrlattice.cpp 324
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数“ -1”为负。 tetrlattice.cpp 350
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数“ -1”为负。 tetrlattice.cpp 617
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数“ -1”为负。 tetrlattice.cpp 622
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数'(〜(0xF))'为负。 d3ddeferredrender.cpp 876
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数'(〜(0xF))'为负。 d3ddeferredshading.cpp 791
  • V610未定义的行为。 检查移位运算符“ <<”。 左操作数'(〜(1 << 0))'为负。 d3dsprites.cpp 1038

V567未定义的行为。 修改“ m_current”变量,同时在序列点之间使用两次。 105页

 布尔COperatorQueue :: Prepare(....) 
{
++ m_current&= 1;
m_ops [m_current] .clear();
返回true;
}

分析器检测到导致未定义行为的表达式。 变量在两个序列点之间多次使用,而其值会更改。 因此,无法确定执行该表达式的结果。

其他类似问题:

  • V567未定义的行为。 在序列点之间使用两次“ itail”变量时会对其进行修改。 trimesh.cpp 3101
  • V567未定义的行为。 修改“ ihead”变量,同时在序列点之间使用两次。 trimesh.cpp 3108
  • V567未定义的行为。 修改“ ivtx”变量,同时在序列点之间使用两次。 boolean3d.cpp 1194
  • V567未定义的行为。 修改“ ivtx”变量,同时在序列点之间使用两次。 boolean3d.cpp 1202
  • V567未定义的行为。 修改“ ivtx”变量,同时在序列点之间使用两次。 boolean3d.cpp 1220
  • V567未定义的行为。 在序列点之间两次使用’m_commandBufferIndex’变量时进行了修改。 xconsole.cpp 180
  • V567未定义的行为。 在序列点之间两次使用’m_FrameFenceCursor’变量时进行了修改。 ccrydx12devicecontext.cpp 952
  • V567未定义的行为。 在序列点之间两次使用’m_iNextAnimIndex’变量时会对其进行修改。 hitdeathreactionsdefs.cpp 192

条件错误

V579 memcmp函数接收指针及其大小作为参数。 这可能是一个错误。 检查第三个论点。 graphicspipelinestateset.h 58

 布尔 
运算符==(const SComputePipelineStateDescription&other)const
{
返回0 == memcmp(this,&other,sizeof(this)); // <=
}

程序员在调用memcmp()函数时在相等运算中犯了一个错误,这导致传递指针大小而不是对象大小作为函数参数。 结果,仅比较对象的前几个字节。

固定版本:

  memcmp(this,&other,sizeof(* this)); 

不幸的是,在项目中又发现了三个类似的问题:

  • V579 memcpy函数接收指针及其大小作为参数。 这可能是一个错误。 检查第三个论点。 geomcacherendernode.cpp 286
  • V579 AddObject函数接收指针及其大小作为参数。 这可能是一个错误。 检查第二个参数。 clipvolumemanager.cpp 145
  • V579 memcmp函数接收指针及其大小作为参数。 这可能是一个错误。 检查第三个论点。 graphicspipelinestateset.h 34

V640代码的操作逻辑与其格式不符。 第二条语句将始终被执行。 大括号可能丢失。 第181章

  CLivingEntity ::〜CLivingEntity() 
{
for(int i = 0; i <m_nParts; i ++){
如果(!m_parts [i] .pPhysGeom || ....)
delete [] m_parts [i] .pMatMapping; m_parts [i] .pMatMapping = 0;
}
....
}

我发现了很多代码块,其中的语句用一行编写。 这些不仅包括普通的分配,还包括循环,条件,函数调用,有时还包括所有这些的混合体(参见图3)。

图3 —不良的代码格式

在这样大小的代码中,这种编程风格几乎不可避免地会导致错误。 在上面的示例中,当满足特定条件时,将释放由对象数组占用的内存块,并清除指针。 但是,错误的代码格式会导致在每次循环迭代时清除m_parts [i] .pMatMapping指针。 这个问题的含义无法预料,但是代码确实看起来很奇怪。

其他格式奇怪的片段:

  • V640代码的操作逻辑与其格式不符。 第二条语句将始终被执行。 大括号可能丢失。 physicalworld.cpp 2449
  • V640代码的操作逻辑与其格式不符。 第二条语句将始终被执行。 大括号可能丢失。 第1723章
  • V640代码的操作逻辑与其格式不符。 第二条语句将始终被执行。 大括号可能丢失。 第1726章

在条件表达式中V695范围交点是可能的。 示例:if(A <5){…}否则if(A <2){…}。 检查行:538、540。statobjrend.cpp 540

 布尔CStatObj :: RenderDebugInfo(....) 
{
....
ColorB clr(0,0,0,0);
如果(nRenderMats == 1)
clr = ColorB(0,0,255,255);
否则,如果(nRenderMats == 2)
clr = ColorB(0,255,255,255);
否则,如果(nRenderMats == 3)
clr = ColorB(0,255,0,255);
否则,如果(nRenderMats == 4)
clr = ColorB(255,0,255,255);
否则(nRenderMats == 5)
clr = ColorB(255,255,0,255);
否则if(nRenderMats> = 6)// <=
clr = ColorB(255,0,0,255);
否则if(nRenderMats> = 11)// <=
clr = ColorB(255,255,255,255);
....
}

程序员犯了一个错误,即阻止选择颜色ColorB(255,255,255,255) 。 首先将nRenderMats值与1到5的数字进行一次比较,但是当将它们与值范围进行比较时,程序员没有考虑到大于11的值已经属于大于6的值的范围,因此最后一个条件将永远不会执行。

这一系列条件被完全复制为另一个片段:

  • 在条件表达式中V695范围交点是可能的。 示例:if(A <5){…}否则if(A <2){…}。 检查行:338,340。modelmesh_debugpc.cpp 340

在条件表达式中V695范围交点是可能的。 示例:if(A <5){…}否则if(A <2){…}。 检查行:393、399。xmlcpb_nodelivewriter.cpp 399

 枚举eNodeConstants 
{
....
CHILDBLOCKS_MAX_DIST_FOR_8BITS = BIT(7)-1,// 127
CHILDBLOCKS_MAX_DIST_FOR_16BITS = BIT(6)-1,// 63
....
};
 无效的CNodeLiveWriter :: Compact() 
{
....
if(dist <= CHILDBLOCKS_MAX_DIST_FOR_8BITS)// dist <= 127
{
uint8 byteDist = dist;
writeBuffer.AddData(&byteDist,sizeof(byteDist));
isChildBlockSaved = true;
}
否则if(dist <= CHILDBLOCKS_MAX_DIST_FOR_16BITS)// dist <= 63
{
uint8 byteHigh = CHILDBLOCKS_USING_MORE_THAN_8BITS | ....);
uint8 byteLow = dist&255;
writeBuffer.AddData(&byteHigh,sizeof(byteHigh));
writeBuffer.AddData(&byteLow,sizeof(byteLow));
isChildBlockSaved = true;
}
....
}

在上面的片段中也发现了一个条件内的类似错误,只是这次无法控制的代码更大。 常量CHILDBLOCKS_MAX_DIST_FOR_8BITSCHILDBLOCKS_MAX_DIST_FOR_16BITS的值使得第二个条件永远不会为真。

V547表达式’pszScript [iSrcBufPos]!=’==”始终为true。 字符类型的值范围:[-128、127]。 luadbg.cpp 716

  bool CLUADbg :: LoadFile(const char * pszFile,bool bForceReload) 
{
FILE * hFile = NULL;
char * pszScript = NULL,* pszFormattedScript = NULL;
....
而(pszScript [iSrcBufPos]!=''&&
....
pszScript [iSrcBufPos]!='='&&
pszScript [iSrcBufPos]!='=='&& // <=
pszScript [iSrcBufPos]!='*'&&
pszScript [iSrcBufPos]!='+'&&
pszScript [iSrcBufPos]!='/'&&
pszScript [iSrcBufPos]!='〜'&&
pszScript [iSrcBufPos]!='“')
{}
....
}

大条件表达式包含始终为真的子表达式。 ‘==’文字的类型将为int并对应于值15677。pszScript数组由char类型的元素组成,并且char类型的值不能等于15677,因此pszScript [iSrcBufPos]!=’ ==’表达式始终为真。

V734表达式过多。 检查子字符串“ _ddn”和“ _ddna”。 texture.cpp 4212

 无效CTexture :: PrepareLowResSystemCopy(byte * pTexData,....) 
{
....
//确保我们跳过非扩散纹理
如果(strstr(GetName(),“ _ddn”)// // =
|| strstr(GetName(),“ _ddna”)// <=
|| strstr(GetName(),“ _mask”)
|| strstr(GetName(),“ _spec。”)
|| strstr(GetName(),“ _gloss”)
|| strstr(GetName(),“ _displ”)
|| strstr(GetName(),“字符”)
|| strstr(GetName(),“ $”)

返回;
....
}

strstr()函数在另一个字符串中查找指定子字符串的第一次出现,并返回指向第一次出现的指针或空指针。 字符串“ _ddn”是第一个要搜索的字符串,“ _ ddna”是第二个要搜索的字符串,这意味着如果找到较短的字符串,则条件为true。 此代码可能无法按预期方式工作; 或者此表达式是多余的,可以通过删除多余的检查来简化。

V590考虑检查此表达式。 表达式过多或打印错误。 Goalop_crysis2.cpp 3779

 无效COPCrysis2FlightFireWeapons :: ParseParam(....) 
{
....
否则如果(!已暂停&&
(m_State == eFP_PAUSED)&& // <=
(m_State!= eFP_PAUSED_OVERRIDE))// <=
....
}

ParseParam()函数中的条件表达式的编写方式使其结果不依赖于( m_State!= eFP_PAUSED_OVERRIDE )子表达式。

这是一个更简单的示例:

 如果(err == code1 && err!= code2) 
{
....
}

整个条件表达式的结果不依赖于(err!= code2)子表达式的结果,对于该示例,可以从真值表中清楚地看到它(请参见图4)。

图4 —逻辑表达式的真值表

将无符号值与零进行比较

扫描项目时,我们经常会遇到无符号值与零的比较,每次比较都会产生truefalse 。 这样的代码并不总是包含严重的错误。 这通常是由于过于谨慎或将变量的类型从有符号的更改为无符号的结果。 无论如何,这种比较需要检查。

V547表达式’m_socket <0'始终为false。 无符号类型的值永远不会小于0。servicenetwork.cpp 585

  typedef SOCKET CRYSOCKET; 
//内部套接字数据
CRYSOCKET m_socket;
 布尔CServiceNetworkConnection :: TryReconnect() 
{
....
//根据需要创建新的套接字
如果(m_socket == 0)
{
m_socket = CrySock :: socketinet();
如果(m_socket <0)
{
....
返回false;
}
}
....
}

我想详细说明一下SOCKET类型。 根据平台的不同,它可以是有符号的也可以是无符号的,因此强烈建议您在使用此类型时使用标准标头中指定的特殊宏和常量。

在跨平台项目中,通常使用0或-1进行比较,这会导致错误代码的误解。 尽管某些检查正确完成,但CryEngine V项目也不例外,例如:

 如果(m_socket == CRY_INVALID_SOCKET) 

但是,代码的许多部分使用了这些检查的不同版本。

参见文件CryEngine5_V547.txt,了解其他47个零无符号变量的可疑比较。 代码作者需要检查这些警告。

危险的指针

诊断V595会在取消引用后检测经过测试为空的指针。 实际上,此诊断程序会捕获非常棘手的错误。 在极少数情况下,它会发出误报,这可以通过以下事实进行解释:间接检查指针(即通过一个或多个其他变量)来检查指针,但是弄清楚这样的代码对人类来说也不是一件容易的事,不是吗? 下面给出了三个代码示例,它们触发了该诊断,并且看起来特别令人惊讶,因为尚不清楚它们为什么起作用。 有关此类型的其他警告,请参见文件CryEngine5_V595.txt。

例子1

V595在针对nullptr对其进行验证之前,已使用了’m_pPartManager’指针。 检查行:1441,1442。3denginerender.cpp 1441

 无效C3DEngine :: RenderInternal(....) 
{
....
m_pPartManager-> GetLightProfileCounts()。ResetFrameTicks();
如果(passInfo.IsGeneralPass()&& m_pPartManager)
m_pPartManager-> Update();
....
}

m_pPartManager指针被取消引用,然后检查。

例子2

V595在针对nullptr对其进行验证之前,已使用’gEnv-> p3DEngine’指针。 检查行:1477、1480。gameserialize.cpp 1477

 布尔CGameSerialize :: LoadLevel(....) 
{
....
//可以快速加载
如果(!gEnv-> p3DEngine-> RestoreTerrainFromDisk())
返回false;
 如果(gEnv-> p3DEngine) 
{
gEnv-> p3DEngine-> ResetPostEffects();
}
....
}

gEnv-> p3DEngine指针取消引用,然后进行检查。

例子3

V595在针对nullptr进行验证之前,已使用了’pSpline’指针。 检查行:158,161。facechannelkeycleanup.cpp 158

 无效FaceChannel :: CleanupKeys(....) 
{
  CFacialAnimChannelInterpolator backupSpline(* pSpline); 
  //创建键条目数组。 
int numKeys =(pSpline?pSpline-> num_keys():0);
....
}

pSpline指针被取消引用,然后检查。

V622考虑检查“ switch”语句。 第一个“ case”运算符可能会丢失。 合并后的meshrendernode.cpp 999

 静态内联void ExtractSphereSet(....) 
{
....
开关(statusPos.pGeom-> GetType())
{
如果(假)
{
案例GEOM_CAPSULE:
statusPos.pGeom-> GetPrimitive(0,&cylinder);
}
如果(假)
{
案例GEOM_CYLINDER:
statusPos.pGeom-> GetPrimitive(0,&cylinder);
}
对于(int i = 0; i <2 && ....; ++ i)
{
....
}
打破;
....
}

这个片段可能是CryEngine V中最奇怪的片段。是否选择大小写标签并不取决于if语句,即使if(false)也不例外。 在switch语句中,如果满足switch语句的条件,则会无条件跳转到标签。 如果没有break语句,则可以使用这样的代码来“绕过”不相关的语句,但是再次,要维护这样晦涩的代码并不容易。 另一个问题是,为什么跳转到标签GEOM_CAPSULEGEOM_CYLINDER时会执行相同的代码?

V510’LogError’函数不应接收类类型的变量作为第二个实际参数。 behaviortreenodes_action.cpp 143

  typedef CryStringT 字符串; 
//实际的片段名称。
字符串m_fragName;
//! 转换为C字符串。
const value_type * c_str()const {return m_str; }
const value_type * data()const {return m_str; };

无效LogError(const char * format,...)const
{....}

无效QueueAction(const UpdateContext&context)
{
....
ErrorReporter(* this,context).LogError(“ ....'%s'”,m_fragName);
....
}

如果无法为一个函数指定所有可接受参数的数量和类型,则在函数声明中将省略号(…)放在参数列表的末尾,这意味着“可能还有更多”。 仅POD(普通旧数据)类型可以用作省略号的实际参数。 如果将类的对象作为参数传递给函数的省略号,则它几乎总是表示存在错误。 在上面的代码中,到达栈的是对象的内容,而不是字符串的指针。 这样的代码导致在缓冲区中形成“垃圾”或崩溃。 CryEngine V的代码使用自己的字符串类,并且已经具有适当的方法c_str()

固定版本:

  LogError(“ ....'%s'”,m_fragName.c_str(); 

其他一些可疑片段:

  • V510’LogError’函数不应接收类类型的变量作为第二个实际参数。 behaviortreenodes_core.cpp 1339
  • V510“格式”功能不应接收类类型的变量作为第二个实际参数。 behaviortreenodes_core.cpp 2648
  • V510’CryWarning’函数不应接收类类型的变量作为第六个实际参数。 crypak.cpp 3324
  • V510’CryWarning’函数不应接收类类型的变量作为第五个实际参数。 crypak.cpp 3333
  • V510’CryWarning’函数不应接收类类型的变量作为第五个实际参数。 shaderfxparsebin.cpp 4864
  • V510’CryWarning’函数不应接收类类型的变量作为第五个实际参数。 shaderfxparsebin.cpp 4931
  • V510“格式”功能不应接收类类型的变量作为第三个实际参数。 featuretester.cpp 1727

V529奇数分号’;’ 在“ for”运算符之后。 boolean3d.cpp 1314

  int CTriMesh :: Slice(....) 
{
....
bop_meshupdate * pmd =新的bop_meshupdate,* pmd0;
pmd-> pMesh [0] = pmd-> pMesh [1] = this; AddRef(); AddRef();
for(pmd0 = m_pMeshUpdate; pmd0-> next; pmd0 = pmd0-> next); // <=
pmd0-> next = pmd;
....
}

这段代码很奇怪。 程序员在for循环后放置一个分号,而代码格式表明它应该有一个主体。

V535变量“ j”被用于该循环和外部循环。 检查行:3447、3490。physicalworld.cpp 3490

  CPhysicalWorld :: SimulateExplosion(....) 
{
....
for(j = 0; j nIslands; j ++)// <=第3447行
{
....
for(j = 0; j <pcontacts [ncont] .nborderpt; j ++)// <=第3490行
{
....
}

该项目的代码充满了其他不安全的片段; 例如,在某些情况下,嵌套循环和外部循环都使用一个计数器。 大型源文件包含格式复杂的代码和片段,其中在代码的不同部分更改了相同的变量-您不能在那里没有静态分析!

还有一些奇怪的循环:

  • V535变量“ i”用于该循环和外部循环。 检查行:1630、1683。entity.cpp 1683
  • V535变量“ i1”用于该循环和外部循环。 检查行:1521、1576。softentity.cpp 1576
  • V535变量“ i”用于该循环和外部循环。 检查行:2315、2316。physicalentity.cpp 2316
  • V535变量“ i”用于该循环和外部循环。 检查行:1288、1303。shadercache.cpp 1303

V539考虑检查作为参数传递给函数“ erase”的迭代器。 frameprofilerender.cpp 1090

 浮动CFrameProfileSystem :: RenderPeaks() 
{
....
std :: vector &rPeaks = m_peaks;

//穿越所有高峰。
对于(int i = 0; i <(int)rPeaks.size(); i ++)
{
....
如果(年龄> fHotToColdTime)
{
rPeaks.erase(m_peaks.begin()+ i); // <=
一世 - ;
}
....
}

分析器怀疑处理容器的函数将从另一个容器接收迭代器。 这是一个错误的假设,这里没有错误: rPeaks变量是对m_peaks的引用。 但是,此代码不仅会使分析器产生混淆,还会使其他维护者迷惑。 不应以这种方式编写代码。

V713在针对同一逻辑表达式中的nullptr进行验证之前,在逻辑表达式中使用了指针pCollision。 动作游戏cpp 4235

  int CActionGame :: OnCollisionImmediate(const EventPhys * pEvent) 
{
....
否则(pMat-> GetBreakability()== 2 &&
pCollision-> idmat [0]!= pCollision-> idmat [1] &&
(能源= pMat-> GetBreakEnergy())> 0 &&
pCollision-> mass [0] * 2>能量&&
....
pMat-> GetHitpoints()<= FtoI(min(1E6f,能量/能量))&&
pCollision)// <=
返回0;
....
}

if语句包含一个相当长的条件表达式,其中pCollision指针被多次使用。 该代码的错误之处在于,在最后(即多次取消引用操作之后)测试了该指针是否为null。

V758当函数返回的智能指针被破坏时,“ commandList”引用无效。 renderitemdrawer.cpp 274

  typedef std :: shared_ptr  CDeviceGraphicsCommandListPtr; 
  CDeviceGraphicsCommandListPtr 
CDeviceObjectFactory :: GetCoreGraphicsCommandList()常量
{
返回m_pCoreCommandList;
}
 无效CRenderItemDrawer :: DrawCompiledRenderItems(....)const 
{
....
{
auto&RESTRICT_REFERENCE commandList = * CCryDeviceWrapper ::
GetObjectFactory()。GetCoreGraphicsCommandList();
  passContext ....-> PrepareRenderPassForUse(commandList); 
}
....
}

commandList变量接收对存储在智能指针中的值的引用。 当该指针破坏对象时,引用将变为无效。

此类型的一些其他问题:

  • V758当函数返回的智能指针被破坏时,“ commandList”引用无效。 renderitemdrawer.cpp 384
  • V758当函数返回的智能指针被破坏时,“ commandList”引用无效。 renderitemdrawer.cpp 368
  • V758当函数返回的智能指针被破坏时,“ commandList”引用无效。 renderitemdrawer.cpp 485
  • V758当函数返回的智能指针被破坏时,“ commandList”引用无效。 renderitemdrawer.cpp 553

结论

修复在编码阶段捕获的错误几乎不需要花什么钱,而与测试人员相比,要花费几乎任何钱,而修复最终用户所遇到的错误则需要花费大量费用。 无论您使用哪种分析仪,静态分析技术本身早已被证明是一种控制程序代码和软件产品质量的极其有效的方法。

我们与Epic Games的合作很好地展示了将分析仪集成到Unreal Engine 4中如何使该项目受益。 我们在分析仪集成的各个方面帮助开发人员,甚至修复了项目中发现的错误,以便开发人员团队可以继续定期自行扫描新代码。 我们想与Crytek尝试这种合作。

欢迎在您的C / C ++ / C#项目上尝试PVS-Studio。

由Svyatoslav Razmyslov