👐🏻 🏇🏿 🥚 使用PVS-Studio静态分析器重新检查牛顿游戏动力学 👦🏼 ⛹🏿 👋🏽

最近，在Internet上，我发现了物理引擎Newton Game Dynamics。知道这样的项目通常包含大量复杂的代码，我认为使用PVS-Studio静态分析器对其进行检查将很有趣。我的同事安德烈·卡尔波夫（Andrei Karpov）已在2014年对该项目进行了测试，这进一步激发了我的热情，这意味着这也是展示我们的分析仪在过去六年中发展的绝佳机会。值得注意的是，在撰写本文时，Newton Game Dynamics的最新版本发布于2020年2月27日，也就是说，该项目在过去6年中也在积极开发。因此，我希望除了我们的团队之外，引擎开发人员也可以对本文感兴趣，他们将摆脱一些错误并修复其代码。

分析仪输出

2014年，PVS-Studio发布：

48个一级警报；
79级
261第三级。

但是，对于2020年：

124个一级警告；
272级
第三等级的787（其中也有有趣的等级）。

与Andrey的文章相比，警告更加有趣，但我们将对其进行更详细的研究。

警告说明

警告N1

V519为'tmp [i] [2]'变量连续分配了两次值。也许这是一个错误。检查行：468、469。dgCollisionConvexHull.cpp 469

bool dgCollisionConvexHull::Create (dgInt32 count,....)
{
  ....
  dgStack<dgVector> tmp(3 * count);
  for (dgInt32 i = 0; i < count; i ++) 
  {
    tmp[i][0] = dgFloat32 (buffer[i*3 + 0]);
    tmp[i][1] = dgFloat32 (buffer[i*3 + 1]);
    tmp[i][2] = dgFloat32 (buffer[i*3 + 2]);
    tmp[i][2] = dgFloat32 (0.0f);
  }
  ....
}

数组元素tmp [i] [2]连续初始化两次。最常见的是，这样的代码谈到复制粘贴。您可以通过删除不必要的初始化（如果不需要的话）或将数组索引替换为后续的索引来解决此问题，这已经取决于count变量的值了。此外，我想描述另一个警告V519，Andrei在文章中没有，但有我们的错误数据库：

V519为'damp'对象连续分配了两次值。也许这是一个错误。物理dgbody.cpp 404

void dgBody::AddBuoyancyForce (....)
{
  ....
  damp = (m_omega % m_omega) * dgFloat32 (10.0f) *
        fluidAngularViscousity; 
  damp = GetMax (GetMin ((m_omega % m_omega) * 
       dgFloat32 (1000.0f) * 
       fluidAngularViscousity, dgFloat32(0.25f)), 
       dgFloat32(2.0f));
  ....
}

我承认，我没有在分析器日志中找到此错误。另外，我在dgbody.cpp中没有找到AddBuoyancyForce函数。这是正常的：如果使用分析仪的警告发现新的错误示例可以指示PVS-Studio的发展，那么在项目早期发现的错误就可以指示项目的发展。我不能假装说一个小小的题外话，以下代码片段包含错误或无法按程序员的预期运行，但它们的行为相当可疑。分析器对此代码段发出两个警告： V621考虑检查“ for”运算符。循环有可能执行不正确或根本不执行。 MultiBodyCar.cpp 942

V654循环的条件“ i <计数”始终为false。MultiBodyCar.cpp 942

void MultibodyBodyCar(DemoEntityManager* const scene)
{
  ....
  int count = 10;
  count = 0;
  for (int i = 0; i < count; i++) 
  {
    for (int j = 0; j < count; j++) 
    {
      dMatrix offset(location);
      offset.m_posit += dVector (j * 5.0f + 4.0f, 0.0f, i * 5.0f, 0.0f);
      //manager->CreateSportCar(offset, viperModel.GetData());
      manager->CreateOffRoadCar(offset, monsterTruck.GetData());
    }
  }
  ....
}

也许此代码用于调试，然后关闭循环似乎是正常的举动。还发现了其他几个类似的点：

V519'ret'变量被连续分配两次值。也许这是一个错误。检查行：325，326. dString.cpp 326

void dString::LoadFile (FILE* const file)
{
  ....
  size_t ret = fread(m_string, 1, size, file);
  ret = 0;
  ....
}

V519'ret'变量被连续两次赋值，也许这是一个错误。检查行：1222、1223。DemoEntityManager.cpp 1223

