Kesalahan yang tidak ditemukan oleh analisis kode statis karena tidak digunakan

Pembaca artikel kami kadang-kadang mencatat bahwa penganalisis kode statis PVS-Studio mendeteksi sejumlah besar kesalahan yang tidak signifikan dan tidak mempengaruhi aplikasi. Benar-benar begitu. Sebagian besar, bug penting telah diperbaiki karena pengujian manual, umpan balik pengguna, dan metode mahal lainnya. Pada saat yang sama, banyak kesalahan ini dapat ditemukan pada tahap penulisan kode dan diperbaiki dengan kehilangan waktu, reputasi, dan uang yang minimal. Artikel ini akan memberikan beberapa contoh kesalahan nyata, yang bisa segera diperbaiki, jika penulis proyek menggunakan analisis kode statis.

Idenya sangat sederhana. Kami akan mencari contoh permintaan tarik di GitHub yang menentukan bahwa masalah adalah perbaikan bug. Kemudian kita akan mencoba menemukan bug ini menggunakan penganalisis kode statis PVS-Studio. Jika kesalahan dapat ditemukan oleh penganalisa, maka itu adalah bug yang bisa ditemukan pada tahap penulisan kode. Semakin dini bug diperbaiki, semakin murah biayanya.

Sayangnya, GitHub mengecewakan kami dan kami tidak berhasil membuat artikel besar tentang masalah ini. GitHub sendiri memiliki kesalahan (atau fitur) yang tidak memungkinkan Anda untuk mencari komentar permintaan tarik dalam proyek yang hanya ditulis dalam bahasa pemrograman tertentu. Atau saya tidak tahu cara memasaknya. Meskipun demikian saya menentukan untuk mencari komentar dalam proyek C, C ++, C #, hasilnya diberikan untuk semua bahasa, termasuk PHP, Python, JavaScript, dan lainnya. Sebagai hasilnya, mencari kasus yang cocok telah terbukti sangat membosankan, dan saya akan memberikan beberapa contoh saja. Namun, mereka cukup untuk menunjukkan kegunaan alat analisis kode statis bila digunakan secara teratur.

Bagaimana jika bug telah ditangkap pada tahap paling awal? Jawabannya sederhana: pemrogram tidak harus menunggu untuk itu muncul sendiri, kemudian mencari dan memperbaiki kode yang rusak.

Mari kita lihat kesalahan yang bisa segera terdeteksi oleh PVS-Studio:

Contoh pertama diambil dari proyek SatisfactoryModLoader. Sebelum memperbaiki kesalahan, kode tampak sebagai berikut:

// gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } } 

Kode ini mengandung kesalahan, bahwa PVS-Studio akan segera mengeluarkan peringatan kepada:

Fungsi V591 Non-void harus mengembalikan nilai. ModFunctions.cpp 44

Fungsi di atas tidak memiliki pernyataan pengembalian , sehingga akan mengembalikan nilai yang tidak ditentukan secara formal. Programmer tidak menggunakan penganalisa kode, jadi dia harus mencari bug sendiri. Fungsi setelah pengeditan:

 // gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; PVOID func = NULL; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { func = reg.func; found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } return func; } 

Anehnya, dalam komit, penulis menandai bug sebagai kritis: " bug kritis tetap di mana fungsi API tidak dikembalikan ".

Dalam komit kedua dari sejarah proyek mc6809, suntingan diperkenalkan dalam kode berikut:

 void mc6809dis_direct( mc6809dis__t *const dis, mc6809__t *const cpu, const char *const op, const bool b16 ) { assert(dis != NULL); assert(op != NULL); addr.b[MSB] = cpu->dp; addr.b[LSB] = (*dis->read)(dis, dis->next++); ... if (cpu != NULL) { ... } } 

Penulis hanya mengoreksi satu baris. Dia mengganti ekspresi

 addr.b[MSB] = cpu->dp; 

untuk yang berikutnya

 addr.b[MSB] = cpu != NULL ? cpu->dp : 0; 

Dalam versi kode lama tidak ada pemeriksaan untuk pointer nol. Jika itu terjadi sehingga null pointer dilewatkan ke fungsi mc6809dis_direct sebagai argumen kedua, dereferensi akan terjadi di tubuh fungsi. Hasilnya menyedihkan dan tidak dapat diprediksi .

Null pointer dereference adalah salah satu pola paling umum yang kita ketahui: "Ini bukan bug kritis. Siapa yang peduli itu berkembang dalam kode? Jika dereference terjadi, program akan diam-diam crash dan hanya itu. ” Sungguh aneh dan sedih mendengar ini dari programmer C ++, tetapi hidup terjadi.

Bagaimanapun, dalam proyek ini dereference seperti itu telah berubah menjadi bug, seperti subjek komit memberi tahu kita: " Bug fix --- NULL dereference ".

Jika pengembang proyek menggunakan PVS-Studio, ia bisa memeriksa dan menemukan peringatan itu dua setengah bulan yang lalu. Inilah saat bug diperkenalkan. Ini peringatannya:

V595 Penunjuk 'cpu' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 1814, 1821. mc6809dis.c 1814

Dengan demikian, bug akan diperbaiki pada saat kemunculannya, yang akan menghemat waktu dan saraf pengembang :).

Contoh perbaikan menarik lainnya ditemukan dalam proyek libmorton.

Kode yang harus diperbaiki:

 template<typename morton> inline bool findFirstSetBitZeroIdx(const morton x, unsigned long* firstbit_location) { #if _MSC_VER && !_WIN64 // 32 BIT on 32 BIT if (sizeof(morton) <= 4) { return _BitScanReverse(firstbit_location, x) != 0; } // 64 BIT on 32 BIT else { *firstbit_location = 0; if (_BitScanReverse(firstbit_location, (x >> 32))) { // check first part firstbit_location += 32; return true; } return _BitScanReverse(firstbit_location, (x & 0xFFFFFFFF)) != 0; } #elif _MSC_VER && _WIN64 .... #elif __GNUC__ .... #endif } 

Dalam editnya, seorang programmer mengganti ungkapan " firstbit_location + = 32 " dengan " * firstbit_location + = 32 ". Pemrogram berharap bahwa 32 akan ditambahkan ke nilai variabel yang dirujuk oleh firstbit_location pointer, tetapi 32 ditambahkan ke pointer itu sendiri. Nilai pointer yang berubah tidak lagi digunakan di mana pun dan nilai variabel yang diharapkan tetap tidak berubah.

PVS-Studio akan mengeluarkan peringatan untuk kode ini:

V1001 Variabel 'firstbit_location' diberikan tetapi tidak digunakan pada akhir fungsi. morton_common.h 22

Nah, apa yang sangat buruk tentang ekspresi yang dimodifikasi tetapi tidak digunakan lebih lanjut? Diagnostik V1001 sepertinya tidak dimaksudkan untuk mendeteksi bug yang sangat berbahaya. Meskipun demikian, ia menemukan kesalahan penting yang mempengaruhi logika program.

Selain itu, ternyata kesalahan itu tidak mudah ditemukan! Tidak hanya telah di program sejak file dibuat, tetapi juga telah mengalami banyak pengeditan di garis tetangga dan ada di proyek selama 3 (!) Tahun! Selama ini logika program rusak, dan tidak berfungsi seperti yang diharapkan pengembang. Jika mereka menggunakan PVS-Studio, bug tersebut akan terdeteksi lebih awal.

Pada akhirnya, mari kita lihat contoh bagus lainnya. Ketika saya sedang mengumpulkan perbaikan bug di GitHub, saya menemukan perbaikan dengan konten berikut beberapa kali. Kesalahan yang diperbaiki ada di sini:

 int kvm_arch_prepare_memory_region(...) { ... do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; ... if (vma->vm_flags & VM_PFNMAP) { ... phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; ... } ... } while (hva < reg_end); ... } 

PVS-Studio mengeluarkan peringatan untuk cuplikan kode ini:

V629 Pertimbangkan untuk memeriksa ekspresi 'vma-> vm_pgoff << 12'. Pergeseran bit dari nilai 32-bit dengan ekspansi selanjutnya ke tipe 64-bit. mmu.c 1795

Saya memeriksa deklarasi variabel, yang digunakan dalam ekspresi " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; " dan menemukan bahwa kode yang diberikan di atas sama dengan contoh sintetik berikut:

 void foo(unsigned long a, unsigned long b) { unsigned long long x = (a << 12) + b; } 

Jika nilai variabel 32-bit lebih besar dari 0xFFFFF , 12 bit tertinggi akan memiliki setidaknya satu nilai nonnull. Setelah menggeser variabel ini ke kiri, bit signifikan ini akan hilang, sehingga menghasilkan informasi yang salah ditulis dalam x.

Untuk menghilangkan kehilangan bit tinggi, pertama-tama kita perlu melemparkan ke tipe lama yang tidak ditandatangani dan hanya setelah ini menggeser variabel:

 pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT; pa += vm_start - vma->vm_start; 

Dengan cara ini, nilai yang benar akan selalu ditulis dalam pa.

Itu akan baik-baik saja tetapi bug ini, sama dengan contoh pertama dari artikel, juga ternyata sangat penting. Penulisnya menulis tentang itu di komentar. Selain itu, kesalahan ini menemukan jalannya ke sejumlah besar proyek. Untuk sepenuhnya menghargai skala tragedi, saya sarankan melihat jumlah hasil ketika mencari perbaikan bug ini di GitHub. Menakutkan, bukan?

Jadi saya telah mengambil pendekatan baru untuk menunjukkan manfaat penggunaan penganalisa kode statis biasa. Saya harap Anda menikmatinya. Unduh dan coba penganalisa kode statis PVS-Studio untuk memeriksa proyek Anda sendiri. Pada saat penulisan, ini memiliki sekitar 700 aturan diagnostik yang diterapkan untuk mendeteksi berbagai pola kesalahan. Mendukung C, C ++, C # dan Java.