Get best of the week

Periksa kembali Newton Game Dynamics dengan analisa statis PVS-Studio

Gambar 1

Baru-baru ini, di Internet, saya menemukan mesin fisika Newton Game Dynamics. Mengetahui bahwa proyek semacam itu biasanya memiliki sejumlah besar kode kompleks, saya pikir akan menarik untuk memeriksanya dengan penganalisa statis PVS-Studio. Antusiasme saya semakin terpacu oleh fakta bahwa kolega saya Andrei Karpov sudah menguji proyek ini pada tahun 2014, yang berarti ini juga merupakan kesempatan yang baik untuk menunjukkan pengembangan alat analisis kami selama enam tahun terakhir. Perlu juga dicatat bahwa pada saat penulisan, rilis terbaru Newton Game Dynamics bertanggal 27 Februari 2020, yaitu, proyek ini juga aktif dikembangkan selama 6 tahun terakhir. Jadi, saya berharap bahwa selain tim kami, artikel ini akan menarik bagi para pengembang mesin, yang akan dapat menyingkirkan beberapa bug dan memperbaiki kode mereka.

Output analisa


Pada tahun 2014, PVS-Studio menerbitkan:

  • 48 peringatan tingkat pertama;
  • 79 tingkat kedua;
  • 261 tingkat ketiga.

Namun untuk tahun 2020:

  • 124 peringatan tingkat pertama;
  • 272 tingkat kedua;
  • 787 dari level ketiga (di antaranya juga ada yang menarik).

Ada peringatan yang jauh lebih menarik daripada di artikel Andrey , tapi kami akan mempelajarinya lebih detail.

Deskripsi peringatan


Peringatan N1

V519 Variabel 'tmp [i] [2]' diberikan nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 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);
  }
  ....
}

Elemen array tmp [i] [2] diinisialisasi dua kali berturut-turut. Paling sering, kode seperti itu berbicara tentang salin-tempel. Anda dapat memperbaiki ini dengan menghapus inisialisasi yang tidak perlu jika tidak diperlukan, atau dengan mengganti indeks array dengan yang berikutnya - ini sudah tergantung pada nilai variabel jumlah . Lebih lanjut, saya ingin menggambarkan peringatan V519 lain , yang tidak dimiliki Andrei dalam artikel tersebut, tetapi memiliki basis data kesalahan kami :

V519 Objek 'basah' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. fisika 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));
  ....
}

Saya akui, saya tidak menemukan kesalahan ini di log analyzer. Juga, saya tidak menemukan AddBuoyancyForce fungsi di dgbody.cpp . Ini normal: jika contoh baru kesalahan yang ditemukan menggunakan peringatan dari penganalisa kami adalah indikator pengembangan PVS-Studio, maka tidak adanya kesalahan yang ditemukan sebelumnya dalam proyek adalah indikator pengembangan proyek.

Offtopic kecil

saya tidak bisa berpura-pura mengatakan bahwa fragmen kode berikut mengandung kesalahan atau tidak berfungsi seperti yang diharapkan oleh programmer, tetapi mereka berperilaku agak curiga.

Penganalisis mengeluarkan dua peringatan untuk fragmen kode ini:

V621 Pertimbangkan untuk memeriksa operator 'untuk'. Ada kemungkinan bahwa loop akan dieksekusi secara tidak benar atau tidak akan dieksekusi sama sekali. MultiBodyCar.cpp 942

V654 Kondisi 'i <count' dari loop selalu salah. 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());
    }
  }
  ....
}

Mungkin kode ini digunakan untuk debugging, kemudian mematikan loop sepertinya merupakan langkah normal. Beberapa poin serupa lainnya juga ditemukan:

V519 Variabel 'ret' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 325, 326. dString.cpp 326

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

V519 Variabel 'ret' diberi nilai dua kali berturut-turut. Mungkin ini adalah kesalahan. Periksa baris: 1222, 1223. DemoEntityManager.cpp 1223

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

V560 Bagian dari ekspresi kondisional selalu benar: (hitung <10). dMathDefines.h 726

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

