PVS-Studio瞥了一眼Red Dead Redemption引擎-Bullet

图片4

如今,例如,在开发游戏时,不再需要从头开始独立实现物体的物理原理,因为为此有大量的库。 Bullet曾经在许多AAA游戏,虚拟现实项目,各种模拟和机器学习中得到积极使用。 是的,它现在仍在使用,例如,它是Red Dead Redemption和Red Dead Redemption 2引擎之一,所以为什么不使用PVS-Studio检查Bullet看看静态分析可以在如此大规模的项目中检测到哪些错误,与物理模拟有关。

该库是免费分发的 ,因此每个人都可以选择在其项目中使用它。 除《荒野大镖客:救赎》外,该物理引擎还用于电影行业中以创建特殊效果。 例如,他参与了盖·里奇(Guy Ritchie)拍摄的《福尔摩斯探案》(Sherlock Holmes),以计算碰撞。

如果这是您第一次接触PVS-Studio检查项目的文章,那么我将做一点题外话。 PVS-Studio是静态代码分析器,可以帮助您在C,C ++,C#和Java程序的源代码中查找错误,缺点和潜在漏洞。 静态分析是一种自动的代码检查过程。

热身


范例1:

让我们从一个有趣的错误开始:

V624'3.141592538 '常量中可能存在打印错误。 考虑使用<math.h>中的M_PI常量。 PhysicsClientC_API.cpp 4109

B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... } 

数字Pi(3.141592653 ...)的一个小错字,在小数部分的第7个位置缺少数字“ 6”。

图片1
也许小数点后第1百万位的错误不会导致明显的后果,但是您仍然应该使用现有的库常量而不会出现错别字。 对于Pi,在math.h标头中有一个M_PI常量。

复制粘贴


范例2:

有时,分析仪可以让您间接找到错误。 因此,例如,此处将三个相关的参数HalfExtentsX,halfExtentsY,halfExtentsZ传递给函数,但后者未在函数中的任何地方使用。 您可能会注意到,在调用addVertex方法时,变量halfExtentsY被使用了两次。 因此,也许这是一个复制粘贴错误,在这里应该使用一个被遗忘的参数。

V751参数“ halfExtentsZ”未在功能体内使用。 TinyRenderer.cpp 375

 void TinyRenderObjectData::createCube(float halfExtentsX, float halfExtentsY, float halfExtentsZ, ....) { .... m_model->addVertex(halfExtentsX * cube_vertices_textured[i * 9], halfExtentsY * cube_vertices_textured[i * 9 + 1], halfExtentsY * cube_vertices_textured[i * 9 + 2], cube_vertices_textured[i * 9 + 4], ....); .... } 

范例3:

分析器还发现了以下有趣的片段,我将首先以其原始形式进行介绍。

图片11

看到最后一行?

图片12

程序员决定将这么长的条件写在一行中是很奇怪的。 但是,错误很可能蔓延到错误的事实一点也不奇怪。

分析仪在此行上发出以下警告。

V501在'&&'运算符的左侧和右侧有相同的子表达式'rotmat.Column1()。Norm()<1.0001'。 线性R4.cpp 351

V501在&&运算符的左侧和右侧有相同的子表达式'0.9999 <rotmat.Column1()。Norm()'。 线性R4.cpp 351

如果我们以可视化的“表格”形式编写所有内容,那么很显然,对Column1也会应用相同的检查。 最后两个比较显示有Column1Column2 。 最有可能的是,第三次和第四次比较应该已经检查了Column2的值。

  Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() && Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() &&(Column1() ^ Column2()) < 0.001 && (Column1() ^ Column2()) > -0.001 

以这种形式,相同的比较变得更加明显。

范例4:

类似错误:

V501在'&&'运算符的左侧和右侧有相同的子表达式'cs.m_fJacCoeffInv [0] == 0'。 b3CpuRigidBodyPipeline.cpp 169

 float m_fJacCoeffInv[2]; static inline void b3SolveFriction(b3ContactConstraint4& cs, ....) { if (cs.m_fJacCoeffInv[0] == 0 && cs.m_fJacCoeffInv[0] == 0) { return; } .... } 

