10 Bug Top Ditemukan di Proyek C ++ pada 2019

Gambar 7

Satu tahun lagi akan berakhir, dan ini adalah waktu yang tepat untuk membuat diri Anda secangkir kopi dan membaca kembali ulasan bug yang dikumpulkan di proyek-proyek open-source selama tahun ini. Ini akan memakan waktu cukup lama, tentu saja, jadi kami menyiapkan artikel ini untuk memudahkan Anda. Hari ini kita akan mengingat titik-titik gelap paling menarik yang kita temui dalam proyek-proyek C / C ++ open-source pada tahun 2019.

Tidak. 10. Sistem operasi apa yang kita jalankan?


V1040 Kemungkinan salah ketik dalam ejaan nama makro yang telah ditentukan sebelumnya. Makro '__MINGW32_' mirip dengan '__MINGW32__'. winapi.h 4112

#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif 

Ada kesalahan ketik atas nama __MINGW32 _ macro (MINGW32 sebenarnya dideklarasikan oleh __MINGW32__). Di tempat lain dalam proyek ini, cek ditulis dengan benar:

Gambar 3


Ngomong-ngomong, bug ini bukan hanya yang pertama kali dijelaskan dalam artikel " CMake: Kasus ketika Kualitas Proyek Tidak Terampuni " tetapi bug asli pertama yang ditemukan oleh diagnostik V1040 dalam proyek sumber terbuka nyata (19 Agustus , 2019).

Tidak. 9. Siapa yang pertama?


V502 Mungkin operator '?:' Bekerja dengan cara yang berbeda dari yang diharapkan. Operator '?:' Memiliki prioritas lebih rendah daripada operator '=='. mir_parser.cpp 884

 enum Opcode : uint8 { kOpUndef, .... OP_intrinsiccall, OP_intrinsiccallassigned, .... kOpLast, }; bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) { Opcode o = !isAssigned ? (....) : (....); auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....); lexer.NextToken(); if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind())); } else { intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....)); } .... } 

Kami tertarik pada bagian berikut:

 if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

Diutamakan operator '==' lebih tinggi daripada operator ternary (? :). Oleh karena itu, ekspresi kondisional dievaluasi dalam urutan yang salah dan setara dengan kode berikut:

 if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

Karena konstanta OP_intrinsiccall dan OP_intrinsiccallassigned non-null, kondisi akan kembali benar setiap saat, yang berarti tubuh cabang lain adalah kode yang tidak dapat dijangkau.

Bug ini dideskripsikan dalam artikel " Memeriksa Kompresor Tabut Baru-Baru Ini Dibuat Open-Source oleh Huawei ."

Tidak. 8. Operasi bitwise berbahaya


V1046 Penggunaan bool 'dan' int 'yang tidak aman secara bersamaan dalam operasi' & = '. GSLMultiRootFinder.h 175

 int AddFunction(const ROOT::Math::IMultiGenFunction & func) { ROOT::Math::IMultiGenFunction * f = func.Clone(); if (!f) return 0; fFunctions.push_back(f); return fFunctions.size(); } template<class FuncIterator> bool SetFunctionList( FuncIterator begin, FuncIterator end) { bool ret = true; for (FuncIterator itr = begin; itr != end; ++itr) { const ROOT::Math::IMultiGenFunction * f = *itr; ret &= AddFunction(*f); } return ret; } 

Kode menunjukkan bahwa fungsi SetFunctionList melintasi daftar iterator. Jika setidaknya satu iterator tidak valid, fungsi mengembalikan false , atau true sebaliknya.

Namun, fungsi SetFunctionList dapat mengembalikan false bahkan untuk iterator yang valid. Mari kita mencari tahu alasannya. Fungsi AddFunction mengembalikan jumlah iterator yang valid pada daftar fungsi . Artinya, menambahkan iterator non-nol akan menyebabkan daftar semakin bertambah ukurannya: 1, 2, 3, 4, dan seterusnya. Di sinilah bug berperan:

 ret &= AddFunction(*f); 

