Geekly articles weekly

CMake: kasus ketika proyek tidak termaafkan kualitas kodenya

Gambar 1

CMake adalah sistem otomatisasi lintas-platform untuk membangun proyek. Sistem ini jauh lebih tua daripada penganalisis kode statis PVS-Studio, sementara belum ada yang mencoba menerapkannya pada kode dan meninjau kesalahan. Ternyata ada banyak kesalahan. Audiensi CMake sangat besar. Di atasnya, proyek baru dimulai dan yang lama ditransfer. Menakutkan membayangkan berapa banyak programmer yang mengalami kesalahan ini atau itu.

Pendahuluan


CMake (dari bahasa Inggris cross-platform make) adalah sistem otomasi lintas-platform untuk membangun perangkat lunak dari kode sumber. CMake tidak langsung membangun, tetapi hanya menghasilkan file kontrol bangun dari file CMakeLists.txt. Rilis pertama dari program ini berlangsung pada tahun 2000. Sebagai perbandingan, analisa statis PVS-Studio hanya muncul pada tahun 2008. Kemudian itu difokuskan pada menemukan kesalahan ketika porting program dari sistem 32-bit ke yang 64-bit, dan pada tahun 2010 set pertama diagnostik tujuan umum ( V501 - V545 ) muncul. Omong-omong, ada beberapa peringatan dari set pertama ini pada kode CMake.

Kesalahan yang Tidak Dimaafkan


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

/* from winternl.h */ #if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif

Diagnostik V1040 baru saja diimplementasikan. Pada saat publikasi artikel, kemungkinan besar, tidak akan ada rilis dengannya, tetapi dengan bantuan diagnostik ini kami telah berhasil menemukan kesalahan yang sulit.

Di sini mereka membuat salah ketik nama __MINGW32_ . Pada akhirnya, satu garis bawah hilang. Jika Anda mencari berdasarkan kode dengan nama ini, Anda dapat memastikan bahwa proyek tersebut benar-benar menggunakan versi dengan dua garis bawah di kedua sisi:

Gambar 3