在这种情况下,将对同一数组元素进行两次检查。 该情况最有可能看起来像这样: cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 。 这是复制粘贴错误的经典示例。

范例5:

发现另一个缺点:

V517检测到使用'if(A){...} else if(A){...}'模式。 存在逻辑错误的可能性。 检查行:79、112。main.cpp 79

 int main(int argc, char* argv[]) { .... while (serviceResult > 0) { serviceResult = enet_host_service(client, &event, 0); if (serviceResult > 0) { .... } else if (serviceResult > 0) { puts("Error with servicing the client"); exit(EXIT_FAILURE); } .... } .... } 

enet_host_service函数的结果分配给serviceResult ,如果成功 ,则返回1,如果失败,则返回-1。 最可能的是, else if分支应该对serviceResult的负值作出响应,但是验证条件是重复的。 这很可能也是复制粘贴错误。

类似的分析器警告,在本文中对其进行描述是没有意义的:

V517检测到使用'if(A){...} else if(A){...}'模式。 存在逻辑错误的可能性。 检查行:151、190。PhysicsClientUDP.cpp 151

超越极限:超越数组的界限


范例6:

寻找错误的令人不快的事情之一就是走出阵列。 通常由于循环中的复杂索引而发生此错误。

在此,在循环条件下, dofIndex变量从上方限制为128, dof限制为4(含)。 但是m_desiredState也仅包含128个元素。 结果,索引[dofIndex + dof]可能导致数组流出

V557阵列可能超限。 “ dofIndex + dof”索引的值可以达到130。PhysicsClientC_API.cpp 968

 #define MAX_DEGREE_OF_FREEDOM 128 double m_desiredState[MAX_DEGREE_OF_FREEDOM]; B3_SHARED_API int b3JointControl(int dofIndex, double* forces, int dofCount, ....) { .... if ( (dofIndex >= 0) && (dofIndex < MAX_DEGREE_OF_FREEDOM ) && dofCount >= 0 && dofCount <= 4) { for (int dof = 0; dof < dofCount; dof++) { command->m_sendState.m_desiredState[dofIndex+dof] = forces[dof]; .... } } .... } 

范例7:

一个类似的错误,但是现在求和不是在索引数组时而是在某种情况下导致的。 如果文件名越长越好,那么终端零将被写到数组之外( “一失一错” )。 自然,仅在特殊情况下len变量将等于MAX_FILENAME_LENGTH ,但这不能消除错误,只是使错误很少发生。

V557阵列可能超限。 “ len”索引的值可能达到1024。PhysicsClientC_API.cpp 5223

 #define MAX_FILENAME_LENGTH MAX_URDF_FILENAME_LENGTH 1024 struct b3Profile { char m_name[MAX_FILENAME_LENGTH]; int m_durationInMicroSeconds; }; int len = strlen(name); if (len >= 0 && len < (MAX_FILENAME_LENGTH + 1)) { command->m_type = CMD_PROFILE_TIMING; strcpy(command->m_profile.m_name, name); command->m_profile.m_name[len] = 0; } 

测量一次-切七次


范例8:

如果您需要多次使用某个函数的结果或反复使用需要通过一系列调用访问的变量,则应使用临时变量来优化和更好地阅读代码。 分析仪在代码中发现了100多个可以进行这种校正的位置。

V807性能下降 。 考虑创建一个指针,以避免重复使用'm_app-> m_renderer-> getActiveCamera()'表达式。 InverseKinematicsExample.cpp 315

 virtual void resetCamera() { .... if (....) { m_app->m_renderer->getActiveCamera()->setCameraDistance(dist); m_app->m_renderer->getActiveCamera()->setCameraPitch(pitch); m_app->m_renderer->getActiveCamera()->setCameraYaw(yaw); m_app->m_renderer->getActiveCamera()->setCameraPosition(....); } } 

在这里,相同的调用链被重用,可以用单个指针替换。

范例9:

V810性能下降 。 使用相同的参数多次调用了“ btCos(euler_out.pitch)”函数。 结果可能应该保存到一个临时变量中,然后可以在调用“ btAtan2”函数时使用它。 btMatrix3x3.h 576

V810性能下降 。 使用相同的参数多次调用了“ btCos(euler_out2.pitch)”函数。 结果可能应该保存到一个临时变量中,然后可以在调用“ btAtan2”函数时使用它。 btMatrix3x3.h 578

 void getEulerZYX(....) const { .... if (....) { .... } else { .... euler_out.roll = btAtan2(m_el[2].y() / btCos(euler_out.pitch), m_el[2].z() / btCos(euler_out.pitch)); euler_out2.roll = btAtan2(m_el[2].y() / btCos(euler_out2.pitch), m_el[2].z() / btCos(euler_out2.pitch)); euler_out.yaw = btAtan2(m_el[1].x() / btCos(euler_out.pitch), m_el[0].x() / btCos(euler_out.pitch)); euler_out2.yaw = btAtan2(m_el[1].x() / btCos(euler_out2.pitch), m_el[0].x() / btCos(euler_out2.pitch)); } .... } 

在这种情况下,您可以创建两个变量并将btCos函数返回的值存储在euler_out.pitcheuler_out2.pitch中 ,而不是为每个参数调用该函数四次。

泄漏


范例10:

在项目中发现了许多以下类型的错误:

V773在不释放内存的情况下退出了“导入程序”指针的可见性范围。 可能发生内存泄漏。 SerializeSetup.cpp 94

 void SerializeSetup::initPhysics() { .... btBulletWorldImporter* importer = new btBulletWorldImporter(m_dynamicsWorld); .... fclose(file); m_guiHelper->autogenerateGraphicsObjects(m_dynamicsWorld); } 

这里没有从导入器指针释放的内存。 这可能会导致内存泄漏。 对于物理引擎而言,这可能是一个坏趋势。 为了避免泄漏,在不再需要该变量添加delete importer之后就足够了。 但是,当然,最好使用智能指针。

这是你的法律


示例11:

由于C ++规则并不总是与数学规则或“常识”一致,因此在代码中出现以下错误。 自己注意到错误是在一小段代码中吗?

 btAlignedObjectArray<btFractureBody*> m_fractureBodies; void btFractureDynamicsWorld::fractureCallback() { for (int i = 0; i < numManifolds; i++) { .... int f0 = m_fractureBodies.findLinearSearch(....); int f1 = m_fractureBodies.findLinearSearch(....); if (f0 == f1 == m_fractureBodies.size()) continue; .... } .... } 

分析仪生成以下警告:

V709发现可疑比较:“ f0 == f1 == m_fractureBodies.size()”。 请记住,“ a == b == c”不等于“ a == b && b == c”。 btFractureDynamicsWorld.cpp 483

似乎该条件检查f0等于f1并等于m_fractureBodies中的元素 。 看起来此比较应该检查f0f1是否m_fractureBodies数组的末尾,因为它们包含由findLinearSearch()方法找到的对象的位置。 但是,实际上,此表达式变成一个测试,以查看f0f1是否相等,然后检查m_fractureBodies.size()是否等于f0 == f1的结果。 结果,此处将第三个操作数与0或1进行比较。

美丽的错误! 而且,幸运的是,非常罕见。 到目前为止,我们仅在两个开放项目中遇到了她,有趣的是,所有这些仅仅是游戏引擎。

示例12:

使用字符串时,通常最好使用字符串类提供的功能。 因此,对于以下两种情况,最好分别用MyStr.length()val.clear()替换strlen(MyStr.c_str())val =“”

V806性能下降 。 strlen(MyStr.c_str())类型的表达式可以重写为MyStr.length()。 RobotLoggingUtil.cpp 213

 FILE* createMinitaurLogFile(const char* fileName, std::string& structTypes, ....) { FILE* f = fopen(fileName, "wb"); if (f) { .... fwrite(structTypes.c_str(), strlen(structTypes.c_str()), 1, f); .... } .... } 

