Amazon Lumberyard: tangisan jiwa


Game adalah salah satu produk perangkat lunak paling populer. Ini adalah industri besar di mana mesin game baru - Amazon Lumberyard. Proyek ini masih dalam status beta dan memiliki waktu untuk memperbaiki kesalahan dan meningkatkan kualitas kode. Para pengembang mesin memiliki banyak pekerjaan yang harus dilakukan dalam waktu dekat untuk tidak mengecewakan jutaan gamer dan pengembang game.

Pendahuluan


Amazon Lumberyard adalah mesin permainan lintas-platform AAA gratis yang dikembangkan oleh Amazon dan didasarkan pada arsitektur mesin CryEngine , yang dilisensikan oleh Crytek pada 2015. Ngomong-ngomong, analisis CryEngine sudah dilakukan dua kali oleh saya pada Agustus 2016 dan April 2017 . Pada saat yang sama, saya harus perhatikan bahwa setelah satu tahun, kodenya semakin memburuk. Dan tempo hari, saya memutuskan untuk melihat apa yang dilakukan Amazon berdasarkan mesin game ini. Mereka bekerja sangat baik di lingkungan. Dokumentasi untuk pengembang dan perangkat lunak untuk menyebarkan lingkungan kerja dibuat sangat keren dan pada tingkat tinggi. Tetapi masalahnya lagi dengan kode! Saya berharap bahwa Amazon memiliki lebih banyak sumber daya untuk bekerja dengan proyek ini, dan mereka masih memperhatikan kualitas kode. Dengan ulasan ini, saya berharap dapat menarik perhatian pengembang terhadap kualitas kode dan mendorong pendekatan baru dalam pengembangan mesin game ini. Keadaan kode saat ini sangat memprihatinkan sehingga saya mengubah judul artikel beberapa kali dan menggambar ulang gambar judul saat saya melihat laporan dengan hasil analisis. Versi pertama dari gambar itu kurang emosional:



Kami menganalisis sumber Amazon Lumberyard dari versi terbaru yang tersedia 1.14.0.1. Kode sumber diambil dari repositori di Github . Salah satu game pertama di mesin Lumberyard adalah Star Citizen . Pemain potensial yang menunggunya, saya juga mengundang Anda untuk melihat apa yang saat ini "di bawah tenda" dari permainan.

Integrasi dengan PVS-Studio


PVS-Studio digunakan sebagai penganalisa kode statis. Ini tersedia untuk Windows, Linux, dan macOS. Yaitu untuk analisis proyek lintas platform bahkan ada sesuatu yang bisa dipilih untuk pekerjaan yang lebih nyaman. Selain C dan C ++, analisis proyek dalam bahasa C # didukung. Rencana Jawa . Sebagian besar kode di dunia ditulis dalam bahasa yang terdaftar (bukan tanpa kesalahan, tentu saja), jadi cobalah alat analisa PVS-Studio pada proyek Anda, Anda akan belajar banyak hal menarik ;-).

Sebagai sistem perakitan Lumberyard, WAF digunakan, yang juga di CryEngine. Alat analisis tidak memiliki cara khusus untuk berintegrasi dengan sistem perakitan ini. Saya memutuskan untuk bekerja dengan sebuah proyek di Windows dan memilih metode ini untuk memulai analisis: sistem pemantauan kompilasi . File proyek untuk Visual Studio dibuat secara otomatis. Ini dapat digunakan untuk membangun proyek dan melihat laporan penganalisa.

Daftar perintah untuk analisis terlihat seperti ini:

cd /path/to/lumberyard/dev lmbr_waf.bat ... CLMonitor.exe monitor MSBuild.exe ... LumberyardSDK_vs15.sln ... CLMonitor.exe analyze --log /path/to/report.plog 

Laporan, seperti yang saya katakan, dapat dilihat di Visual Studio.

Tentang Igor dan Qualcomm


Amazon Lumberyard memposisikan dirinya sebagai mesin permainan lintas platform. Mempromosikan proyek kepada massa dengan fitur seperti itu mudah, tetapi mempertahankannya sangat sulit. Salah satu peringatan dari PVS-Studio dikeluarkan pada sebuah fragmen kode di mana programmer Igor bertarung dengan kompiler Qualcomm. Mungkin dia memecahkan masalahnya, tetapi meninggalkan kode yang sangat mencurigakan. Saya memutuskan untuk menggambarnya dengan sebuah gambar.

V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. toglsloperand.c 700


Di sini kode yang sama dijalankan, terlepas dari kondisi yang dihitung. Terhadap latar belakang komentar yang tersisa, keputusan seperti itu terlihat mencurigakan.