Karena fungsi mengembalikan nilai tipe int daripada bool , operasi '& =' akan mengembalikan false untuk nilai genap karena bit paling tidak signifikan dari angka genap selalu disetel ke nol. Ini adalah bagaimana satu bug halus dapat mematahkan nilai balik SetFunctionsList bahkan ketika argumennya valid.

Jika Anda membaca cuplikan dengan hati-hati (dan ternyata benar, bukan?), Anda dapat memperhatikan bahwa itu berasal dari ROOT proyek. Ya, kami memeriksanya juga: " Menganalisis Kode ROOT, Kerangka Analisis Data Ilmiah ."

Tidak. 7. Variabel tercampur


V1001 [CWE-563] Variabel 'Mode' ditugaskan tetapi tidak digunakan pada akhir fungsi. SIModeRegister.cpp 48

 struct Status { unsigned Mask; unsigned Mode; Status() : Mask(0), Mode(0){}; Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { Mode &= Mask; }; .... }; 

Sangat berbahaya menggunakan nama yang sama untuk argumen fungsi seperti untuk anggota kelas karena Anda berisiko mencampurnya. Dan itulah yang terjadi di sini. Ungkapan berikut tidak masuk akal:

 Mode &= Mask; 

Argumen fungsi berubah, dan hanya itu. Argumen ini tidak digunakan dengan cara apa pun setelah itu. Apa yang benar-benar ingin ditulis oleh programmer adalah sebagai berikut:

 Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; }; 

Bug ini ditemukan di LLVM . Kami memiliki tradisi untuk memeriksa proyek ini sesekali. Tahun ini kami memeriksanya sekali lagi.

Tidak. 6. C ++ memiliki hukumnya sendiri


Bug ini berasal dari kenyataan bahwa aturan C ++ tidak selalu mengikuti aturan matematika atau "akal sehat". Lihatlah potongan kecil di bawah ini dan coba temukan sendiri bug tersebut.

V709 Perbandingan mencurigakan ditemukan: 'f0 == f1 == m_fractureBodies.size ()'. Ingat bahwa 'a == b == c' tidak sama dengan 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

 btAlignedObjectArray<btFractureBody*> m_fractureBodies; void btFractureDynamicsWorld::fractureCallback() { for (int i = 0; i < numManifolds; i++) { .... int f0 = m_fractureBodies.findLinearSearch(....); int f1 = m_fractureBodies.findLinearSearch(....); if (f0 == f1 == m_fractureBodies.size()) continue; .... } .... } 

Kondisi ini tampaknya memeriksa bahwa f0 sama dengan f1 dan sama dengan jumlah elemen di m_fractureBodies . Itu mungkin dimaksudkan untuk memeriksa apakah f0 dan f1 terletak di akhir array m_fractureBodies karena mengandung posisi objek yang ditemukan oleh metode findLinearSearch () . Tetapi pada kenyataannya, ekspresi kondisional ini memeriksa apakah f0 sama dengan f1 dan kemudian jika m_fractureBodies.size () sama dengan hasil dari ekspresi f0 == f1 . Artinya, operan ketiga di sini diperiksa terhadap 0 atau 1.

Itu bug yang bagus! Dan, untungnya, yang cukup langka. Sejauh ini kita telah melihatnya hanya dalam tiga proyek open-source, dan, yang menarik, ketiganya adalah mesin game. Ini bukan satu-satunya bug yang ditemukan di Bullet; yang paling menarik dijelaskan dalam artikel " PVS-Studio Melihat ke dalam Mesin Peluru Red Dead Redemption's ".

Tidak. 5. Apa yang ada di akhir baris?


Ini mudah jika Anda tahu satu detail rumit.