void DemoEntityManager::DeserializeFile (....)
{
  ....
  size_t ret = fread(buffer, size, 1, (FILE*) serializeHandle);
  ret = 0;
  ....
}

V560条件表达式的一部分始终为true ：（计数<10）。dMathDefines.h 726

bool dCholeskyWithRegularizer(....)
{
  ....
  int count = 0;
  while (!pass && (count < 10))
  {
    ....
  }
  ....
}

V654循环的条件“ ptr！= Edge”始终为false。dgPolyhedra.cpp 1571

void dgPolyhedra::Triangulate (....)
{
  ....
  ptr = edge;
  ....
  while (ptr != edge);
  ....
}

V763参数“ count”在使用前总是在功能体内被重写。ConvexCast.cpp 31

StupidComplexOfConvexShapes (...., int count)
{
  count = 40;
  //count = 1;
  ....
}

V547表达式'axisCount'始终为false。MultiBodyCar.cpp 650

void UpdateDriverInput(dVehicle* const vehicle, dFloat timestep) 
{
  ....
  int axisCount = scene->GetJoystickAxis(axis);
  axisCount = 0;
  if (axisCount)
  {
    ....
  }
  ....
}

可能很多人会说，当对存储在公共领域中的代码进行这种更改时，您至少必须写一个注释。好吧，我同意。我认为，在宠物项目中可以轻松使用的某些东西在许多人将要使用的代码中是不可接受的。但是，选择权取决于作者。

警告N2

V769“结果+ i”表达式中的“结果”指针等于nullptr。结果值是没有意义的，不应使用。win32_monitor.c 286

GLFWvidmode* _glfwPlatformGetVideoModes(_GLFWmonitor* monitor, int* count)
{
  GLFWvidmode* result = NULL;
  ....
  for (i = 0;  i < *count;  i++)
    {
    if (_glfwCompareVideoModes(result + i, &mode) == 0)
      break;
    }
}

问题是自初始化以来结果没有改变。结果指针将无意义，无法使用。

警告N3，N4，N5

V778找到了两个类似的代码片段。也许这是一个错字，应该使用“ m_colorChannel”变量而不是“ m_binormalChannel”。dgMeshEffect1.cpp 1887

void dgMeshEffect::EndBuildFace ()
{
  ....
  if (m_attrib.m_binormalChannel.m_count) <=
  {
    attibutes.m_binormalChannel.
      PushBack(m_attrib.m_binormalChannel[m_constructionIndex + i]);
  }
  if (m_attrib.m_binormalChannel.m_count) <= 
  {
    attibutes.m_colorChannel.
      PushBack(m_attrib.m_colorChannel[m_constructionIndex + i]);
  }
}

程序员似乎复制了两个条件。第二个应该看起来像这样：

if (m_attrib.m_colorChannel.m_count) <= 
{
  attibutes.m_colorChannel.
  PushBack(m_attrib.m_colorChannel[m_constructionIndex + i]);
}

还发现了另一个非常相似的错误：

V524很奇怪，“ EnabledAxis1”函数的主体与“ EnabledAxis0”函数的主体完全等效。dCustomDoubleHingeActuator.cpp 88

void dCustomDoubleHingeActuator::EnabledAxis0(bool state)
{
  m_axis0Enable = state;  <=
}
void dCustomDoubleHingeActuator::EnabledAxis1(bool state)
{
  m_axis0Enable = state;  <=
}

这里的代码应该像这样固定：

void dCustomDoubleHingeActuator::EnabledAxis1(bool state)
{
  m_axis1Enable = state;
}

另一个复制粘贴：

V525代码包含相似块的集合。在第73、74、75行中检查项目'm_x'，'m_y'，'m_y'。dWoodFracture.cpp 73

WoodVoronoidEffect(....)
{
  ....
  for (int i = 0; i < count; i ++) 
  {
    dFloat x = dGaussianRandom(size.m_x * 0.1f);
    dFloat y = dGaussianRandom(size.m_y * 0.1f);  <=
    dFloat z = dGaussianRandom(size.m_y * 0.1f);  <=
  ....
  }
  ....
}

最有可能的是，变量z的初始化应如下所示：

dFloat z = dGaussianRandom(size.m_z * 0.1f);

警告N6，N7

在Newton Game Dynamics中，几乎在任何大型C或C ++项目中，都有一些有关指针不安全工作的警告。此类错误通常很难查找和调试；它们会导致程序崩溃；通常，它们非常危险且不可预测。幸运的是，我们的分析仪能够检测出许多此类错误。显然，写一张支票而不要洗个澡比花很多时间重现问题，在代码中查找问题点并进行调试要好。以下是一些警告：