V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 558

 bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile, const std::string& regKeyBase, std::string& nextAvailableSubKeyName) { .... if (ERROR_SUCCESS == result) { wchar_t subkeyname[256]; // <= DWORD cch_subkeyname = sizeof(subkeyname) * sizeof(subkeyname[0]); // <= wchar_t keyclass[256]; DWORD cch_keyclass = sizeof(keyclass) * sizeof(keyclass[0]); FILETIME lastWriteTime; lastWriteTime.dwHighDateTime = 0; lastWriteTime.dwLowDateTime = 0; while (ERROR_SUCCESS == RegEnumKeyExW(hkey, index, subkeyname, &cch_subkeyname, 0, keyclass, &cch_keyclass, &lastWriteTime)) { .... } .... }

Ketika array dinyatakan secara statis, sizeof operator akan menghitung ukurannya dalam byte, dengan mempertimbangkan jumlah elemen dan ukuran elemen. Saat menghitung nilai variabel cch_subkeyname, programmer tidak memperhitungkan ini dan menerima nilai 4 kali lebih besar dari yang direncanakan. Mari kita jelaskan di mana "4 kali."

Array dan ukurannya yang salah diteruskan ke fungsi RegEnumKeyExW :

 LSTATUS RegEnumKeyExW( HKEY hKey, DWORD dwIndex, LPWSTR lpName, // <= subkeyname LPDWORD lpcchName, // <= cch_subkeyname LPDWORD lpReserved, LPWSTR lpClass, LPDWORD lpcchClass, PFILETIME lpftLastWriteTime );

Pointer lpcchName harus mengarah ke variabel yang berisi ukuran buffer yang ditentukan dalam karakter: "Pointer ke variabel yang menentukan ukuran buffer yang ditentukan oleh parameter lpClass , dalam karakter". Ukuran array subkeyname adalah 512 byte dan mampu menyimpan 256 karakter tipe wchar_t (pada Windows wchar_t adalah 2 byte). Nilai ini 256 dan harus diteruskan ke fungsi. Sebaliknya, 512 dikalikan dengan 2 lagi untuk mendapatkan 1024.

Saya pikir cara memperbaiki kesalahan sekarang jelas. Alih-alih multiplikasi, gunakan pembagian:

 DWORD cch_subkeyname = sizeof(subkeyname) / sizeof(subkeyname[0]);

Omong-omong, kesalahan yang persis sama terjadi ketika menghitung nilai variabel cch_keyclass .

Kesalahan yang dijelaskan berpotensi menyebabkan buffer overflow. Perlu untuk memperbaiki semua tempat seperti itu:

  • V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 556
  • V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 572
  • V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 621
  • V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 622
  • V531 Aneh bahwa operator sizeof () dikalikan dengan sizeof (). cmGlobalVisualStudioGenerator.cxx 649

V595 Pointer 'this-> BuildFileStream' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 133, 134. cmMakefileTargetGenerator.cxx 133

 void cmMakefileTargetGenerator::CreateRuleFile() { .... this->BuildFileStream->SetCopyIfDifferent(true); if (!this->BuildFileStream) { return; } .... }

Pointer ini-> BuildFileStream dereferenced tepat sebelum pemeriksaan validasi. Apakah ini benar-benar menyebabkan masalah? Di bawah ini adalah contoh lain dari tempat seperti itu. Itu dibuat tepat di bawah kertas karbon. Tetapi pada kenyataannya, ada banyak peringatan V595 dan kebanyakan dari mereka tidak begitu jelas. Dari pengalaman, saya dapat mengatakan bahwa mengoreksi peringatan dari diagnosis ini adalah yang terpanjang.

  • V595 Pointer 'this-> FlagFileStream' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 303, 304. cmMakefileTargetGenerator.cxx 303

V614 Uninitialized pointer 'str' digunakan. cmVSSetupHelper.h 80

 class SmartBSTR { public: SmartBSTR() { str = NULL; } SmartBSTR(const SmartBSTR& src) { if (src.str != NULL) { str = ::SysAllocStringByteLen((char*)str, ::SysStringByteLen(str)); } else { str = ::SysAllocStringByteLen(NULL, 0); } } .... private: BSTR str; };

Analyzer mendeteksi penggunaan str pointer yang tidak diinisialisasi. Dan ini muncul karena kesalahan ketik yang biasa. Saat memanggil fungsi SysAllocStringByteLen, Anda harus menggunakan pointer src.str .

V557 Array overrun dimungkinkan. Nilai indeks 'lensymbol' bisa mencapai 28. archive_read_support_format_rar.c 2749

 static int64_t expand(struct archive_read *a, int64_t end) { .... if ((lensymbol = read_next_symbol(a, &rar->lengthcode)) < 0) goto bad_data; if (lensymbol > (int)(sizeof(lengthbases)/sizeof(lengthbases[0]))) goto bad_data; if (lensymbol > (int)(sizeof(lengthbits)/sizeof(lengthbits[0]))) goto bad_data; len = lengthbases[lensymbol] + 2; if (lengthbits[lensymbol] > 0) { if (!rar_br_read_ahead(a, br, lengthbits[lensymbol])) goto truncated_data; len += rar_br_bits(br, lengthbits[lensymbol]); rar_br_consume(br, lengthbits[lensymbol]); } .... }

Beberapa masalah ditemukan dalam potongan kode ini. Ketika mengakses array dari basis panjang dan panjang, dimungkinkan untuk melampaui batas array, karena di atas kode, pengembang menulis operator '>' bukan '> ='. Cek semacam itu mulai melewati satu nilai yang tidak valid. Kita dihadapkan dengan pola kesalahan klasik yang disebut Kesalahan Satu-per-satu .

Seluruh daftar tempat untuk mengakses array menurut indeks tidak valid:

  • V557 Array overrun dimungkinkan. Nilai indeks 'lensymbol' bisa mencapai 28. archive_read_support_format_rar.c 2750
  • V557 Array overrun dimungkinkan. Nilai indeks 'lensymbol' bisa mencapai 28. archive_read_support_format_rar.c 2751
  • V557 Array overrun dimungkinkan. Nilai indeks 'lensymbol' bisa mencapai 28. archive_read_support_format_rar.c 2753
  • V557 Array overrun dimungkinkan. Nilai indeks 'lensymbol' bisa mencapai 28. archive_read_support_format_rar.c 2754
  • V557 Array overrun dimungkinkan. Nilai indeks 'offssymbol' bisa mencapai 60. archive_read_support_format_rar.c 2797

Kebocoran memori


V773 Fungsi itu keluar tanpa melepaskan pointer 'testRun'. Kebocoran memori dimungkinkan. cmCTestMultiProcessHandler.cxx 193

 void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner, bool started) { .... delete runner; if (started) { this->StartNextTests(); } } bool cmCTestMultiProcessHandler::StartTestProcess(int test) { .... cmCTestRunTest* testRun = new cmCTestRunTest(*this); // <= .... if (testRun->StartTest(this->Completed, this->Total)) { return true; // <= } this->FinishTestProcess(testRun, false); // <= return false; }

Analyzer mendeteksi kebocoran memori. Memori oleh pointer testRun tidak dibebaskan jika fungsi testRun-> StartTest mengembalikan true . Ketika cabang kode lain dieksekusi, memori dengan pointer testRun dibebaskan dalam fungsi ini-> FinishTestProcess .

Kebocoran sumber daya


V773 Fungsi ini keluar tanpa menutup file yang direferensikan oleh pegangan 'fd'. Kebocoran sumber daya mungkin terjadi. rhash.c 450

 RHASH_API int rhash_file(....) { FILE* fd; rhash ctx; int res; hash_id &= RHASH_ALL_HASHES; if (hash_id == 0) { errno = EINVAL; return -1; } if ((fd = fopen(filepath, "rb")) == NULL) return -1; if ((ctx = rhash_init(hash_id)) == NULL) return -1; // <= fclose(fd); ??? res = rhash_file_update(ctx, fd); fclose(fd); rhash_final(ctx, result); rhash_free(ctx); return res; }

Logika aneh dalam kondisi


V590 Pertimbangkan untuk memeriksa ekspresi '* s! =' \ 0 '&& * s ==' ''. Ekspresi berlebihan atau mengandung kesalahan cetak. archive_cmdline.c 76

 static ssize_t get_argument(struct archive_string *as, const char *p) { const char *s = p; archive_string_empty(as); /* Skip beginning space characters. */ while (*s != '\0' && *s == ' ') s++; .... }

Membandingkan karakter * s dengan terminal nol tidak perlu. Kondisi loop sementara hanya bergantung pada apakah karakter sama dengan spasi atau tidak. Ini bukan kesalahan, tetapi komplikasi tambahan dari kode.

V592 Ekspresi tertutup oleh tanda kurung dua kali: ((ekspresi)). Sepasang tanda kurung tidak perlu atau salah cetak ada. cmCTestTestHandler.cxx 899

 void cmCTestTestHandler::ComputeTestListForRerunFailed() { this->ExpandTestsToRunInformationForRerunFailed(); ListOfTests finalList; int cnt = 0; for (cmCTestTestProperties& tp : this->TestList) { cnt++; // if this test is not in our list of tests to run, then skip it. if ((!this->TestsToRun.empty() && std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end())) { continue; } tp.Index = cnt; finalList.push_back(tp); } .... }

Penganalisa memperingatkan bahwa mungkin negasi harus dikeluarkan dari kurung. Tampaknya tidak ada kesalahan seperti itu di sini - hanya kurung ganda tambahan. Tetapi, kemungkinan besar, ada kesalahan logis dalam kondisi ini.

Pernyataan melanjutkan dieksekusi jika daftar tes ini-> TestsToRun tidak kosong dan cnt tidak ada di dalamnya. Adalah logis untuk mengasumsikan bahwa jika daftar tes kosong, maka tindakan yang sama harus dilakukan. Kemungkinan besar, kondisinya harus seperti ini:

 if (this->TestsToRun.empty() || std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end()) { continue; }

V592 Ekspresi tertutup oleh tanda kurung dua kali: ((ekspresi)). Sepasang tanda kurung tidak perlu atau salah cetak ada. cmMessageCommand.cxx 73

 bool cmMessageCommand::InitialPass(std::vector<std::string> const& args, cmExecutionStatus&) { .... } else if (*i == "DEPRECATION") { if (this->Makefile->IsOn("CMAKE_ERROR_DEPRECATED")) { fatal = true; type = MessageType::DEPRECATION_ERROR; level = cmake::LogLevel::LOG_ERROR; } else if ((!this->Makefile->IsSet("CMAKE_WARN_DEPRECATED") || this->Makefile->IsOn("CMAKE_WARN_DEPRECATED"))) { type = MessageType::DEPRECATION_WARNING; level = cmake::LogLevel::LOG_WARNING; } else { return true; } ++i; } .... }

Contoh serupa, tapi di sini saya lebih percaya diri dengan adanya kesalahan. Fungsi IsSet ("CMAKE_WARN_DEPRECATED") memverifikasi bahwa nilai CMAKE_WARN_DEPRECATED diatur secara global, dan fungsi IsOn ("CMAKE_WARN_DEPRECATED") memverifikasi bahwa nilai tersebut ditentukan dalam konfigurasi proyek. Kemungkinan besar, operator negasi berlebihan, karena dalam kedua kasus, sudah benar untuk menetapkan jenis dan nilai level yang sama.

V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. cmCTestRunTest.cxx 151

 bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started) { .... } else if ((success && !this->TestProperties->WillFail) || (!success && this->TestProperties->WillFail)) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; } .... }

Kode tersebut dapat sangat disederhanakan dengan menulis ulang ekspresi kondisional dengan cara ini:

 } else if (success != this->TestProperties->WillFail) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; }