V654 Kondisi 'ptr! = Edge' dari loop selalu salah. dgPolyhedra.cpp 1571

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

V763 Parameter 'count' selalu ditulis ulang di badan fungsi sebelum digunakan. ConvexCast.cpp 31

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

Ekspresi V547 'axisCount' selalu salah. MultiBodyCar.cpp 650

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

Mungkin, banyak yang akan mengatakan bahwa ketika membuat perubahan dalam kode yang disimpan dalam domain publik, Anda setidaknya harus menulis komentar. Ya saya setuju. Beberapa hal yang dapat digunakan tanpa rasa sakit dalam proyek hewan peliharaan, menurut pendapat saya, tidak dapat diterima dalam kode yang akan digunakan oleh banyak orang. Namun, pilihan ada di tangan penulis.

Peringatan N2

V769 Penunjuk 'hasil' dalam ekspresi 'hasil + i' sama dengan nullptr. Nilai yang dihasilkan tidak masuk akal dan tidak boleh digunakan. 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;
    }
}

Masalahnya adalah bahwa hasil tidak berubah sejak inisialisasi. Pointer yang dihasilkan tidak masuk akal, tidak bisa digunakan.

Peringatan N3, N4, N5

V778 Dua fragmen kode serupa ditemukan. Mungkin, ini adalah kesalahan ketik dan variabel 'm_colorChannel' harus digunakan daripada '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]);
  }
}

Tampaknya programmer telah menyalin dua syarat. Yang kedua akan terlihat seperti ini:

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

Kesalahan lain yang sangat mirip juga ditemukan:

V524 Aneh bahwa tubuh fungsi 'EnabledAxis1' sepenuhnya setara dengan tubuh fungsi 'EnabledAxis0'. dCustomDoubleHingeActuator.cpp 88

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

Di sini kode harus diperbaiki seperti ini:

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

Salin-tempel lain:

V525 Kode ini berisi kumpulan blok serupa. Periksa item 'm_x', 'm_y', 'm_y' pada baris 73, 74, 75. 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);  <=
  ....
  }
  ....
}

Kemungkinan besar, inisialisasi variabel z akan terlihat seperti ini:

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

Peringatan N6, N7

Dalam Newton Game Dynamics, seperti pada hampir semua proyek C atau C ++ besar, ada beberapa peringatan tentang pekerjaan yang tidak aman dengan pointer. Kesalahan seperti itu seringkali sangat sulit ditemukan dan didebug, mereka menyebabkan crash program, secara umum, mereka sangat berbahaya dan tidak dapat diprediksi. Untungnya, penganalisa kami dapat mendeteksi banyak kesalahan ini. Tampaknya jelas bahwa lebih baik menulis cek sekali dan tidak mandi uap, daripada menghabiskan banyak waktu mereproduksi masalah, menemukan tempat masalah dalam kode dan men-debug-nya. Berikut adalah beberapa peringatan:

V522 Mungkin ada dereferensi dari 'wajah' penunjuk nol potensial. dgContactSolver.cpp 351

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

Definisi fungsi NewFace kecil, jadi saya akan memberikannya secara penuh:

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;
}

Salah satu fungsi titik keluar NewFace mengembalikan NULL , jika terjadi null pointer dereferencing dan akan ada perilaku program yang tidak ditentukan.

Ada juga peringatan yang mirip dengan potongan kode yang lebih berbahaya:

V522 Mungkin ada dereferensi dari 'perimeter' penunjuk null potensial. dgPolyhedra.cpp 2541

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

Berikut adalah definisi dari 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;
  }
}

Di sini, NULL adalah nilai balik pada dua titik dari tiga kemungkinan titik keluar fungsi.

Secara total, saya menerima 48 peringatan V522 . Sebagian besar, mereka dari jenis yang sama, jadi saya tidak melihat alasan untuk menjelaskan lagi dalam kerangka artikel ini.

Peringatan N8

V668 Tidak ada gunanya menguji pointer 'pBits' terhadap null, karena memori dialokasikan menggunakan operator 'baru'. Pengecualian akan dihasilkan jika terjadi kesalahan alokasi memori. TargaToOpenGl.cpp 166

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