V739 EOF tidak boleh dibandingkan dengan nilai tipe 'char'. 'Ch' harus dari tipe 'int'. json.cpp 762

 void JsonIn::skip_separator() { signed char ch; .... if (ch == ',') { if( ate_separator ) { .... } .... } else if (ch == EOF) { .... } 

Ini adalah salah satu bug yang tidak mudah Anda temui jika Anda tidak tahu bahwa EOF didefinisikan sebagai -1. Jadi, jika Anda mencoba membandingkannya dengan variabel tipe char yang ditandatangani , kondisinya akan hampir selalu salah . Satu-satunya pengecualian adalah karakter yang dikodekan sebagai 0xFF (255). Jika dibandingkan dengan EOF , karakter ini akan berubah menjadi -1, sehingga membuat kondisi ini benar.

Banyak bug di Top 10 tahun ini ditemukan dalam perangkat lunak game komputer: mesin atau game open-source. Seperti yang sudah Anda tebak, yang ini juga berasal dari daerah itu. Lebih banyak kesalahan dijelaskan dalam artikel " Hari-Hari Kegelapan Bencana: Analisis Statis dan Permainan Roguelike ".

Tidak. 4. Konstanta ajaib Pi


V624 Mungkin ada kesalahan cetak dalam konstanta '3.141592538'. Pertimbangkan untuk menggunakan konstanta M_PI dari <math.h>. PhysicsClientC_API.cpp 4109

 B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... } 


Ada kesalahan ketik kecil pada nomor Pi (3,141592653 ...): angka "6" tidak ada di tempat desimal ke-7.

Gambar 4
Digit desimal sepersejuta tidak akan menyebabkan kerusakan yang nyata, tetapi masih lebih baik untuk menggunakan konstanta yang ada dari perpustakaan, yang kebenarannya dijamin. Nomor Pi, misalnya, diwakili oleh M_PI konstan dari header math.h.

Anda sudah membaca tentang bug ini di artikel " PVS-Studio Melihat ke dalam Mesin Peluru Red Dead Redemption ", di mana ia ditempatkan keenam. Jika Anda belum membacanya, ini adalah kesempatan terakhir Anda.

Pengalihan kecil


Kami mendekati 3 bug paling menarik teratas. Seperti yang mungkin telah Anda perhatikan, saya menyortir bug bukan berdasarkan dampaknya, tetapi dengan upaya yang dilakukan pemeriksa manusia untuk menemukannya. Bagaimanapun, keuntungan dari analisis statis atas ulasan kode pada dasarnya adalah ketidakmampuan alat perangkat lunak untuk lelah atau melupakan hal-hal. :)

Sekarang, mari kita lihat apa yang kita miliki di Top 3 kita.

Gambar 8


Tidak. 3. Pengecualian yang sulit dipahami


Kelas V702 harus selalu diturunkan dari std :: exception (dan yang sama) sebagai 'publik' (tidak ada kata kunci yang ditentukan, jadi kompiler secara default ke 'private'). CalcManager CalcException.h 4

 class CalcException : std::exception { public: CalcException(HRESULT hr) { m_hr = hr; } HRESULT GetException() { return m_hr; } private: HRESULT m_hr; }; 

Penganalisis telah mendeteksi kelas yang berasal dari kelas std :: exception menggunakan pengubah pribadi (yang digunakan secara default jika tidak ditentukan sebaliknya). Masalah dengan kode ini adalah bahwa upaya untuk menangkap std :: exception generik akan menyebabkan program kehilangan pengecualian dari tipe CalcException . Perilaku ini berasal dari kenyataan bahwa warisan pribadi melarang konversi tipe implisit.

Anda pasti tidak ingin melihat program Anda macet karena pengubah publik yang terlewat. Omong-omong, saya yakin Anda telah menggunakan aplikasi ini setidaknya sekali dalam hidup Anda karena ini adalah Kalkulator Windows lama yang baik, yang kami juga periksa awal tahun ini.

Tidak. 2. Tag HTML tidak tertutup


V735 Mungkin HTML yang salah. Tag penutup "</body>" ditemukan, sedangkan tag "</div>" diharapkan. book.cpp 127

 static QString makeAlgebraLogBaseConversionPage() { return BEGIN INDEX_LINK TITLE(Book::tr("Logarithmic Base Conversion")) FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a)) END; } 

