Jumlah pembaca kami bertambah, jadi kami menulis artikel berulang kali yang menjelaskan cara menggunakan metodologi analisis kode statis dengan benar. Kami menganggap sangat penting untuk menjelaskan bahwa alat analisis statis tidak boleh digunakan secara sporadis, tetapi secara teratur. Sekali lagi, kami menunjukkan ini dengan contoh praktis, memeriksa kembali proyek Godot.
Gunakan analisa secara teratur
Bersiap untuk berbicara di konferensi pengembang game, saya memutuskan untuk mendapatkan contoh baru dari kesalahan menarik yang dapat dideteksi oleh alat
PVS-Studio . Untuk tujuan ini, beberapa mesin game diuji, salah satunya adalah Godot. Saya tidak menemukan kesalahan khusus yang menarik untuk laporan, tetapi saya ingin menulis artikel tentang kesalahan biasa. Kesalahan-kesalahan ini dengan sangat baik menunjukkan relevansi penggunaan alat analisis kode statis secara teratur.
Perlu dicatat bahwa kami sudah
memeriksa proyek ini pada 2015, dan penulis bekerja dengan kesalahan yang kami tulis.
Di sini Anda dapat melihat komit yang sesuai.
3 tahun telah berlalu. Proyek telah berubah. Alat analisa PVS-Studio juga telah berubah, dan diagnostik baru telah muncul di dalamnya. Oleh karena itu, tidak mengherankan bahwa saya dapat dengan mudah dan cepat menuliskan cukup contoh kesalahan untuk menulis artikel ini.
Namun, ada hal lain yang penting. Ketika mengembangkan Godot atau proyek lainnya, kesalahan baru terus muncul dan diperbaiki. Kesalahan yang tidak terdeteksi "menetap" dalam kode untuk waktu yang lama, dan kemudian banyak dari mereka dapat dideteksi ketika menjalankan analisis kode statis. Karena itu, kadang-kadang ada perasaan salah bahwa analis statis hanya menemukan beberapa kesalahan yang tidak menarik di bagian kode yang jarang digunakan. Tentu saja, ini masalahnya jika Anda menggunakan analisa secara tidak benar dan menjalankannya hanya dari waktu ke waktu, misalnya, sesaat sebelum rilis rilis.
Ya, saat menulis artikel, kami sendiri melakukan satu kali pemeriksaan proyek terbuka. Tetapi kami memiliki tujuan yang berbeda. Kami ingin menunjukkan kemampuan penganalisa kode untuk mendeteksi cacat. Tugas ini tidak ada hubungannya dengan meningkatkan kualitas kode proyek secara keseluruhan dan mengurangi biaya yang terkait dengan koreksi kesalahan.
Sekali lagi tentang hal utama. Titik analisis kode statis bukan untuk menemukan kesalahan lama. Kesalahan lama ini biasanya tidak signifikan, jika tidak mereka akan mengganggu pengguna dan mereka sudah ditemukan dan diperbaiki. Inti dari analisis statis adalah untuk dengan cepat memperbaiki kesalahan dalam kode yang baru ditulis atau dimodifikasi. Ini mengurangi waktu debugging, jumlah keluhan pengguna dan, pada akhirnya, mengurangi anggaran proyek yang dikembangkan.
Sekarang mari kita beralih ke bug yang sangat disukai pembaca publikasi kita.
Kesalahan Salin-Tempel
Mari kita lihat apa yang saya perhatikan saat mempelajari laporan PVS-Studio. Saya akan mulai dengan diagnostik V501 favorit saya, yang menemukan
kesalahan di hampir setiap proyek yang kami periksa :).
Kesalahan N1virtual bool can_export(....) { .... if (!exists_export_template("uwp_" + platform_infix + "_debug.zip", &err) || !exists_export_template("uwp_" + platform_infix + "_debug.zip", &err)) { valid = false; r_missing_templates = true; } .... }
Peringatan PVS-Studio: V501 CWE-570 Ada sub-ekspresi yang identik '! Exists_export_template ("uwp_" + platform_infix + "_debug.zip", & err)' ke kiri dan ke kanan '||' operator. export.cpp 1135
Kesalahan Salin-Tempel Klasik. Panggilan fungsi telah disalin tetapi tidak diedit. Nama file kedua yang akan diproses harus diakhiri dengan "_release.zip".
Kesalahan N2, N3 static String dump_node_code(SL::Node *p_node, int p_level) { .... if (bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW || bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW) { code += scode;
PVS-Studio Peringatan: V501 CWE-570 Ada sub-ekspresi identik 'bnode-> pernyataan [i] -> type == SL :: Node :: TYPE_CONTROL_FLOW' ke kiri dan ke kanan '||' operator. test_shader_lang.cpp 183
void EditorSpinSlider::_notification(int p_what) { if (p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT || p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT) { if (grabbing_spinner) { Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE); grabbing_spinner = false; grabbing_spinner_attempt = false; } } .... }
Peringatan PVS-Studio: V501 CWE-570 Ada sub-ekspresi yang identik 'p_what == MainLoop :: NOTIFICATION_WM_FOCUS_OUT' ke kiri dan ke kanan '||' operator. editor_spin_slider.cpp 157
Saya pikir kesalahannya jelas terlihat dan tidak memerlukan penjelasan apa pun. Sama persis dengan Copy-Paste klasik, seperti pada kasus pertama.
Kesalahan N4 String SoftBody::get_configuration_warning() const { .... Transform t = get_transform(); if ((ABS(t.basis.get_axis(0).length() - 1.0) > 0.05 || ABS(t.basis.get_axis(1).length() - 1.0) > 0.05 || ABS(t.basis.get_axis(0).length() - 1.0) > 0.05)) { if (!warning.empty()) .... }
PVS-Studio Warning: V501 CWE-570 Ada sub-ekspresi yang identik ke kiri dan ke kanan '||' operator. soft_body.cpp 399
Di sini baris pertama disalin dua kali. Tetapi nomor sumbu koordinat hanya diubah di baris kedua. Dan mereka lupa mengedit baris ketiga. Ini tidak lain adalah "
Efek Lini Terakhir ".
Catatan Saat ini, selain "Last Line Effect", saya telah melakukan pengamatan menarik berikut: "
Fungsi paling berbahaya di dunia C / C ++ ", "
Evil hidup dalam fungsi perbandingan ". Dan sekarang saya akan membuat pengumuman artikel baru, tulisan yang saya rencanakan untuk dilakukan dalam waktu dekat. Judul yang berfungsi adalah "0, 1, 2". Itu harus menarik dan instruktif. Saya mengundang Anda untuk berlangganan salah satu saluran agar tidak ketinggalan:
twitter ,
vk.com ,
Instagram ,
telegram dan
rss "old school".
Kesalahan N5 void ScrollContainer::_notification(int p_what) { .... if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this) size.y -= h_scroll->get_minimum_size().y; if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this) size.x -= h_scroll->get_minimum_size().x; .... }
Peringatan PVS-Studio: V778 CWE-682 Ditemukan dua fragmen kode yang serupa. Mungkin, ini adalah kesalahan ketik dan variabel 'v_scroll' harus digunakan alih-alih 'h_scroll'. scroll_container.cpp 249
Mengenai potongan kode ini, saya tidak sepenuhnya yakin bahwa ada kesalahan. Namun, saya setuju dengan penganalisa bahwa blok kedua terlihat sangat mencurigakan. Kemungkinan besar, kode tersebut ditulis menggunakan Copy-Paste, dan pada blok teks kedua mereka lupa mengganti
h_scroll dengan
v_scroll .
Kode mungkin harus seperti ini:
if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this) size.y -= h_scroll->get_minimum_size().y; if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this) size.x -= v_scroll->get_minimum_size().x;
Kesalahan N6Kasus lain di mana fragmen kode yang cukup besar disalin dan gagal diubah. Baris kesalahan ditandai oleh komentar saya "// <=".
void ShaderGLES2::bind_uniforms() { .... const Map<uint32_t, Variant>::Element *E = uniform_defaults.front(); while (E) { int idx = E->key(); int location = version->uniform_location[idx]; if (location < 0) { E = E->next(); continue; } Variant v; v = E->value(); _set_uniform_variant(location, v); E = E->next(); } const Map<uint32_t, CameraMatrix>::Element *C = uniform_cameras.front(); while (C) { int idx = E->key();
Peringatan PVS-Studio: V522 CWE-476 Dereferencing dari null pointer 'E' mungkin terjadi. shader_gles2.cpp 102
Kesalahan terdeteksi secara tidak langsung. Berkat analisis
aliran data, PVS-Studio mengungkapkan bahwa pointer
E mungkin nol pada saat dereferencing.
Kesalahannya adalah bahwa dalam fragmen kode yang disalin mereka lupa mengganti
E dalam
C di satu tempat. Karena kesalahan ini, fungsinya bekerja dengan cara yang sangat aneh dan melakukan hal-hal aneh.
Salah ketik
Kesalahan N7Sulit bagi programmer yang menulis dalam bahasa selain C atau C ++ untuk membayangkan bahwa kesalahan ketik dapat dibuat dengan menulis koma alih-alih tanda bintang *, dan kode akan dikompilasi. Namun demikian, ini benar.
LRESULT OS_Windows::WndProc(....) { .... BITMAPINFO bmi; ZeroMemory(&bmi, sizeof(BITMAPINFO)); bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER); bmi.bmiHeader.biWidth = dib_size.x; bmi.bmiHeader.biHeight = dib_size.y; bmi.bmiHeader.biPlanes = 1; bmi.bmiHeader.biBitCount = 32; bmi.bmiHeader.biCompression = BI_RGB; bmi.bmiHeader.biSizeImage = dib_size.x, dib_size.y * 4; .... }
Peringatan PVS-Studio: V521 CWE-480 Ekspresi seperti itu menggunakan operator ',' berbahaya. Pastikan ekspresinya benar. os_windows.cpp 776
Variabel
bmi.bmiHeader.biSizeImage diberi nilai variabel
dib_size.x . Berikutnya, operator koma ',' dieksekusi, yang
prioritasnya lebih rendah daripada operator '='. Hasil dari ekspresi
dib_size.y * 4 tidak digunakan dengan cara apa pun.
Alih-alih koma dalam ekspresi, operator perkalian '*' harus digunakan. Pertama, ungkapan seperti itu masuk akal. Kedua, di bawah ini ada opsi yang serupa, tetapi sudah benar untuk menginisialisasi variabel yang sama:
bmi.bmiHeader.biSizeImage = dib_size.x * dib_size.y * 4;
Kesalahan N8, N9 void Variant::set(....) { .... int idx = p_index; if (idx < 0) idx += 4; if (idx >= 0 || idx < 4) { Color *v = reinterpret_cast<Color *>(_data._mem); (*v)[idx] = p_value; valid = true; } .... }
Peringatan PVS-Studio: V547 CWE-571 Ekspresi 'idx> = 0 || idx <4 'selalu benar. variant_op.cpp 2152
Indeks apa pun akan dianggap benar. Untuk memperbaiki kesalahan, Anda harus mengganti
|| pada
&& :
if (idx >= 0 && idx < 4) {
Kesalahan logis ini kemungkinan besar muncul karena kecerobohan, oleh karena itu saya cenderung mengaitkannya dengan kesalahan ketik.
Kesalahan yang persis sama dapat diamati pada file yang sama di bawah ini. Kesalahan kesalahan pembiakan, tampaknya, adalah Copy-Paste.
Galat berganda: V547 CWE-571 Ekspresi 'idx> = 0 || idx <4 'selalu benar. variant_op.cpp 2527
Kesalahan N10Contoh kesalahan yang ingin diserukan: WTF?!
void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }
Peringatan PVS-Studio: V621 CWE-835 Pertimbangkan untuk memeriksa operator 'untuk'. Ada kemungkinan bahwa loop akan dieksekusi secara tidak benar atau tidak akan dieksekusi sama sekali. animation_blend_space_1d.cpp 113
Perhatikan kondisi berhenti loop:
i> p_at_index . Itu selalu benar, karena variabel
i diinisialisasi dengan nilai
blend_points_used - 1 . Selain itu, dari dua pemeriksaan sebelumnya mengikuti bahwa
blend_points_used> p_at_index .
Kondisi ini bisa menjadi salah hanya ketika melimpah variabel tanda
i terjadi, yang merupakan perilaku tidak terdefinisi. Selain itu, itu tidak akan mencapai overflow, karena array akan melampaui batas lebih awal.
Di hadapan kami, menurut pendapat saya, adalah kesalahan ketik yang indah ketika mereka menulis '>' bukan '<'. Ya, saya memiliki pandangan mesum tentang keindahan kesalahan :).
Siklus yang benar:
for (int i = blend_points_used - 1; i < p_at_index; i++) {
Kesalahan N11Kasus kesalahan ketik yang sama baiknya dalam kondisi siklus.
void AnimationNodeStateMachineEditor::_state_machine_pos_draw() { .... int idx = -1; for (int i = 0; node_rects.size(); i++) { if (node_rects[i].node_name == playback->get_current_node()) { idx = i; break; } } .... }
Peringatan PVS-Studio: V693 CWE-835 Pertimbangkan untuk memeriksa ekspresi kondisional dari loop. Ada kemungkinan bahwa 'i <X.size ()' harus digunakan alih-alih 'X.size ()'. animation_state_machine_editor.cpp 852
Overflow dari array dapat terjadi, karena nilai
i meningkat tanpa terkendali. Kode aman:
for (int i = 0; i < node_rects.size(); i++) {
Kesalahan N12 GDScriptDataType GDScriptCompiler::_gdtype_from_datatype( const GDScriptParser::DataType &p_datatype) const { .... switch (p_datatype.kind) { .... case GDScriptParser::DataType::NATIVE: { result.kind = GDScriptDataType::NATIVE; result.native_type = p_datatype.native_type; } break; case GDScriptParser::DataType::SCRIPT: { result.kind = GDScriptDataType::SCRIPT; result.script_type = p_datatype.script_type; result.native_type = result.script_type->get_instance_base_type(); } case GDScriptParser::DataType::GDSCRIPT: { result.kind = GDScriptDataType::GDSCRIPT; result.script_type = p_datatype.script_type; result.native_type = result.script_type->get_instance_base_type(); } break; .... }
Peringatan PVS-Studio: V796 CWE-484 Ada kemungkinan bahwa pernyataan 'break' tidak ada dalam pernyataan switch. gdscript_compiler.cpp 135
Sengaja lupa menulis pernyataan
istirahat . Oleh karena itu, ketika masuk ke
case GDScriptParser :: DataType :: SCRIPT, variabel akan diberi nilai, seolah-olah itu adalah
case GDScriptParser :: DataType :: GDSCRIPT .
Kesalahan N13Kesalahan berikut dapat diklasifikasikan sebagai Salin-Tempel. Namun, saya tidak yakin apakah garis pendek seperti itu disalin. Jadi kami akan menganggap ini salah ketik sederhana saat mengetik.
void CPUParticles::_particles_process(float p_delta) { .... if (flags[FLAG_DISABLE_Z]) { p.velocity.z = 0.0; p.velocity.z = 0.0; } .... }
PVS-Studio Warning: V519 CWE-563 Variabel 'p.velocity.z' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 664, 665. cpu_particles.cpp 665
Dua kali penugasan dari variabel yang sama. Di bawah ini Anda dapat melihat fragmen kode berikut:
if (flags[FLAG_DISABLE_Z]) { p.velocity.z = 0.0; p.transform.origin.z = 0.0; }
Kemungkinan besar, untuk kasus pertama seharusnya ditulis dengan cara yang sama.
Kesalahan N14 bool AtlasTexture::is_pixel_opaque(int p_x, int p_y) const { if (atlas.is_valid()) { return atlas->is_pixel_opaque( p_x + region.position.x + margin.position.x, p_x + region.position.y + margin.position.y ); } return true; }
Peringatan PVS-Studio: V751 Parameter 'p_y' tidak digunakan di dalam badan fungsi. tekstur.cpp 1085
Fragmen dari deskripsi diagnostik
V751 :
Alat analisis mendeteksi fungsi yang mencurigakan, salah satu parameter yang tidak pernah digunakan. Pada saat yang sama, parameter lainnya digunakan beberapa kali, yang, mungkin, menunjukkan adanya kesalahan.Seperti yang Anda lihat, ini memang benar, dan sangat mencurigakan. Variabel
p_x digunakan dua kali, dan
p_y tidak digunakan. Kemungkinan besar, harus ditulis:
return atlas->is_pixel_opaque( p_x + region.position.x + margin.position.x, p_y + region.position.y + margin.position.y );
By the way, dalam kode sumber panggilan fungsi ditulis dalam satu baris. Karena itu, kesalahan lebih sulit untuk diperhatikan. Jika pembuat kode menulis argumen yang sebenarnya di kolom, seperti yang saya lakukan di artikel, maka kesalahan akan segera menarik perhatian saya. Ingat bahwa pemformatan tabel sangat berguna dan menghindari banyak kesalahan ketik. Lihat bab "Sejajarkan jenis kode yang sama dengan" tabel "di artikel"
Masalah utama pemrograman, refactoring, dan semua itu . "
Kesalahan N15 bool SpriteFramesEditor::can_drop_data_fw(....) const { .... Vector<String> files = d["files"]; if (files.size() == 0) return false; for (int i = 0; i < files.size(); i++) { String file = files[0]; String ftype = EditorFileSystem::get_singleton()->get_file_type(file); if (!ClassDB::is_parent_class(ftype, "Texture")) { return false; } } .... }
Peringatan PVS-Studio: V767 Akses mencurigakan ke elemen array 'file' dengan indeks konstan di dalam satu loop. sprite_frames_editor_plugin.cpp 602
Loop memproses file yang sama di semua iterasi loop. Ketik di sini:
String file = files[0];
Harus:
String file = files[i];
Kesalahan lainnya
Kesalahan N16 CSGBrush *CSGBox::_build_brush() { .... for (int i = 0; i < 6; i++) { .... if (i < 3) face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); else face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); .... } .... }
Alat analisis PVS-Studio segera menghasilkan dua respons terhadap kode ini:
- V547 CWE-570 Ekspresi 'i> = 3' selalu salah. csg_shape.cpp 939
- V547 CWE-571 Ekspresi 'i> = 3' selalu benar. csg_shape.cpp 941
Memang, operator ternary ini di kedua ekspresi terlihat sangat aneh:
i >= 3 ? -1 : 1
Dalam satu kasus, kondisinya selalu benar, dan dalam kasus lain itu salah. Sulit mengatakan bagaimana kode ini terlihat benar. Mungkin itu hanya berlebihan dan dapat ditulis seperti ini:
for (int i = 0; i < 6; i++) { .... if (i < 3) face_points[j][(i + k) % 3] = v[k]; else face_points[3 - j][(i + k) % 3] = -v[k]; .... }
Saya mungkin salah, dan kode perlu diperbaiki dengan cara yang sama sekali berbeda.
Kesalahan N17Hampir tidak ada kesalahan seperti V595, meskipun mereka biasanya
ditemukan berlimpah di proyek apa pun . Rupanya, kesalahan ini diperbaiki setelah pemeriksaan sebelumnya, dan kemudian kesalahan jenis ini hampir tidak muncul. Saya hanya melihat beberapa kesalahan positif dan satu kesalahan.
bool CanvasItemEditor::_get_bone_shape(....) { .... Node2D *from_node = Object::cast_to<Node2D>( ObjectDB::get_instance(bone->key().from)); .... if (!from_node->is_inside_tree()) return false;
Peringatan PVS-Studio: V595 CWE-476 Pointer 'from_node' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 565, 567. canvas_item_editor_plugin.cpp 565
Pointer
from_node pertama kali
direferensikan untuk memanggil fungsi
is_inside_tree, dan hanya kemudian diperiksa untuk kesetaraan
nullptr . Cek harus ditukar:
if (!from_node) return false; if (!from_node->is_inside_tree()) return false;
Kesalahan N18 enum JoystickList { .... JOY_AXIS_MAX = 10, .... }; static const char *_axes[] = { "Left Stick X", "Left Stick Y", "Right Stick X", "Right Stick Y", "", "", "L2", "R2" }; int InputDefault::get_joy_axis_index_from_string(String p_axis) { for (int i = 0; i < JOY_AXIS_MAX; i++) { if (p_axis == _axes[i]) { return i; } } ERR_FAIL_V(-1); }
Peringatan PVS-Studio: V557 CWE-125 Array overrun dimungkinkan. Nilai indeks 'i' bisa mencapai 9. input_default.cpp 1119
Array
_axes terdiri dari delapan elemen. Dalam hal ini,
JOY_AXIS_MAX konstan, yang menetapkan jumlah iterasi dari loop, adalah 10. Ternyata loop melampaui batas array.
Kesalahan N19Dan fungsi terakhir yang sangat aneh, yang, tampaknya, dirancang untuk menguji sesuatu. Fungsinya panjang, jadi saya akan memberikannya sebagai gambar (klik gambar untuk memperbesar).
Peringatan PVS-Studio: V779 CWE-561 Kode yang tidak dapat dideteksi terdeteksi. Mungkin saja ada kesalahan. test_math.cpp 457
Ada beberapa
pernyataan pengembalian tanpa syarat dalam suatu fungsi. Dalam gambar saya menandainya dengan oval merah. Tampaknya beberapa unit test yang berbeda dikumpulkan untuk fungsi ini, tetapi lupa untuk menghapus
NULL pengembalian ekstra. Akibatnya, fungsi tidak memeriksa apa yang harus diperiksa. Hampir seluruh isi fungsi terdiri dari kode yang tidak dapat dijangkau.
Mungkin, tentu saja, ini semacam ide licik. Tapi menurut saya ini terjadi secara kebetulan dan kode harus diperbaiki.
Mari kita akhiri ini. Tentunya, jika Anda mengamati laporan penganalisa dengan cermat, Anda dapat menemukan kesalahan lainnya. Namun, bahkan menulis lebih dari cukup untuk menulis artikel. Maka itu akan menjadi membosankan bagi saya dan pembaca :).
Kesimpulan
Artikel ini menjelaskan kesalahan yang tidak akan ada jika kode dianalisis secara teratur menggunakan PVS-Studio. Namun, yang lebih penting, menggunakan analisis reguler, orang dapat segera menemukan dan menghilangkan banyak kesalahan lainnya. Kolega saya menjelaskan ide ini secara lebih rinci dalam catatannya: "
Filsafat analisis kode statis: kami memiliki 100 programmer, penganalisa menemukan beberapa kesalahan, apakah itu tidak berguna? “. Saya sangat merekomendasikan menghabiskan 10 menit membaca artikel pendek tapi sangat penting ini.
Terima kasih atas perhatian anda Saya mengundang semua orang untuk mengunduh dan mencoba analisis statis PVS-Studio untuk menguji proyek Anda sendiri.
Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Andrey Karpov.
Godot: Tentang Penggunaan Analisis Statis Secara Reguler .