Penganalisa mendeteksi situasi di mana nilai pointer dikembalikan oleh operator baru dibandingkan dengan nol. Sebagai aturan, ini berarti bahwa program, jika tidak mungkin mengalokasikan memori, tidak akan berperilaku seperti yang diharapkan oleh programmer. Jika operator baru tidak dapat mengalokasikan memori, maka, menurut standar bahasa C ++ , pengecualian std :: bad_alloc () dilemparkan. Dengan demikian, kondisinya tidak akan pernah terpenuhi. Ini jelas bukan perilaku yang diandalkan oleh programmer. Dia berencana untuk menutup file jika terjadi kesalahan alokasi memori. Ini tidak akan terjadi dan kebocoran sumber daya akan terjadi.

Peringatan N9, N10, N11

  • V764 Kemungkinan urutan argumen yang salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 791
  • V764 Kemungkinan urutan argumen yang salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 833
  • V764 Kemungkinan urutan argumen yang salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 884

Beginilah tampilan fungsi panggilan:

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

Ini adalah bagaimana deklarasi fungsi terlihat:

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

Diagnosis ini menunjukkan bahwa saat memanggil fungsi, mungkin argumennya campur aduk.

Peringatan N12, N13

Penganalisis mengeluarkan peringatan untuk dua metode serupa dengan nama yang berbeda:

V621 Pertimbangkan untuk memeriksa operator 'untuk'. Ada kemungkinan bahwa loop akan dieksekusi secara tidak benar atau tidak akan dieksekusi sama sekali. dgCollisionUserMesh.cpp 161

V621 Pertimbangkan untuk memeriksa operator 'untuk'. Ada kemungkinan bahwa loop akan dieksekusi secara tidak benar atau tidak akan dieksekusi sama sekali. 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++)
  {
    ....
  }
  ....
}

Masalah pada bagian kondisi ini adalah: i <data-> m_faceCount. Karena data-> m_faceCount diberikan 0, loop ini tidak akan dieksekusi sekalipun. Mungkin, ketika menulis potongan kode ini, programmer lupa menginisialisasi ulang bidang m_faceCount , dan kemudian cukup menyalin tubuh metode.

Peringatan N14, N15

Analyzer mengeluarkan dua peringatan untuk baris kode yang serupa secara berurutan:

V630 Fungsi '_alloca' digunakan untuk mengalokasikan memori untuk array objek yang merupakan kelas yang mengandung konstruktor. dgSkeletonContainer.cpp 1341

V630 Fungsi '_alloca' digunakan untuk mengalokasikan memori untuk array objek yang merupakan kelas yang mengandung konstruktor. 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);

Masalah dengan fragmen kode ini adalah bahwa mereka bekerja dengan memori yang dialokasikan sebagai array objek yang memiliki konstruktor atau destruktor. Dengan alokasi memori ini untuk kelas, konstruktor tidak akan dipanggil. Saat membebaskan memori, destruktor tidak akan dipanggil. Ini sangat mencurigakan. Kode semacam itu dapat bekerja dengan variabel yang tidak diinisialisasi dan kesalahan lainnya. Selain itu, dibandingkan dengan pendekatan menggunakan malloc / gratis , kode seperti itu buruk karena jika Anda mencoba mengalokasikan lebih banyak memori daripada yang dapat diberikan mesin, Anda tidak akan menerima pesan kesalahan yang jelas. Sebagai gantinya, Anda mendapatkan kesalahan segmentasi ketika Anda mencoba mengakses memori ini. Berikut adalah beberapa pesan analisa lainnya:
  • 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 .


Secara umum, PVS-Studio gagal lagi, selama verifikasi, kesalahan menarik baru ditemukan. Dan ini berarti bahwa penganalisa melakukan tugasnya dengan baik, memungkinkan kita untuk membuat dunia di sekitar kita sedikit lebih sempurna. Untuk mencoba analisa statis PVS-Studio pada proyek Anda, Anda dapat mengklik tautan tersebut .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Vladislav Stolyarov. Pemeriksaan Kedua Newton Game Dynamics dengan PVS-Studio .

More articles:


All Articles