Secara umum, proyek ini bukan satu-satunya tempat di mana kondisi perlu disederhanakan untuk kejelasan, atau untuk memperbaiki kesalahan nyata. Berikut adalah daftar tempat-tempat tersebut:

  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. livingentity.cpp 1385
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. tometalinstruction.c 4201
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. scripttable.cpp 905
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. budgetingsystem.cpp 701
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. editorframeworkapplication.cpp 562
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. particleitem.cpp 130
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. trackviewnodes.cpp 1223
  • V523 Pernyataan 'lalu' sama dengan pernyataan 'lain'. propertyoarchive.cpp 447

Python ++




Penganalisa menemukan kode yang sangat lucu:

V709 CWE-682 Perbandingan mencurigakan ditemukan: 'a == b == c'. Ingat bahwa 'a == b == c' tidak sama dengan 'a == b && b == c'. toglslinstruction.c 564

 void CallBinaryOp(....) { .... uint32_t src1SwizCount = GetNumSwizzleElements(....); uint32_t src0SwizCount = GetNumSwizzleElements(....); uint32_t dstSwizCount = GetNumSwizzleElements(....); .... if (src1SwizCount == src0SwizCount == dstSwizCount) // <= { .... } .... } 

Di C ++, kode seperti itu, sayangnya, berhasil dikompilasi, tetapi tidak dieksekusi sama sekali seperti yang terlihat. Ekspresi dalam C ++ dievaluasi dalam urutan prioritas, mengeksekusi kasta implisit, jika perlu.

Cek semacam itu akan benar, misalnya, dalam bahasa Python. Tetapi dalam situasi ini, pengembang "menembak dirinya sendiri di kaki."

3 tembakan kontrol lainnya:

  • V709 CWE-682 Perbandingan mencurigakan ditemukan: 'a == b == c'. Ingat bahwa 'a == b == c' tidak sama dengan 'a == b && b == c'. toglslinstruction.c 654
  • V709 CWE-682 Perbandingan mencurigakan ditemukan: 'a == b == c'. Ingat bahwa 'a == b == c' tidak sama dengan 'a == b && b == c'. toglslinstruction.c 469
  • V709 CWE-682 Perbandingan mencurigakan ditemukan: 'a == b == c'. Ingat bahwa 'a == b == c' tidak sama dengan 'a == b && b == c'. tometalinstruction.c 539

Yang pertama dan salah satu diagnostik terbaik




Ini akan menjadi tentang peringatan V501 - diagnostik tujuan umum pertama di PVS-Studio. Kesalahan yang ditemukan oleh diagnostik ini saja sudah cukup untuk menulis artikel. Dan proyek Amazon Lumberyard menunjukkan ini dengan sangat baik.

Sayangnya, membosankan untuk melihat kesalahan dari jenis yang sama untuk waktu yang lama, jadi di bagian ini saya hanya akan mengomentari beberapa fragmen kode, dan sisa peringatan akan terdaftar.

V501 Ada sub-ekspresi identik ke kiri dan ke kanan '||' operator: hotX <0 || hotX <0 editorutils.cpp 166

 QCursor CMFCUtils::LoadCursor(....) { .... if (!pm.isNull() && (hotX < 0 || hotX < 0)) { QFile f(path); f.open(QFile::ReadOnly); QDataStream stream(&f); stream.setByteOrder(QDataStream::LittleEndian); f.read(10); quint16 x; stream >> x; hotX = x; stream >> x; hotY = x; } .... } 

Kondisi ini tidak memiliki variabel Y panas . Kesalahan ketik klasik.

V501 Ada sub-ekspresi identik 'sp.m_pTexture == m_pTexture' di sebelah kiri dan di sebelah kanan operator '&&'. shadercomponents.h 487

V501 Ada sub-ekspresi identik 'sp.m_eCGTextureType == m_eCGTextureType' di kiri dan di kanan operator '&&'. shadercomponents.h 487

 bool operator != (const SCGTexture& sp) const { if (sp.m_RegisterOffset == m_RegisterOffset && sp.m_Name == m_Name && sp.m_pTexture == m_pTexture && // <= 1 sp.m_RegisterCount == m_RegisterCount && sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_BindingSlot == m_BindingSlot && sp.m_Flags == m_Flags && sp.m_pAnimInfo == m_pAnimInfo && sp.m_pTexture == m_pTexture && // <= 1 sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_bSRGBLookup == m_bSRGBLookup && sp.m_bGlobal == m_bGlobal) { return false; } return true; } 

Dua copy paste ditemukan dalam fragmen ini sekaligus. Untuk lebih jelasnya, saya melukis panah.

V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '==': pSrc.GetLen () == pSrc.GetLen () fbxpropertytypes.h 978

 inline bool FbxTypeCopy(FbxBlob& pDst, const FbxString& pSrc) { bool lCastable = pSrc.GetLen() == pSrc.GetLen(); FBX_ASSERT( lCastable ); if( lCastable ) pDst.Assign(pSrc.Buffer(), (int)pSrc.GetLen()); return lCastable; } 

Di sini saya ingin menyapa para pengembang dari AUTODESK . Kesalahan ini dari perpustakaan FBX SDK mereka. Variabel yang membingungkan pSrc dan pDst . Saya pikir, selain Lumberyard, ada juga banyak pengguna lain yang proyeknya bergantung pada kode dengan kesalahan ini.

V501 Ada sub-ekspresi yang identik di sebelah kiri dan di kanan operator '&&': pTS-> pRT_ALD_1 && pTS-> pRT_ALD_1 d3d_svo.cpp 857

 void CSvoRenderer::ConeTracePass(SSvoTargetsSet* pTS) { .... if (pTS->pRT_ALD_1 && pTS->pRT_ALD_1) { static int nPrevWidth = 0; if (....) { .... } else { pTS->pRT_ALD_1->Apply(10, m_nTexStateLinear); pTS->pRT_RGB_1->Apply(11, m_nTexStateLinear); } } .... } 

Kembali ke kode Lumberyard. Dalam kondisi tersebut, pointer pTS-> pRT_ALD_1 yang sama dicentang . Salah satunya seharusnya pTS-> pRT_RGB_1 . Mungkin bahkan setelah penjelasan itu tidak segera mungkin untuk melihat perbedaannya, tetapi ada satu: perbedaannya adalah pada substring pendek ALD dan RGB . Jika Anda diberi tahu bahwa Tinjauan Kode manual sudah cukup, maka perlihatkan contoh ini.

Dan jika contoh ini tidak cukup, maka ada 5 yang lebih mirip.
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan '||' operator :! pTS-> pRT_ALD_0 ||! pTS-> pRT_ALD_0 d3d_svo.cpp 1041
  • V501 Ada sub-ekspresi yang identik ke kiri dan ke kanan operator '&&': m_pRT_AIR_MIN && m_pRT_AIR_MIN d3d_svo.cpp 1808
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '&&': m_pRT_AIR_MAX && m_pRT_AIR_MAX d3d_svo.cpp 1819
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '&&': m_pRT_AIR_SHAD && m_pRT_AIR_SHAD d3d_svo.cpp 1830
  • V501 Ada sub-ekspresi yang identik ke kiri dan ke kanan operator '&&': s_pPropertiesPanel && s_pPropertiesPanel entitasobject.cpp 1700

Dan seperti yang saya janjikan, berikut adalah daftar peringatan V501 yang tersisa tanpa contoh kode:
Buka daftar
  • V501 Ada sub-ekspresi identik 'MaxX <0' di kiri dan di kanan '||' operator. czbufferculler.h 128
  • V501 Ada sub-ekspresi identik 'm_joints [op [1]]. Batas [1] [i]' ke kiri dan ke kanan operator '-'. articulatedentity.cpp 795
  • V501 Ada sub-ekspresi identik 'm_joints [i] .limits [1] [j]' ke kiri dan ke kanan operator '-'. articulatedentity.cpp 2044
  • V501 Ada sub-ekspresi identik 'irect [0] .x + 1 - irect [1] .x >> 31' ke kiri dan ke kanan '|' operator. trimesh.cpp 4029
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1779
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1827
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1865
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1779
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1827
  • V501 Ada sub-ekspresi identik 'b-> mlen <= 0' ke kiri dan ke kanan '||' operator. bstrlib.c 1865
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '-': dd - dd finalizingspline.h 669
  • V501 Ada sub-ekspresi identik 'pVerts [2] - pVerts [3]' di sebelah kiri dan di sebelah kanan operator '^'. roadrendernode.cpp 307
  • V501 Ada sub-ekspresi identik '! PGroup-> GetStatObj ()' di sebelah kiri dan di sebelah kanan '||' operator. terrain_node.cpp 594
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan '||' operator: val == 0 || val == - 0 xmlcpb_attrwriter.cpp 367
  • V501 Ada sub-ekspresi identik 'geom_colltype_solid' ke kiri dan ke kanan '|' operator. attachmentmanager.cpp 1058
  • V501 Ada sub-ekspresi identik '(TriMiddle - RMWPosition)' di kiri dan di kanan '' ' operator. modelmesh.cpp 174
  • V501 Ada sub-ekspresi identik '(sasaran - pAbsPose [b3] .t)' di kiri dan di kanan '' ' operator. posemodifierhelper.cpp 115
  • V501 Ada sub-ekspresi identik '(sasaran - pAbsPose [b4] .t)' di kiri dan di kanan '' ' operator. posemodifierhelper.cpp 242
  • V501 Ada sub-ekspresi identik '(m_eTFSrc == eTF_BC6UH)' di kiri dan di kanan '||' operator. texturestreaming.cpp 983
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '-': q2.vz - q2.vz azentitynode.cpp 102
  • V501 Ada sub-ekspresi identik ke kiri dan ke kanan operator '-': q2.vz - q2.vz entitasnode.cpp 107
  • V501 Ada sub-ekspresi identik 'm_listRect.contains (event-> pos ())' ke kiri dan ke kanan '||' operator. aidebuggerview.cpp 463
  • V501 Ada sub-ekspresi yang identik ke kiri dan ke kanan operator '&&': pObj-> GetParent () && pObj-> GetParent () designerpanel.cpp 253

Tentang posisi kamera dalam game




Ada diagnostik tercepat kedua di PVS-Studio - V502 . Diagnosis ini lebih tua dari beberapa bahasa pemrograman baru, di mana kesalahan seperti itu tidak bisa lagi dibuat. Dan untuk C ++, kesalahan ini akan relevan, mungkin selalu.

Mari kita mulai dengan contoh sederhana kecil.

V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '+'. zipencryptor.cpp 217

 bool ZipEncryptor::ParseKey(....) { .... size_t pos = i * 2 + (v1 == 0xff) ? 1 : 2; RCLogError("....", pos); return false; .... } 

Operasi penambahan memiliki prioritas yang lebih tinggi daripada operator ternary. Oleh karena itu, ungkapan ini memiliki logika penghitungan yang sama sekali berbeda dari yang diasumsikan oleh penulis.

Anda dapat memperbaiki kesalahan dengan cara ini:

 size_t pos = i * 2 + (v1 == 0xff ? 1 : 2); 

V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '-'. 3dengine.cpp 1898

 float C3DEngine::GetDistanceToSectorWithWater() { .... return (bCameraInTerrainBounds && (m_pTerrain && m_pTerrain->GetDistanceToSectorWithWater() > 0.1f)) ? m_pTerrain->GetDistanceToSectorWithWater() : max(camPostion.z - OceanToggle::IsActive() ? OceanRequest::GetOceanLevel() : GetWaterLevel(), 0.1f); } 

Dan berikut ini adalah contoh kode tempat mereka bekerja dengan posisi kamera. Contoh sulit untuk dilihat dengan mata dan ada kesalahan di dalamnya. Untuk publikasi, pemformatan kode telah diubah, tetapi dalam file sumber kode ini bahkan lebih tidak dapat dibaca.

Ada kesalahan dalam substring ini:

 camPostion.z - OceanToggle::IsActive() ? .... : .... 

Jika kamera di game Anda tiba-tiba mulai berperilaku tidak tepat, maka Anda harus tahu bahwa pengembang menyimpannya dalam analisis kode statis: D.

Contoh lain dengan peringatan serupa:

  • V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '-'. scriptbind_ai.cpp 5203
  • V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '+'. qcolumnwidget.cpp 136
  • V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '&&'. shapetool.h 98

CryEngine Legacy


Amazon Lumberyard didasarkan pada kode CryEngine . Dan bukan pada versi terbaiknya. Saya membuat kesimpulan seperti itu dengan melihat laporan penganalisa. Beberapa bug yang ditemukan telah diperbaiki di CryEngine versi terbaru untuk dua ulasan kode saya, tetapi masih ada dalam kode Lumberyard. Selain itu, selama setahun terakhir, alat analisis telah meningkat secara signifikan dan dimungkinkan untuk menemukan kesalahan tambahan yang sekarang ada di dua mesin game. Tetapi dengan Lumberyard situasinya lebih buruk. Amazon pada dasarnya mewarisi semua hutang teknis CryEngine. Tapi tugas teknisnya sendiri, tentu saja, muncul di masing-masing perusahaan sendiri :).

Di bagian ini, saya akan memberi Anda beberapa kesalahan yang diperbaiki di CryEngine versi terbaru, dan sekarang hanya tinggal masalah proyek Lumberyard.

V519 Variabel 'BlendFactor [2]' diberikan nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 1283, 1284. ccrydxgldevicecontext.cpp 1284



Kira-kira emosi semacam itu akan dialami oleh pengembang Lumberyard ketika mereka mengetahui bahwa kesalahan ini hanya ada pada mereka.

Omong-omong, ada dua lagi:

  • V519 Variabel 'm_auBlendFactor [2]' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 919, 920. ccrydxgldevicecontext.cpp 920
  • V519 Variabel 'm_auBlendFactor [2]' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 926, 927. ccrydxgldevicecontext.cpp 927

Ada kesalahan seperti itu:

V546 Anggota kelas diinisialisasi dengan sendirinya: 'eConfigMax (eConfigMax.VeryHigh)'. particleparams.h 1837

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

Di CryEngine kelas ini umumnya ditulis ulang, tetapi di sini kesalahan inisialisasi tetap ada.

V521 Ekspresi seperti itu menggunakan operator ',' berbahaya. Pastikan ungkapan '! Pedang [iWord] .empty (), iWord ++' benar. tacticalpointsystem.cpp 3376

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

Siklus yang mencurigakan yang telah ditulis ulang CryEngine juga.

Kesalahan hidup lebih lama dari yang Anda pikirkan


Pengguna yang mulai menggunakan PVS-Studio untuk pertama kalinya memiliki situasi yang hampir sama: mereka menemukan kesalahan, mengetahui bahwa itu ditambahkan beberapa bulan yang lalu dan senang menyadari bahwa mereka secara ajaib menghindari manifestasi masalah di antara pengguna mereka. Banyak klien kami datang ke penggunaan biasa PVS-Studio tepat setelah cerita seperti itu.

Terkadang, untuk memulai proses kontrol kualitas kode, perusahaan harus mengunjungi situasi seperti itu beberapa kali. Berikut adalah contoh tentang CryEngine dan Lumberyard:

V557 CWE-119 Overray array dimungkinkan. Indeks 'id' menunjuk di luar batas array. gameobjectsystem.cpp 113

 uint32 CGameObjectSystem::GetExtensionSerializationPriority(....) { if (id > m_extensionInfo.size()) { return 0xffffffff; // minimum possible priority } else { return m_extensionInfo[id].serializationPriority; } } 

Seperti yang Anda ketahui, Amazon Lumberyard bukan didasarkan pada CryEngine versi terbaru. Namun demikian, dengan bantuan analisa PVS-Studio, adalah mungkin untuk menemukan kesalahan yang ada sekarang di dua mesin game. Itu perlu untuk memeriksa indeks menggunakan operator '> =' ...

Kesalahan pengindeksan serius. Apalagi ada enam tempat seperti itu ! Ini adalah contoh lain:

V557 CWE-119 Overray array dimungkinkan. Indeks 'indeks' menunjuk di luar batas array. 73iciceseesegroup.cpp 73

 CVehicleSeat* CVehicleSeatGroup::GetSeatByIndex(unsigned index) { if (index >= 0 && index <= m_seats.size()) { return m_seats[index]; } return NULL; } 

Seseorang membuat kesalahan dengan tipe yang sama, dan mereka tidak diperbaiki hanya karena mereka tidak pernah dimasukkan dalam ulasan bug CryEngine.

Peringatan yang tersisa:

  • V557 CWE-119 Overray array dimungkinkan. Indeks 'id' menunjuk di luar batas array. gameobjectsystem.cpp 195
  • V557 CWE-119 Overray array dimungkinkan. Indeks 'id' menunjuk di luar batas array. gameobjectsystem.cpp 290
  • V557 CWE-119 Overray array dimungkinkan. Indeks 'stateId' menunjuk di luar batas array. vehicleanimation.cpp 311
  • V557 CWE-119 Overray array dimungkinkan. Indeks 'stateId' menunjuk di luar batas array. vehicleanimation.cpp 354

Keberadaan panjang kesalahan dalam kode hanya dapat dijelaskan oleh tingkat pengujian proyek yang sesuai. Diyakini bahwa analisis statis hanya menemukan kesalahan dalam kode yang tidak digunakan. Jadi tidak demikian. Pengembang lupa bahwa sebagian besar pengguna secara diam-diam menderita bug yang tidak terlihat dalam program, dan manifestasi dari bug yang sangat jarang ini sering berdampak buruk pada pekerjaan seluruh perusahaan, reputasi dan penjualannya, jika ada.

Berbagai Pemrograman Copy-Paste




Ketika Anda mencapai bagian artikel ini, Anda mungkin memperhatikan bahwa pemrograman Copy-Paste adalah penyebab banyak masalah. Dalam PVS-Studio, pencarian kesalahan tersebut diimplementasikan dalam berbagai diagnostik. Bagian ini akan memberikan contoh copy-paste yang ditemukan dengan V561 .

Berikut adalah contoh kode mencurigakan saat mendeklarasikan variabel dengan nama yang sama dalam cakupan yang tumpang tindih.

V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'pLibrary' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: entityobject.cpp, baris 4703. entitasobject.cpp 4706

 void CEntityObject::OnMenuConvertToPrefab() { .... IDataBaseLibrary* pLibrary = GetIEditor()->Get....; if (pLibrary == NULL) { IDataBaseLibrary* pLibrary = GetIEditor()->Get....; } if (pLibrary == NULL) { QString sError = tr(....); CryMessageBox(....); return; } .... } 

Pointer pLibrary tidak menimpa seperti yang diharapkan. Inisialisasi pointer ini sepenuhnya disalin di bawah kondisi bersama dengan deklarasi tipe.

Saya akan mencantumkan semua tempat serupa:

  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'eType' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: toglsloperand.c, baris 838. toglsloperand.c 1224
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'eType' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: toglsloperand.c, baris 838. toglsloperand.c 1305
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'rSkelPose' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: attachmentmanager.cpp, baris 409. attachmentmanager.cpp 458
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'nThreadID' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: d3dmeshbaker.cpp, line 797. d3dmeshbaker.cpp 867
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'directoryNameList' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: assetimportermanager.cpp, line 720. assetimportermanager.cpp 728
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'pNode' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: breakpointsctrl.cpp, line 340. breakpointsctrl.cpp 349
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'pLibrary' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: prefabobject.cpp, line 1443. prefabobject.cpp 1446
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'pLibrary' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: prefabobject.cpp, line 1470. prefabobject.cpp 1473
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'cmdLine' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: fileutil.cpp, line 110. fileutil.cpp 130
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'sfunctionArgs' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: attributeitemlogiccallbacks.cpp, baris 291. attributeitemlogiccallbacks.cpp 303
  • V561 CWE-563 Mungkin lebih baik untuk memberikan nilai pada variabel 'curveName' daripada mendeklarasikannya lagi. Deklarasi sebelumnya: qgradientselectorwidget.cpp, line 475. qgradientselectorwidget.cpp 488

Daftar panjang ... beberapa tempat yang terdaftar bahkan merupakan salinan lengkap dari contoh yang dijelaskan.

Inisialisasi nilai eigen




Dalam kode mesin game, banyak tempat ditemukan di mana variabel ditugaskan untuk dirinya sendiri. Di suatu tempat kode ini ditinggalkan untuk debugging, di suatu tempat kode dirancang dengan indah (itu juga sering menjadi sumber kesalahan), jadi saya akan memberikan Anda satu bagian dari kode yang paling mencurigakan bagi saya.

V570 Variabel 'behaviourParams.ignoreOnVehicleDestroyed' ditugaskan untuk dirinya sendiri. komponen kendaraan.cpp 168

 bool CVehicleComponent::Init(....) { .... if (!damageBehaviorTable.getAttr(....) { behaviorParams.ignoreOnVehicleDestroyed = false; } else { behaviorParams.ignoreOnVehicleDestroyed = // <= behaviorParams.ignoreOnVehicleDestroyed; // <= } .... } 

Dalam tampilan saat ini, cabang lain tidak diperlukan sama sekali. Tapi mungkin bagian kode ini mengandung kesalahan: mereka ingin menetapkan nilai yang berlawanan ke variabel:

 bValue = !bValue 

Tetapi lebih baik bagi pengembang untuk membiasakan diri dengan hasil diagnosis ini.

Penanganan Masalah




Bagian ini akan memberikan banyak contoh ketika terjadi kesalahan selama penanganan kesalahan.

Contoh 1

V606 Token tanpa pemilik 'nullptr'. dx12rootsignature.cpp 599

 RootSignature* RootSignatureCache::AcquireRootSignature(....) { .... RootSignature* result = new RootSignature(m_pDevice); if (!result->Init(params)) { DX12_ERROR("Could not create root signature!"); nullptr; } m_RootSignatureMap[hash] = result; return result; } } 

Lupa menulis return nullptr; . Sekarang, nilai variabel hasil yang tidak valid akan digunakan di tempat lain dalam kode.

Kode yang sama persis disalin ke satu tempat lagi:

  • V606 Token tanpa pemilik 'nullptr'. dx12rootsignature.cpp 621

Contoh 2

V606 Token tanpa pemilik 'salah'. fillspacetool.cpp 191

 bool FillSpaceTool::FillHoleBasedOnSelectedElements() { .... if (validEdgeList.size() == 2) { .... } if (validEdgeList.empty()) { .... for (int i = 0, iVertexSize(....); i < iVertexSize; ++i) { validEdgeList.push_back(....); } } if (validEdgeList.empty()) // <= { false; // <= fail } std::vector<BrushEdge3D> linkedEdgeList; std::set<int> usedEdgeSet; linkedEdgeList.push_back(validEdgeList[0]); // <= fail .... } 

Contoh yang sangat menarik dari kesalahan dengan pernyataan pengembalian yang hilang. Sekarang situasi mengakses wadah kosong ke indeks dimungkinkan.

Contoh 3

V564 CWE-480 Operator '&' diterapkan pada nilai tipe bool. Anda mungkin lupa menyertakan tanda kurung atau bermaksud menggunakan operator '&&'. toglslinstruction.c 2914

 void SetDataTypes(....) { .... // Check assumption that both the values which MOVC might pick // have the same basic data type. if(!psContext->flags & HLSLCC_FLAG_AVOID_TEMP_REGISTER_ALIASING) { ASSERT(GetOperandDataType(psContext, &psInst->asOperands[2]) == GetOperandDataType(psContext, &psInst->asOperands[3])); } .... } 

Tidak diperiksa benar keberadaan bit di flag. Operator negasi berlaku untuk nilai flag, bukan keseluruhan ekspresi. Tulis dengan benar seperti ini:

 if(!(psContext->flags & ....)) 

Lebih banyak peringatan serupa:

  • V564 CWE-480 The '|' operator diterapkan pada nilai tipe bool. Anda mungkin lupa menyertakan tanda kurung atau bermaksud menggunakan '||' operator. d3dhwshader.cpp 1832
  • V564 CWE-480 Operator '&' diterapkan pada nilai tipe bool. Anda mungkin lupa menyertakan tanda kurung atau bermaksud menggunakan operator '&&'. trackviewdialog.cpp 2112
  • V564 CWE-480 The '|' operator diterapkan pada nilai tipe bool. Anda mungkin lupa menyertakan tanda kurung atau bermaksud menggunakan '||' operator. imagecompiler.cpp 1039

Contoh 4

V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1491

 static std::vector<std::string> PyGetPrefabLibrarys() { CPrefabManager* pPrefabManager = GetIEditor()->GetPrefabMa....; if (!pPrefabManager) { std::runtime_error("Invalid Prefab Manager."); } .... } 

Kesalahan melempar pengecualian. Itu perlu untuk menulis seperti ini:

 throw std::runtime_error("Invalid Prefab Manager."); 

Seluruh daftar kesalahan tersebut:

  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1515
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1521
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1543
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1549
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1603
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1619
  • V596 CWE-390 Objek telah dibuat tetapi tidak digunakan. Kata kunci 'throw' dapat hilang: throw runtime_error (FOO); prefabobject.cpp 1644


Beberapa masalah saat bekerja dengan memori




V549 CWE-688 Argumen pertama dari fungsi 'memcmp' sama dengan argumen kedua. meshutils.h 894

 struct VertexLess { .... bool operator()(int a, int b) const { .... if (m.m_links[a].links.size() != m.m_links[b].links.size()) { res = (m.m_links[a].links.size() < m.m_links[b].links.size()) ? -1 : +1; } else { res = memcmp(&m.m_links[a].links[0], &m.m_links[a].links[0], sizeof(m.m_links[a].links[0]) * m.m_links[a].links.size()); } .... } .... }; 

Kondisi membandingkan ukuran dua vektor. Jika mereka sama, maka di cabang yang lain nilai - nilai elemen pertama vektor dibandingkan menggunakan fungsi memcmp () . Tetapi argumen pertama dan kedua untuk fungsi ini sama! Akses ke elemen array agak rumit. Ada indeks a dan b . Kemungkinan besar, kesalahan ketik ada di dalamnya.

V611 CWE-762 Memori dialokasikan menggunakan operator 'T baru] tetapi dirilis menggunakan operator' hapus '. Pertimbangkan untuk memeriksa kode ini. Mungkin lebih baik menggunakan 'delete [] data;'. vectorn.h 102

 ~vectorn_tpl() { if (!(flags & mtx_foreign_data)) { delete[] data; } } vectorn_tpl& operator=(const vectorn_tpl<ftype>& src) { if (src.len != len && !(flags & mtx_foreign_data)) { delete data; // <= data = new ftype[src.len]; } .... } 

Memori penunjuk data sedang dibebaskan dengan pernyataan yang tidak valid. Operator delete [] harus digunakan di mana-mana .

Kode tidak dapat dijangkau


V779 CWE-561 Kode yang tidak dapat dideteksi terdeteksi. Mungkin saja ada kesalahan. fbxskinimporter.cpp 67

 Events::ProcessingResult FbxSkinImporter::ImportSkin(....) { .... if (BuildSceneMeshFromFbxMesh(....) { context.m_createdData.push_back(std::move(createdData)); return Events::ProcessingResult::Success; // <= } else { return Events::ProcessingResult::Failure; // <= } context.m_createdData.push_back(); // <= fail return Events::ProcessingResult::Success; } 

Semua cabang pernyataan bersyarat berakhir dengan keluar dari fungsi. Namun, beberapa kode tidak dieksekusi.

V779 CWE-561 Kode yang tidak dapat dideteksi terdeteksi. Mungkin saja ada kesalahan. dockablelibrarytreeview.cpp 153

 bool DockableLibraryTreeView::Init(IDataBaseLibrary* lib) { .... if (m_treeView && m_titleBar && m_defaultView) { if (m_treeView->topLevelItemCount() > 0) { ShowTreeView(); } else { ShowDefaultView(); } return true; // <= } else { return false; // <= } emit SignalFocused(this); // <= fail } 

Mudah untuk menemukan kesalahan pada kode ini. Tetapi jika Anda menulis kode untuk waktu yang lama, maka perhatian turun tajam dan kesalahan seperti itu mudah jatuh ke dalam proyek.

V622 CWE-478 Pertimbangkan untuk memeriksa pernyataan 'sakelar'. Mungkin saja operator 'case' pertama hilang. datum.cpp 872

 AZ_INLINE bool IsDataGreaterEqual(....) { switch (type.GetType()) { AZ_Error("ScriptCanvas", false, "...."); return false; case Data::eType::Number: return IsDataGreaterEqual<Data::NumberType>(lhs, rhs); .... case Data::eType::AABB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::AABBType>::GetName()); return false; case Data::eType::OBB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::OBBType>::GetName()); return false; .... } 

Jika switch berisi kode di luar case / statement default , maka switch tidak pernah dieksekusi.

Kesimpulan


Artikel ini mencakup 95 peringatan penganalisa, 25 di antaranya dengan contoh kode. Berapa banyak bahan dari seluruh laporan analisa? Saya dengan cepat menggulir hanya sepertiga dari peringatan tingkat tinggi . Ada juga Medium dan Low, sekelompok diagnosa untuk mencari optimasi, dan peluang analisa yang belum dimanfaatkan lainnya - ini adalah ratusan kesalahan yang lebih jelas dan ribuan peringatan yang belum dijelajahi.

Dan kemudian pembaca perlu bertanya pada dirinya sendiri pertanyaan: "Apakah mungkin dengan pendekatan ini untuk proyek untuk merilis mesin gim yang bagus?". Tidak ada kontrol kualitas kode. Kode CryEngine dengan kesalahan lama diambil sebagai dasar, kesalahan baru ditambahkan. CryEngine sendiri diselesaikan hanya setelah peninjauan kode selanjutnya. Amazon memiliki setiap kesempatan dengan sumber dayanya untuk bekerja ke arah kualitas kode dan merilis mesin game paling keren!

Jangan terlalu kesal. Di antara klien PVS-Studio ada lebih dari tiga puluh perusahaan lain yang terlibat dalam permainan. Anda dapat berkenalan dengan mereka dan produk mereka di halaman " Klien " situs web kami dengan memilih filter "Pengembangan Game". Jadi kita secara bertahap meningkatkan dunia. Mungkin kita bisa meningkatkan Amazon Lumberyard :).

Seorang rekan baru-baru ini menulis sebuah artikel tentang topik kualitas perangkat lunak permainan, saya sarankan Anda tertarik untuk membaca: " Analisis statis dalam industri video game: 10 kesalahan perangkat lunak teratas ."

Tautan untuk mengunduh penganalisa PVS-Studio , bagaimana Anda dapat melakukannya tanpa itu ;-)



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov. Amazon Lumberyard: A Scream of Anguish

Sudahkah Anda membaca artikel dan memiliki pertanyaan?

Source: https://habr.com/ru/post/id416565/


All Articles