V522可能会取消引用潜在的空指针“面”。dgContactSolver.cpp 351

DG_INLINE dgMinkFace* dgContactSolver::AddFace(dgInt32 v0,dgInt32 v1,
                                               dgInt32 v2)
{
  dgMinkFace* const face = NewFace();
  face->m_mark = 0; 
  ....
}

NewFace函数的定义很小，因此我将完整介绍一下：

DG_INLINE dgMinkFace* dgContactSolver::NewFace()
{
  dgMinkFace* face = (dgMinkFace*)m_freeFace;
  if (m_freeFace) 
  {
    m_freeFace = m_freeFace->m_next;
  } else 
  {
    face = &m_facePool[m_faceIndex];
    m_faceIndex++;
    if (m_faceIndex >= DG_CONVEX_MINK_MAX_FACES) 
    {
      return NULL;
    }
  }
#ifdef _DEBUG
    memset(face, 0, sizeof (dgMinkFace));
#endif
  return face;
}

退出点NewFace的功能之一将返回NULL，如果发生返回的空指针取消引用，并且该程序将有未定义的行为。

对于更危险的代码段，也有类似的警告：

V522可能会取消引用潜在的空指针“周长”。dgPolyhedra.cpp 2541

bool dgPolyhedra::PolygonizeFace(....)
{
  ....
  dgEdge* const perimeter = flatFace.AddHalfEdge
                           (edge1->m_next->m_incidentVertex,
                            edge1->m_incidentVertex);
  perimeter->m_twin = edge1;
  ....
}

这是AddHalfEdge的定义：

dgEdge* dgPolyhedra::AddHalfEdge (dgInt32 v0, dgInt32 v1)
{
  if (v0 != v1) 
  {
    dgPairKey pairKey (v0, v1);
    dgEdge tmpEdge (v0, -1);
    dgTreeNode* node = Insert (tmpEdge, pairKey.GetVal()); 
    return node ? &node->GetInfo() : NULL;
  } else 
  {
    return NULL;
  }
}

此处，NULL是函数三个可能的退出点中的两个点处的返回值。

总共收到了48条V522警告。在大多数情况下，它们是同一类型，因此我认为没有理由在本文的框架中进行任何描述。

警告N8

V668测试'pBits'指针是否为null没有意义，因为使用'new'运算符分配了内存。如果内存分配错误，将生成异常。 TargaToOpenGl.cpp 166

char* const pBits = new char [width * height * 4];
if(pBits == NULL) 
{
  fclose(pFile);
  return 0;
}

分析器检测到新操作符返回的指针的值与零进行比较的情况。通常，这意味着如果无法分配内存，则程序的行为将与程序员期望的不同。如果新操作符无法分配内存，则根据C ++语言标准，将引发异常std :: bad_alloc（）。因此，该条件将永远无法满足。显然这不是程序员指望的行为。他计划在内存分配错误的情况下关闭文件。这不会发生，并且会发生资源泄漏。

警告N9，N10，N11

V764传递给'CreateWheel'函数的参数的可能错误顺序：'height'和'radius'。第791章
V764传递给'CreateWheel'函数的参数的可能错误顺序：'height'和'radius'。StandardJoints.cpp 833
V764传递给'CreateWheel'函数的参数的可能错误顺序：'height'和'radius'。StandardJoints.cpp 884

函数调用的外观如下：

NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);

这是函数声明的外观：

static NewtonBody* CreateWheel (DemoEntityManager* const scene,
  const dVector& location, dFloat radius, dFloat height)

此诊断表明，在调用函数时，参数可能会混淆。

警告N12，N13

分析仪针对两种名称不同的类似方法发出警告：

V621考虑检查“ for”操作员。循环有可能执行不正确或根本不执行。 dgCollisionUserMesh.cpp 161

V621考虑检查“ for”运算符。循环有可能执行不正确或根本不执行。 dgCollisionUserMesh.cpp 236