Beberapa tempat lain yang dapat Anda sederhanakan:

  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A && B) || Ekspresi (! A &&! B) 'sama dengan ekspresi' bool (A) == bool (B) '. cmCTestTestHandler.cxx 702
  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. digest_sspi.c 443
  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. tcp.c 1295
  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. testDynamicLoader.cxx 58
  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. testDynamicLoader.cxx 65
  • V728 Pemeriksaan berlebihan dapat disederhanakan. The '(A &&! B) || Ekspresi (! A && B) 'sama dengan ekspresi' bool (A)! = Bool (B) '. testDynamicLoader.cxx 72

Peringatan lain-lain


V523 Pernyataan 'maka' setara dengan fragmen kode berikutnya. archive_read_support_format_ar.c 415

 static int _ar_read_header(struct archive_read *a, struct archive_entry *entry, struct ar *ar, const char *h, size_t *unconsumed) { .... /* * "__.SYMDEF" is a BSD archive symbol table. */ if (strcmp(filename, "__.SYMDEF") == 0) { archive_entry_copy_pathname(entry, filename); /* Parse the time, owner, mode, size fields. */ return (ar_parse_common_header(ar, entry, h)); } /* * Otherwise, this is a standard entry. The filename * has already been trimmed as much as possible, based * on our current knowledge of the format. */ archive_entry_copy_pathname(entry, filename); return (ar_parse_common_header(ar, entry, h)); }