Seperti yang sering terjadi, kode sumber C / C ++ tidak berbicara banyak dengan sendirinya, jadi mari kita lihat kode preprocessed yang dihasilkan dari snippet di atas:

Gambar 6


Penganalisis telah menemukan tag <div> yang tidak tertutup. Ada banyak fragmen kode html di sini, jadi penulis perlu merevisinya.

Terkejut kita dapat mendiagnosis bug semacam ini? Saya juga terkesan ketika saya melihat itu untuk pertama kalinya. Jadi, ya, kami tahu sesuatu tentang menganalisis kode html. Yah, hanya jika itu dalam kode C ++. :)

Tidak hanya bug ini ditempatkan di urutan kedua, tetapi ini adalah kalkulator kedua dalam daftar Top 10 kami. Untuk mempelajari bug lain apa yang kami temukan dalam proyek ini, lihat artikel " Mengikuti Jejak Kalkulator: SpeedCrunch ".

Tidak. 1. Fungsi standar yang sulit dipahami


Inilah bug yang ditempatkan terlebih dahulu. Yang ini adalah bug yang sangat aneh, yang berhasil membuatnya melalui ulasan kode.

Cobalah untuk menemukannya sendiri:

 static int EatWhitespace (FILE * InFile) /* ----------------------------------------------------------------------- ** * Scan past whitespace (see ctype(3C)) and return the first non-whitespace * character, or newline, or EOF. * * Input: InFile - Input source. * * Output: The next non-whitespace character in the input stream. * * Notes: Because the config files use a line-oriented grammar, we * explicitly exclude the newline character from the list of * whitespace characters. * - Note that both EOF (-1) and the nul character ('\0') are * considered end-of-file markers. * * ----------------------------------------------------------------------- ** */ { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); } /* EatWhitespace */ 

Sekarang mari kita lihat apa yang dikatakan oleh penganalisa:

V560 Bagian dari ekspresi kondisional selalu benar: ('\ n'! = C). params.c 136.

Aneh, bukan? Mari kita lihat beberapa tempat aneh lainnya tetapi dalam file yang berbeda (charset.h):

 #ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t') 

Hm, ini memang aneh ... Jadi, jika variabel c sama dengan '\ n', maka fungsi yang tampaknya tidak berbahaya adalah spasi (c) akan kembali salah , sehingga mencegah bagian kedua dari cek dari mengeksekusi karena evaluasi hubung singkat. Dan jika isspace (c) dijalankan, variabel c akan sama dengan '' atau '\ t', yang jelas tidak sama dengan '\ n' .

Anda bisa berargumen bahwa makro ini mirip dengan #define true false dan kode seperti itu tidak akan pernah berhasil melalui tinjauan kode. Tetapi potongan khusus ini melakukannya - dan sedang duduk di repositori menunggu untuk ditemukan.

Untuk komentar lebih rinci tentang bug ini, lihat artikel " Ingin Bermain Detektif? Temukan Bug dalam Fungsi dari Komandan Midnight ".

Kesimpulan


Gambar 9


Kami telah menemukan banyak bug selama tahun ini. Itu adalah kesalahan umum salin-tempel, konstanta yang tidak akurat, tag tidak tertutup, dan banyak cacat lainnya. Tetapi penganalisa kami terus berkembang dan belajar untuk mendiagnosis lebih banyak jenis masalah, jadi kami tentu tidak akan melambat dan akan menerbitkan artikel baru tentang bug yang ditemukan dalam proyek sama rutinnya seperti sebelumnya.

Jika Anda belum pernah membaca artikel kami sebelumnya, semua bug ini ditemukan menggunakan penganalisa statis PVS-Studio kami, yang dapat Anda unduh dan coba pada proyek Anda sendiri. Ini mendeteksi bug dalam program yang ditulis dalam C, C ++, C #, dan Java.

Anda akhirnya sampai ke garis finish! Jika Anda melewatkan dua level pertama, saya sarankan Anda menggunakan kesempatan ini dan selesaikan level ini bersama kami: C # dan Java .

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


All Articles