Kesalahan yang tidak ditemukan oleh analisis kode statis karena tidak digunakan

Dari waktu ke waktu, pembaca artikel kami tentang pengecekan proyek sumber terbuka memperhatikan bahwa penganalisa kode statis PVS-Studio mendeteksi sejumlah besar kesalahan yang tidak signifikan atau tidak mempengaruhi aplikasi sama sekali. Memang benar. Sebagian besar bug penting telah diperbaiki berkat pengujian manual, ulasan pengguna, dan metode mahal lainnya. Pada saat yang sama, banyak kesalahan ini dapat ditemukan bahkan pada tahap penulisan kode dan diperbaiki dengan kehilangan waktu, reputasi, dan uang yang minimal. Artikel ini akan memberikan beberapa contoh kesalahan nyata yang akan segera diperbaiki jika penulis proyek menggunakan analisis kode statis.

Idenya sangat sederhana. Mari kita lihat GitHub untuk contoh permintaan tarik, komentar yang menunjukkan bahwa ini adalah perbaikan bug. Dan cobalah untuk menemukan kesalahan ini menggunakan penganalisis kode statis PVS-Studio. Jika kesalahan dikoreksi ditemukan oleh penganalisa, maka ini berarti bahwa itu dapat diperbaiki bahkan pada tahap penulisan kode. Dan semakin cepat kesalahan diperbaiki, semakin murah.

Sayangnya, GitHub mengecewakan kami dan tidak mengizinkan untuk membuat artikel yang bagus tentang topik ini. GitHub sendiri juga memiliki kesalahan (atau fitur) yang tidak memungkinkan Anda untuk mencari komentar dalam permintaan tarik dalam proyek yang hanya ditulis dalam bahasa pemrograman tertentu. Yah, atau aku tidak "tahu cara memasaknya." Terlepas dari apa yang saya katakan kepada Anda untuk mencari komentar dalam proyek di C ++, C # atau Java, hasilnya ditampilkan dalam semua bahasa, termasuk PHP, Python, JavaScript dan sebagainya. Akibatnya, mencari kasus yang cocok adalah tugas yang sangat membosankan, dan saya akan membatasi diri hanya pada beberapa contoh. Namun, mereka cukup untuk menunjukkan kegunaan alat analisis kode statis bila digunakan secara teratur.

Bagaimana jika kesalahannya diketahui lebih awal? Jawabannya sederhana: programmer tidak harus menunggu manifestasinya, dan kemudian mencari dan memperbaiki kode yang rusak.
Mari kita lihat kesalahan yang bisa segera dideteksi PVS-Studio:

Contoh pertama diambil dari proyek SatisfactoryModLoader. Sebelum memperbaiki kesalahan, kode tampak seperti ini:

// 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 berisi kesalahan bahwa PVS-Studio akan segera memberikan peringatan:

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

Fungsi di atas tidak memiliki pengembalian , sehingga akan mengembalikan nilai yang tidak ditentukan secara formal. Programmer tidak menggunakan penganalisa kode, jadi dia harus mencari kesalahan 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 menunjukkan bug sebagai kritis: " bug kritis tetap di mana fungsi API tidak dikembalikan ".

Dalam komit kedua dari sejarah proyek mc6809, koreksi dilakukan terhadap 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; 

pada ekspresi

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

Dalam versi kode yang lama, tidak ada pemeriksaan CPU yang dilakukan untuk kesetaraan ke pointer nol. Jika tiba-tiba ternyata null pointer dilewatkan ke fungsi mc6809dis_direct sebagai argumen kedua, maka itu akan ditinjau kembali dalam tubuh fungsi. Hasilnya menyedihkan dan tidak dapat diprediksi .

Null pointer dereferencing adalah salah satu pola paling umum yang kita ketahui: “Ini bukan kesalahan kritis. Yah, dia hidup dalam kode dan kehidupan, dan jika dereferencing terjadi, program akan diam-diam macet dan itu saja. " Sungguh aneh dan sedih mendengar ini dari programmer C ++, tetapi hidup adalah hidup.