Ekspresi dalam kondisi terakhir identik dengan dua baris terakhir dari fungsi. Kode ini dapat disederhanakan dengan menghapus kondisi, atau ada kesalahan dalam kode dan harus diperbaiki.

V535 Variabel 'i' digunakan untuk loop ini dan untuk loop luar. Periksa baris: 2220, 2241. multi.c 2241

 static CURLMcode singlesocket(struct Curl_multi *multi, struct Curl_easy *data) { .... for(i = 0; (i< MAX_SOCKSPEREASYHANDLE) && // <= (curraction & (GETSOCK_READSOCK(i) | GETSOCK_WRITESOCK(i))); i++) { unsigned int action = CURL_POLL_NONE; unsigned int prevaction = 0; unsigned int comboaction; bool sincebefore = FALSE; s = socks[i]; /* get it from the hash */ entry = sh_getentry(&multi->sockhash, s); if(curraction & GETSOCK_READSOCK(i)) action |= CURL_POLL_IN; if(curraction & GETSOCK_WRITESOCK(i)) action |= CURL_POLL_OUT; actions[i] = action; if(entry) { /* check if new for this transfer */ for(i = 0; i< data->numsocks; i++) { // <= if(s == data->sockets[i]) { prevaction = data->actions[i]; sincebefore = TRUE; break; } } } .... }

Variabel i digunakan sebagai penghitung lingkaran di loop luar dan bersarang. Dalam hal ini, nilai penghitung kembali mulai menghitung dari nol pada nilai terlampir. Ini mungkin bukan kesalahan di sini, tetapi kode ini mencurigakan.

V519 Variabel 'tagString' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 84, 86. cmCPackLog.cxx 86

 oid cmCPackLog::Log(int tag, const char* file, int line, const char* msg, size_t length) { .... if (tag & LOG_OUTPUT) { output = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "VERBOSE"; } } if (tag & LOG_WARNING) { warning = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "WARNING"; } } .... }

