Musim dingin sudah di luar jendela, tahun cenderung berakhir, yang berarti sudah waktunya untuk mempertimbangkan kesalahan paling menarik yang terdeteksi oleh penganalisis PVS-Studio pada tahun 2020.
Perlu dicatat bahwa tahun lalu ditandai dengan sejumlah besar aturan diagnostik baru, yang pemicuannya memungkinkan mereka untuk masuk ke puncak ini. Kami juga terus meningkatkan inti analyzer dan menambahkan skenario baru untuk penggunaannya, Anda dapat membaca tentang semua ini di blog kami . Jika Anda tertarik dengan bahasa lain yang didukung oleh penganalisis kami (C, C # dan Java), lihat artikel rekan saya. Sekarang mari kita langsung ke bug paling berkesan yang ditemukan oleh PVS-Studio selama setahun terakhir.
Juara kesepuluh: Divisi modulo per satu
V1063 Operasi modulo by 1 tidak ada artinya. Hasilnya akan selalu nol. llvm-stress.cpp 631
void Act() override {
....
// If the value type is a vector, and we allow vector select,
// then in 50% of the cases generate a vector select.
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
unsigned NumElem =
cast<FixedVectorType>(Val0->getType())->getNumElements();
CondTy = FixedVectorType::get(CondTy, NumElem);
}
....
}
Pengembang ingin mendapatkan nilai acak antara 0 dan 1 dengan menggunakan pembagian modulo. Namun, operasi seperti X% 1 akan selalu menghasilkan 0. Dalam kasus ini, akan benar untuk menulis ulang kondisi sebagai berikut:
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))
Kesalahan ini disertakan di bagian atas artikel: " Memeriksa Clang 11 dengan PVS-Studio ".
Tempat kesembilan: Empat cek
PVS-Studio mengeluarkan empat peringatan untuk bagian kode berikutnya:
- V560 Bagian dari ekspresi bersyarat selalu benar: x> = 0. editor.cpp 1137
- V560 Bagian dari ekspresi kondisional selalu benar: y> = 0. editor.cpp 1137
- V560 Bagian dari ekspresi bersyarat selalu benar: x <40. editor.cpp 1137
- V560 Bagian dari ekspresi bersyarat selalu benar: y <30. editor.cpp 1137
int editorclass::at( int x, int y )
{
if(x<0) return at(0,y);
if(y<0) return at(x,0);
if(x>=40) return at(39,y);
if(y>=30) return at(x,29);
if(x>=0 && y>=0 && x<40 && y<30)
{
return contents[x+(levx*40)+vmult[y+(levy*30)]];
}
return 0;
}
Semua peringatan adalah untuk pernyataan if yang terakhir . Masalahnya adalah bahwa keempat pemeriksaan yang dilakukan di dalamnya akan selalu mengembalikan nilai benar . Saya tidak akan mengatakan bahwa ini adalah kesalahan serius, tetapi ternyata cukup lucu. Secara umum, pemeriksaan ini berlebihan dan dapat dihapus.
Kesalahan ini masuk ke atas dari artikel: " VVVVVV ??? VVVVVV !!! ".
Tempat kedelapan: hapus, bukan hapus []
V611 Memori dialokasikan menggunakan operator 'T baru []' tetapi dilepaskan menggunakan operator 'hapus'. Pertimbangkan untuk memeriksa kode ini. Mungkin lebih baik menggunakan 'delete [] poke_data;'. CCDDE.CPP 410
BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
....
char *poke_data = new char [length + 2*sizeof(int)]; // <=
....
if(DDE_Class->Poke_Server( .... ) == FALSE) {
CCDebugString("C&C95 - POKE failed!\n");
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (FALSE);
}
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (TRUE);
}
Penganalisis telah mendeteksi kesalahan terkait fakta bahwa memori telah dialokasikan dan dibebaskan dengan cara yang tidak kompatibel. Untuk mengosongkan memori yang dialokasikan untuk larik, gunakan operator delete [] , bukan hapus .
Kesalahan ini berhasil mencapai puncak dari artikel: " Kode game Command & Conquer: bug dari tahun 90-an. Volume dua ".
Tempat ketujuh: Buffer di luar batas
Mari kita lihat fungsi net_hostname_get yang akan digunakan selanjutnya.
#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
#endif
Dalam kasus ini, selama praproses, opsi yang terkait dengan cabang #else dipilih . Artinya, dalam file yang diproses sebelumnya, fungsinya diimplementasikan sebagai berikut:
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
Fungsi mengembalikan pointer ke array 7 byte (memperhitungkan nol terminal di akhir string).
Sekarang mari kita lihat kode yang mengarah ke luapan larik.
static int do_net_init(void)
{
....
(void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
....
}
Peringatan PVS-Studio: V512 [CWE-119] Panggilan fungsi 'memcpy' akan menyebabkan buffer 'net_hostname_get ()' berada di luar jangkauan. log_backend_net.c 114
Setelah pra-pemrosesan , MAX_HOSTNAME_LEN meluas sebagai berikut:
(void)memcpy(hostname, net_hostname_get(),
sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));
Karenanya, saat menyalin data, terjadi overrun dari string literal. Bagaimana hal ini akan mempengaruhi eksekusi program sulit untuk diprediksi, karena mengarah pada perilaku yang tidak terdefinisi.
Bug ini berhasil sampai ke bagian atas artikel: " Menyelidiki kualitas kode sistem operasi Zephyr ".
Tempat keenam: Sesuatu yang sangat aneh
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
memcpy(cpy_mntpt, mntpt, strlen(mntpt));
}
return cpy_mntpt;
}
Peringatan PVS-Studio: V575 [CWE-628] Fungsi 'memcpy' tidak menyalin seluruh string. Gunakan fungsi 'strcpy / strcpy_s' untuk mempertahankan null terminal. shell.c 427
Seseorang mencoba membuat analog dari fungsi strdup , tetapi gagal.
Mari kita mulai dengan memperingatkan penganalisis. Ia mengatakan bahwa fungsi memcpy menyalin baris tersebut, tetapi tidak akan menyalin terminal nol, yang sangat mencurigakan.
Tampaknya terminal 0 ini disalin di sini:
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
Tapi tidak! Berikut kesalahan ketik yang menyebabkan terminal nol disalin ke dalamnya sendiri! Perhatikan bahwa penulisan akan menuju ke larik mntpt , bukan ke cpy_mntpt . Akibatnya, fungsi mntpt_prepare mengembalikan string null terminal yang tidak lengkap.
Faktanya, programmer ingin menulis seperti ini:
((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';
Namun, masih belum jelas mengapa hal itu dilakukan begitu sulit! Kode ini dapat disederhanakan menjadi berikut:
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
strcpy(cpy_mntpt, mntpt);
}
return cpy_mntpt;
}
Bug ini berhasil mencapai bagian atas artikel yang disebutkan di atas: " Memeriksa kualitas kode sistem operasi Zephyr ".
Tempat kelima: Perlindungan luapan salah
V547 [CWE-570] Ekspresi 'rel_wait <0' selalu salah. Nilai tipe unsigned tidak pernah <0. os_thread_windows.c 359
static DWORD
get_rel_wait(const struct timespec *abstime)
{
struct __timeb64 t;
_ftime64_s(&t);
time_t now_ms = t.time * 1000 + t.millitm;
time_t ms = (time_t)(abstime->tv_sec * 1000 +
abstime->tv_nsec / 1000000);
DWORD rel_wait = (DWORD)(ms - now_ms);
return rel_wait < 0 ? 0 : rel_wait;
}
Dalam kasus ini, variabel rel_wait adalah tipe DWORD yang tidak bertanda tangan . Ini berarti perbandingan rel_wait <0 tidak masuk akal, karena hasilnya selalu benar.
Kesalahan itu sendiri tidak terlalu menarik. Tapi ternyata menarik dengan cara mereka mencoba memperbaikinya. Ternyata perubahan itu tidak diperbaiki, tetapi hanya menyederhanakan kodenya. Anda dapat membaca lebih lanjut tentang cerita ini di artikel oleh rekan saya: " Mengapa PVS-Studio tidak menawarkan pengeditan kode otomatis ".
Kesalahan masuk ke atas dari artikel: " Analisis statis dari kode koleksi perpustakaan PMDK dari Intel dan kesalahan yang bukan kesalahan ".
Tempat keempat: Jangan menulis ke std, saudara
V1061 Memperluas namespace 'std' dapat mengakibatkan perilaku tidak terdefinisi. ukuran_iterator.hh 210
// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
util::SizedIterator second)
{
util::swap(*first, *second);
}
} // namespace std
Artikel tempat pemicu diambil: " Menganalisis kode proyek DeepSpeech atau mengapa Anda tidak boleh menulis di namespace std " menjelaskan secara rinci mengapa Anda tidak boleh melakukan ini.
Tempat ketiga: Scrollbar, yang gagal
V501 . Ada sub-ekspresi yang identik di kiri dan kanan operator '-': bufferHeight - bufferHeight TermControl.cpp 592
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(bufferHeight - bufferHeight);
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}
Inilah yang disebut "memicu dengan sejarah". Dalam kasus ini, karena kesalahan, scrollbar di Terminal Windows tidak berfungsi. Seluruh artikel ditulis berdasarkan bug ini, di mana kolega saya melakukan penelitian dan mencari tahu mengapa ini terjadi. Apakah kamu tertarik? Ini dia: "The Scrollbar That Couldn't ".
Tempat kedua: radius dan ketinggian yang membingungkan
Dan sekali lagi kita akan berbicara tentang beberapa peringatan penganalisis:
- V764 Kemungkinan urutan argumen salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 791
- V764 Kemungkinan urutan argumen salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 833
- V764 Kemungkinan urutan argumen salah diteruskan ke fungsi 'CreateWheel': 'height' dan 'radius'. StandardJoints.cpp 884
Berikut adalah pemanggilan fungsi:
NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);
Dan seperti inilah tampilan iklannya:
static NewtonBody* CreateWheel (DemoEntityManager* const scene,
const dVector& location, dFloat radius, dFloat height)
Argumen dibalik dalam pemanggilan fungsi.
Kesalahan ini berhasil mencapai puncak dari artikel: " Memeriksa ulang Newton Game Dynamics dengan penganalisis statis PVS-Studio ".
Tempat pertama: Menghapus hasilnya
V519 Variabel 'color_name' diberi nilai dua kali berturut-turut. Mungkin ini salah. Periksa baris: 621, 627. string.cpp 627
static bool parseNamedColorString(const std::string &value,
video::SColor &color)
{
std::string color_name;
std::string alpha_string;
size_t alpha_pos = value.find('#');
if (alpha_pos != std::string::npos) {
color_name = value.substr(0, alpha_pos);
alpha_string = value.substr(alpha_pos + 1);
} else {
color_name = value;
}
color_name = lowercase(value); // <=
std::map<const std::string, unsigned>::const_iterator it;
it = named_colors.colors.find(color_name);
if (it == named_colors.colors.end())
return false;
....
}
Fungsi ini harus mengurai nama warna dengan parameter transparansi dan mengembalikan kode heksadesimalnya. Bergantung pada hasil pemeriksaan kondisi, baik hasil pemisahan baris atau salinan argumen fungsi diteruskan ke variabel color_name .
Namun, kemudian dalam fungsi huruf kecil () , bukan string yang dihasilkan itu sendiri yang diubah menjadi huruf kecil, tetapi argumen fungsi asli. Akibatnya, kita hanya akan kehilangan warna yang seharusnya dikembalikan parseNamedColorString () .
color_name = lowercase(color_name);
Kesalahan ini berhasil mencapai puncak dari artikel: " PVS-Studio: Analisis permintaan penarikan di Azure DevOps menggunakan agen yang dihosting sendiri ".
Kesimpulan
Selama setahun terakhir, kami menemukan banyak bug dalam proyek open source. Ini adalah kesalahan salin-tempel umum, kesalahan konstan, kebocoran memori, dan banyak masalah lainnya. Alat analisa kami tidak berhenti, dan di atas ada beberapa positif dari diagnosa baru yang ditulis tahun ini.
Semoga Anda menikmati bug yang dikumpulkan. Secara pribadi, menurut saya mereka cukup menarik. Tetapi, tentu saja, visi Anda mungkin berbeda dengan saya, jadi Anda dapat membuat "10 Teratas ..." dengan membaca artikel dari blog kami atau dengan melihat daftar kesalahan yang ditemukan oleh PVS-Studio dalam proyek sumber terbuka.
Saya juga membawa ke artikel perhatian Anda dengan 10 kesalahan C ++ teratas dalam beberapa tahun terakhir: 2016 , 2017 , 2018 , 2019 .
Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Vladislav Stolyarov. 10 Bug Teratas Ditemukan di Proyek C ++ pada tahun 2020 .