V815性能下降 。 考虑用“ val.clear()”替换表达式“ val =“”“。 b3CommandLineArgs.h 40

 void addArgs(int argc, char **argv) { .... std::string val; .... val = ""; .... } 

还有其他警告,但我认为您可以停止。 如您所见,静态代码分析可以揭示各种错误。

阅读有关一次性项目检查的信息很有趣,但这不是使用静态代码分析器的正确方法。 关于这一点,我们将在下面讨论。

发现的错误


在最近的文章“ 静态代码分析未发现错误,因为未使用它而找不到错误 ”的精神中,寻找那些已经修复但静态分析器能够检测到的错误或缺点很有趣。
图片2

存储库中没有太多的拉取请求,其中许多与引擎的内部逻辑有关。 但是也存在分析仪可以检测到的错误。

示例13:

 char m_deviceExtensions[B3_MAX_STRING_LENGTH]; void b3OpenCLUtils_printDeviceInfo(cl_device_id device) { b3OpenCLDeviceInfo info; b3OpenCLUtils::getDeviceInfo(device, &info); .... if (info.m_deviceExtensions != 0) { .... } } 

编辑注释说,有必要检查数组是否为空,但要执行无意义的指针检查,该检查始终返回true。 初始检查类型的警告PVS-Studio指示了这一点:

V600考虑检查状况。 “ info.m_deviceExtensions”指针始终不等于NULL。 b3OpenCLUtils.cpp 551

示例14:

您能否立即发现下一个功能中存在的问题?

 inline void Matrix4x4::SetIdentity() { m12 = m13 = m14 = m21 = m23 = m24 = m13 = m23 = m41 = m42 = m43 = 0.0; m11 = m22 = m33 = m44 = 1.0; } 

分析仪会生成以下警告:

V570相同的值两次分配给'm23'变量。 线性R4.h 627

V570相同的值两次分配给'm13'变量。 线性R4.h 627

这种记录形式的重复分配很难用肉眼跟踪,因此,矩阵的某些元素没有收到初始值。 此错误已通过分配记录的表格形式得到纠正:

 m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0; 

示例15:

btSoftBody :: addAeroForceToNode()函数的条件之一中的以下错误导致了显式错误。 根据拉动请求中的注释,力从错误的一侧施加到对象上。

 struct eAeroModel { enum _ { V_Point, V_TwoSided, .... END }; }; void btSoftBody::addAeroForceToNode(....) { .... if (....) { if (btSoftBody::eAeroModel::V_TwoSided) { .... } .... } .... } 

PVS-Studio还会发现此错误并显示以下警告:

V768枚举常量'V_TwoSided'用作布尔型变量。 btSoftBody.cpp 542

更正后的检查如下:

 if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... } 

而不是将对象属性等效于枚举器之一, 而是检查了V_TwoSided枚举器本身。

很明显,我没有查看所有拉取请求,因为我没有为自己设置任务。 我只是想表明,定期使用静态代码分析器可以在很早的阶段检测到错误。 这是使用静态代码分析的正确方法。 静态分析应内置于DevOps流程中,并且应作为错误的主要过滤器。 在“ 将静态分析嵌入到流程中,不要在其中发现错误 ”一文中对此进行了很好的描述。

结论


图片6

根据一些请求请求,有时会通过各种代码分析工具来检查项目,但是,编辑不是逐步进行的,而是成组且间隔较大的时间进行。 在某些请求中,注释表示所做的更改仅是为了禁止显示警告。 这种使用分析的方法大大降低了其实用性,因为它是对项目的不断检查,可让您立即修复错误,而不必等到任何明显的错误出现。

如果您想始终了解我们团队的新闻和事件,请订阅我们的社交服务。 网络: InstagramTwitterVkontakteTelegram



如果您想与讲英语的读者分享这篇文章,请使用翻译链接: PVS-Studio查看Red Dead Redemption的Bullet Engine

Source: https://habr.com/ru/post/zh-CN461849/


All Articles