Variabel tagString terkoyak oleh nilai baru di semua tempat. Sulit untuk mengatakan apa kesalahannya, atau mengapa mereka melakukannya. Mungkin operator '=' dan '+ =' bingung.

Seluruh daftar tempat-tempat tersebut:

  • V519 Variabel 'tagString' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 94, 96. cmCPackLog.cxx 96
  • V519 Variabel 'tagString' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 104, 106. cmCPackLog.cxx 106
  • V519 Variabel 'tagString' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 114, 116. cmCPackLog.cxx 116
  • V519 Variabel 'tagString' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 125, 127. cmCPackLog.cxx 127

V519 Variabel 'aes-> aes_set' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 4052, 4054. archive_string.c 4054

 int archive_mstring_copy_utf8(struct archive_mstring *aes, const char *utf8) { if (utf8 == NULL) { aes->aes_set = 0; // <= } aes->aes_set = AES_SET_UTF8; // <= .... return (int)strlen(utf8); }

Memaksa AES_SET_UTF8 terlihat mencurigakan. Saya pikir kode seperti itu akan menyesatkan pengembang yang dihadapkan pada penyempurnaan tempat ini.

Kode ini disalin ke satu tempat lagi:

  • V519 Variabel 'aes-> aes_set' diberi nilai dua kali berturut-turut. Mungkin ini sebuah kesalahan. Periksa baris: 4066, 4068. archive_string.c 4068

Cara menemukan kesalahan dalam proyek di CMake


Di bagian ini saya akan memberi tahu Anda sedikit cara memeriksa proyek-proyek di CMake menggunakan PVS-Studio dengan mudah dan mudah.

Windows / Visual Studio

Untuk Visual Studio, Anda dapat membuat file proyek menggunakan CMake GUI atau perintah berikut:

 cmake -G "Visual Studio 15 2017 Win64" ..

Selanjutnya, Anda bisa membuka file .sln dan menguji proyek menggunakan plug-in untuk Visual Studio.

Linux / macOS

Pada sistem ini, file compile_commands.json digunakan untuk memverifikasi proyek. Omong-omong, itu dapat dihasilkan dalam sistem perakitan yang berbeda. Di CMake, ini dilakukan seperti ini:

 cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ..

Tetap memulai penganalisis dalam direktori dengan file .json:

 pvs-studio-analyzer analyze -l /path/to/PVS-Studio.lic -o /path/to/project.log -e /path/to/exclude-path -j<N>

Kami juga mengembangkan modul untuk proyek-proyek CMake. Beberapa orang suka menggunakannya. Modul CMake dan contoh penggunaannya dapat ditemukan di repositori kami di GitHub: pvs-studio-cmake-example .

Kesimpulan


Penonton besar pengguna CMake adalah penguji proyek yang baik, tetapi banyak masalah tidak dapat dicegah sebelum rilis, menggunakan alat analisis kode statis seperti PVS-Studio .

Jika Anda menyukai hasil analisis, tetapi proyek Anda tidak ditulis dalam C dan C ++, saya ingin mengingatkan Anda bahwa analisa juga mendukung analisis proyek di C # dan Java. Anda dapat mencoba penganalisa pada proyek Anda dengan membuka halaman ini .



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Svyatoslav Razmyslov. CMake: Kasus ketika Kualitas Proyek tidak dapat dimaafkan .

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

More articles:


All Articles