Bagaimanapun, dalam proyek ini, dereferencing seperti itu masih berubah menjadi bug, seperti header komit memberitahu kita: " Perbaikan bug --- NULL dereference ".

Jika pengembang proyek menggunakan PVS-Studio, ia akan dapat memeriksa kodenya dan mendeteksi peringatan dua setengah bulan yang lalu (itu adalah berapa banyak yang telah berlalu sejak kesalahan dibuat):

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

Dengan demikian, kesalahan akan dihilangkan bahkan pada saat penampilannya, yang akan menghemat waktu dan, mungkin, saraf pengembang :).

Contoh edit lain yang menarik ditemukan di proyek libmorton.

Kode yang 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 pengeditannya, programmer mengganti ekspresi " firstbit_location + = 32 " dengan " * firstbit_location + = 32 ". Pemrogram mengharapkan angka 32 ditambahkan ke nilai variabel yang terikat oleh firstbit_location pointer, tetapi ditambahkan langsung ke pointer. Nilai pointer yang berubah tidak digunakan di tempat lain, dan nilai yang diharapkan dari variabel 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

Tampaknya apa yang bisa mengerikan dalam ekspresi yang dimodifikasi, tetapi selanjutnya tidak digunakan? Diagnostik V1001 jelas tidak terlihat seperti dimaksudkan untuk mengidentifikasi bug yang sangat berbahaya. Meskipun demikian, ia menemukan kesalahan penting yang mempengaruhi logika program.

Selain itu, ternyata kesalahan ini tidak mudah ditemukan! Tidak hanya dia dalam program sejak file dibuat, dia juga selamat dari banyak pengeditan di baris yang berdekatan dan ada dalam proyek selama 3 ( ! ) Tahun! Selama ini, logika program rusak, dan tidak berfungsi persis seperti yang diharapkan pengembang. Jika mereka menggunakan PVS-Studio, maka kesalahannya akan terdeteksi jauh lebih awal.

Pada akhirnya, perhatikan contoh indah lainnya. Ketika saya mengumpulkan perbaikan bug di GitHub, saya menemukan beberapa kali perbaikan dengan konten ini . Bug diperbaiki 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 bagian 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 melihat deklarasi variabel yang digunakan dalam ekspresi " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; ", dan menemukan bahwa kode di atas mirip dengan contoh sintetis berikut:

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

Jika nilai variabel 32-bit a lebih besar dari 0xFFFFF , maka 12 bit paling signifikan akan memiliki setidaknya satu nilai bukan nol. Setelah menerapkan operasi shift kiri ke variabel ini, bit signifikan ini akan hilang, akibatnya informasi yang salah akan ditulis ke x .

Untuk menghilangkan kehilangan bit tinggi, Anda harus terlebih dahulu melemparkan ke tipe unsigned lama , dan hanya setelah itu melakukan operasi shift:

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

Maka pa akan selalu menulis nilai yang benar.

Semuanya akan baik-baik saja, tetapi bug ini, seperti contoh pertama dari artikel, juga ternyata sangat kritis, karena penulis komit menulis secara terpisah dalam komentarnya. Selain itu, kesalahan ini baru saja masuk 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 mengambil pendekatan baru untuk menunjukkan manfaat menggunakan penganalisa kode statis secara teratur. Saya harap Anda menikmatinya. Unduh dan coba penganalisa kode statis PVS-Studio untuk menguji proyek Anda sendiri. Pada saat penulisan, ini telah menerapkan sekitar 700 aturan diagnostik untuk mendeteksi berbagai pola kesalahan. C, C ++, C # dan Java didukung.

Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: George Gribkov. Kesalahan yang tidak ditemukan oleh analisis kode statis karena tidak digunakan