void dgCollisionUserMesh::GetCollidingFacesContinue
    (dgPolygonMeshDesc* const data) const
{
  ....
  data->m_faceCount = 0; <=
  data->m_userData = m_userData;
  data->m_separationDistance = dgFloat32(0.0f);
  m_collideCallback(&data->m_p0, NULL);
  dgInt32 faceCount0 = 0;
  dgInt32 faceIndexCount0 = 0;
  dgInt32 faceIndexCount1 = 0;
  dgInt32 stride = data->m_vertexStrideInBytes / sizeof(dgFloat32);
  dgFloat32* const vertex = data->m_vertex;
  dgInt32* const address = data->m_meshData.m_globalFaceIndexStart;
  dgFloat32* const hitDistance = data->m_meshData.m_globalHitDistance;
  const dgInt32* const srcIndices = data->m_faceVertexIndex;
  dgInt32* const dstIndices = data->m_globalFaceVertexIndex;
  dgInt32* const faceIndexCountArray = data->m_faceIndexCount;
  for (dgInt32 i = 0; (i < data->m_faceCount)&&
       (faceIndexCount0 < (DG_MAX_COLLIDING_INDICES - 32));
       i++)
  {
    ....
  }
  ....
}
void dgCollisionUserMesh::GetCollidingFacesDescrete
    (dgPolygonMeshDesc* const data) const
{
  ....
  data->m_faceCount = 0; <=  
  data->m_userData = m_userData;
  data->m_separationDistance = dgFloat32(0.0f);
  m_collideCallback(&data->m_p0, NULL);
  dgInt32 faceCount0 = 0;
  dgInt32 faceIndexCount0 = 0;
  dgInt32 faceIndexCount1 = 0;
  dgInt32 stride = data->m_vertexStrideInBytes / sizeof(dgFloat32);
  dgFloat32* const vertex = data->m_vertex;
  dgInt32* const address = data->m_meshData.m_globalFaceIndexStart;
  dgFloat32* const hitDistance = data->m_meshData.m_globalHitDistance;
  const dgInt32* const srcIndices = data->m_faceVertexIndex;
  dgInt32* const dstIndices = data->m_globalFaceVertexIndex;
  dgInt32* const faceIndexCountArray = data->m_faceIndexCount;
  for (dgInt32 i = 0; (i < data->m_faceCount)&&
       (faceIndexCount0 < (DG_MAX_COLLIDING_INDICES - 32));
       i++)
  {
    ....
  }
  ....
}

此部分条件中的问题是：i <data-> m_faceCount。由于data-> m_faceCount分配为0，因此即使执行一次该循环也不会执行。可能在编写这段代码时，程序员忘记了重新初始化m_faceCount字段，然后简单地复制了方法主体。

警告N14，N15

分析器连续针对相似的代码行发出了两个警告：

V630'_alloca'函数用于为包含构造函数的类的对象数组分配内存。 dgSkeletonContainer.cpp 1341

V630'_alloca'函数用于为对象数组分配内存，这些对象数组是包含构造函数的类。 dgSkeletonContainer.cpp 1342

#define alloca _alloca
....
#define dAlloca(type,size) (type*) alloca ((size) * sizeof (type))
....
dgSpatialMatrix::dgSpatialMatrix();
dgSpatialMatrix::dgSpatialMatrix(dgFloat32 val);
....
dgSpatialMatrix* const bodyMassArray = dgAlloca(dgSpatialMatrix,
                                                m_nodeCount);
dgSpatialMatrix* const jointMassArray = dgAlloca(dgSpatialMatrix,
                                                 m_nodeCount);

此代码段的问题在于它们将分配的内存作为具有构造函数或析构函数的对象数组使用。通过为此类分配内存，将不会调用构造函数。释放内存时，不会调用析构函数。这是非常可疑的。此类代码可能导致使用未初始化的变量和其他错误。另外，与使用malloc / free的方法相比，这样的代码很糟糕，因为如果您尝试分配的内存超出了计算机可以提供的数量，则不会收到明确的错误消息。而是，当您尝试访问此内存时，您会遇到分段错误。以下是一些分析器消息：

V630 The '_alloca' function is used to allocate memory for an array of objects which are classes containing constructors. dVehicleSolver.cpp 498
V630 The '_alloca' function is used to allocate memory for an array of objects which are classes containing constructors. dVehicleSolver.cpp 499
V630 The '_alloca' function is used to allocate memory for an array of objects which are classes containing constructors. dVehicleSolver.cpp 1144
10 .

通常，PVS-Studio再次失败，在验证期间，发现了新的有趣错误。这意味着分析仪可以很好地完成工作，从而使我们周围的世界更加完美。为了在您的项目上尝试使用PVS-Studio静态分析仪，您可以单击链接。

如果您想与说英语的读者分享这篇文章，请使用以下链接：Vladislav Stolyarov。用PVS-Studio再次检查牛顿游戏动力学。

使用PVS-Studio静态分析器重新检查牛顿游戏动力学

分析仪输出

警告说明

More articles: