Ini adalah artikel klasik tentang bagaimana tim kami menguji proyek LibrePCB terbuka menggunakan penganalisis kode statis PVS-Studio. Namun, artikel ini menarik karena verifikasi dilakukan di dalam wadah Docker. Jika Anda menggunakan wadah, kami berharap artikel tersebut akan menunjukkan cara sederhana lain untuk mengintegrasikan alat analisis ke dalam proses pengembangan.
LibrePCB
LibrePCB adalah perangkat lunak gratis untuk mendesain sirkuit elektronik dan papan sirkuit cetak. Kode program ditulis dalam C ++, dan Qt5 digunakan untuk membangun antarmuka grafis. Baru-baru ini, rilis resmi pertama dari aplikasi ini terjadi, yang menandai stabilisasi format file sendiri (* .lp, * .lplib). Paket biner disiapkan untuk Linux, macOS dan Windows.
LibrePCB adalah proyek kecil yang hanya berisi sekitar 300.000 baris kode tidak kosong dalam C dan C ++. Pada saat yang sama, 25% dari baris yang tidak kosong adalah komentar. Omong-omong, ini adalah persentase yang besar untuk komentar. Kemungkinan besar, ini disebabkan oleh fakta bahwa proyek memiliki banyak file kecil, yang sebagian besar ditempati oleh komentar komentar dan informasi proyek. Kode situs GitHub:
LibrePCB .
Proyek itu tampak menarik bagi kami, dan kami memutuskan untuk memeriksanya. Tetapi hasil tes tidak lagi begitu menarik. Ya, ada kesalahan nyata. Tetapi tidak ada yang istimewa tentang hal itu yang tentunya harus kami sampaikan kepada pembaca artikel kami. Mungkin kami tidak akan menulis artikel dan membatasi diri untuk mengirim kesalahan kepada pengembang proyek. Namun, hal yang menarik adalah bahwa proyek tersebut diuji di dalam gambar Docker, dan oleh karena itu kami memutuskan bahwa masih layak untuk menulis artikel.
Docker
Docker - perangkat lunak untuk mengotomatiskan penyebaran dan pengelolaan aplikasi dalam lingkungan virtualisasi di tingkat sistem operasi. Ini memungkinkan Anda untuk "mengemas" aplikasi dengan semua lingkungan dan dependensinya ke dalam wadah. Meskipun teknologi ini berumur sekitar lima tahun dan banyak perusahaan telah lama mengimplementasikan Docker dalam infrastruktur proyek mereka, di dunia open source ini tidak terlalu terlihat sampai saat ini.
Perusahaan kami bekerja sangat erat dengan proyek sumber terbuka, memeriksa kode sumber menggunakan penganalisa statis PVS-Studio kami sendiri. Saat ini,
lebih dari 300 proyek telah diverifikasi. Hal yang paling sulit dalam kegiatan ini adalah selalu kompilasi proyek orang lain, tetapi pengenalan wadah Docker sangat menyederhanakan proses ini.
Pengalaman pertama menguji proyek sumber terbuka di Docker adalah dengan
Azure Service Fabric . Di sana, pengembang membuat instalasi direktori file sumber ke wadah dan integrasi penganalisa terbatas untuk mengedit salah satu skrip yang berjalan dalam wadah:
diff --git a/src/build.sh b/src/build.sh index 290c57d..2a286dc 100755 --- a/src/build.sh +++ b/src/build.sh @@ -193,6 +193,9 @@ BuildDir() cd ${ProjBinRoot}/build.${DirName} + pvs-studio-analyzer analyze --cfg /src/PVS-Studio.cfg \ + -o ./service-fabric-pvs.log -j4 + if [ "false" = ${SkipBuild} ]; then if (( $NumProc <= 0 )); then NumProc=$(($(getconf _NPROCESSORS_ONLN)+0))
Perbedaan antara proyek LibrePCB adalah bahwa mereka segera menyediakan
Dockerfile untuk membangun gambar dan proyek di dalamnya. Ternyata menjadi lebih nyaman bagi kami. Inilah bagian dari file Docker yang kami minati:
FROM ubuntu:14.04 # install packages RUN DEBIAN_FRONTEND=noninteractive \ apt-get -q update \ && apt-get -qy upgrade \ && apt-get -qy install git g++ qt5-default qttools5-dev-tools qt5-doc \ qtcreator libglu1-mesa-dev dia \ && apt-get clean # checkout librepcb RUN git clone --recursive https:
Kami tidak akan melakukan kompilasi dan instalasi proyek ketika merakit gambar. Dengan demikian, kami mengumpulkan gambar di mana penulis proyek menjamin keberhasilan perakitan proyek.
Setelah memulai wadah, penganalisis diinstal dan perintah berikut dijalankan untuk membangun dan menganalisis proyek:
cd /opt/LibrePCB mkdir build && cd build qmake -r ../librepcb.pro pvs-studio-analyzer trace -- make -j2 pvs-studio-analyzer analyze -l /mnt/Share/PVS-Studio.lic -r /opt/LibrePCB \ -o /opt/LibrePCB/LibrePCB.log -v -j4 cp -R -L -a /opt/LibrePCB /mnt/Share
Omong-omong, semua tindakan dilakukan di Windows 10. Sangat menyenangkan bahwa pengembang semua sistem operasi populer juga berkembang ke arah ini. Sayangnya, wadah dengan Windows tidak begitu nyaman. Terutama karena ketidakmungkinan untuk menginstal perangkat lunak dengan mudah.
Ditemukan bug
Sekarang bagian klasik yang berisi deskripsi kesalahan yang kami temukan menggunakan penganalisis kode statis PVS-Studio. Ngomong-ngomong, dengan mengambil kesempatan ini, saya ingin mengingatkan Anda bahwa baru-baru ini kami sedang mengembangkan alat analisis ke arah analisis kode pendukung untuk sistem tertanam. Berikut adalah beberapa artikel yang mungkin dilewati oleh beberapa pembaca kami:
- PVS-Studio mencakup dukungan untuk GNU Arm Embedded Toolchain ;
- PVS-Studio: dukungan untuk standar pengkodean MISRA C dan MISRA C ++ .
Salah ketik
SymbolPreviewGraphicsItem::SymbolPreviewGraphicsItem( const IF_GraphicsLayerProvider& layerProvider, const QStringList& localeOrder, const Symbol& symbol, const Component* cmp, const tl::optional<Uuid>& symbVarUuid, const tl::optional<Uuid>& symbVarItemUuid) noexcept { if (mComponent && symbVarUuid && symbVarItemUuid) .... if (mComponent && symbVarItemUuid && symbVarItemUuid)
Peringatan PVS-Studio:
V501 CWE-571 Ada sub-ekspresi identik 'symbVarItemUuid' di sebelah kiri dan di sebelah kanan operator '&&'. symbolpreviewgraphicsitem.cpp 74
Kesalahan ketik
klasik :
symbVarItemUuid variabel diperiksa dua kali berturut-turut. Ada pemeriksaan serupa di atas, dan melihatnya, menjadi jelas bahwa variabel kedua yang akan diperiksa harus
symbVarUuid .
Cuplikan kode ketik berikut:
void Clipper::DoMaxima(TEdge *e) { .... if (e->OutIdx >= 0) { AddOutPt(e, e->Top); e->OutIdx = Unassigned; } DeleteFromAEL(e); if (eMaxPair->OutIdx >= 0) { AddOutPt(eMaxPair, e->Top);
Peringatan PVS-Studio:
V778 CWE-682
Ditemukan dua fragmen kode yang serupa. Mungkin, ini adalah kesalahan ketik dan variabel 'eMaxPair' harus digunakan alih-alih 'e'. clipper.cpp 2999
Kemungkinan besar, kode itu ditulis menggunakan Copy-Paste. Karena kesalahan pada blok teks kedua, mereka lupa untuk mengganti
e-> Top dengan
eMaxPair-> Top .
Pemeriksaan ekstra
static int rndr_emphasis(hoedown_buffer *ob, const hoedown_buffer *content, const hoedown_renderer_data *data) { if (!content || !content->size) return 0; HOEDOWN_BUFPUTSL(ob, "<em>"); if (content) hoedown_buffer_put(ob, content->data, content->size); HOEDOWN_BUFPUTSL(ob, "</em>"); return 1; }
Peringatan PVS-Studio:
V547 CWE-571 'Konten' ekspresi selalu benar. html.c 162
Ini mungkin masih bukan kesalahan, tetapi hanya kode yang berlebihan. Tidak perlu memeriksa ulang pointer
konten . Jika nol, maka fungsi segera menyelesaikan pekerjaannya.
Situasi serupa:
void Clipper::DoMaxima(TEdge *e) { .... else if( e->OutIdx >= 0 && eMaxPair->OutIdx >= 0 ) { if (e->OutIdx >= 0) AddLocalMaxPoly(e, eMaxPair, e->Top); DeleteFromAEL(e); DeleteFromAEL(eMaxPair); } .... }
Peringatan PVS-Studio:
V547 CWE-571 Ekspresi 'e-> OutIdx> = 0' selalu benar. clipper.cpp 2983
Memeriksa ulang (e-> OutIdx> = 0) tidak masuk akal. Namun, mungkin ini kesalahan. Sebagai contoh, kita dapat mengasumsikan bahwa variabel
e-> Top harus diperiksa. Namun, ini hanya dugaan. Kami tidak terbiasa dengan kode proyek dan tidak dapat membedakan kesalahan dari kode redundan :).
Dan kasus lain:
QString SExpression::toString(int indent) const { .... if (child.isLineBreak() && nextChildIsLineBreak) { if (child.isLineBreak() && (i > 0) && mChildren.at(i - 1).isLineBreak()) {
Peringatan PVS-Studio:
V571 CWE-571 Pemeriksaan berulang. Kondisi 'child.isLineBreak ()' sudah diverifikasi di baris 208. sexpression.cpp 209
Kesalahan dalam logika
void FootprintPreviewGraphicsItem::paint(....) noexcept { .... for (const Circle& circle : mFootprint.getCircles()) { layer = mLayerProvider.getLayer(*circle.getLayerName()); if (!layer) continue;
Peringatan PVS-Studio:
V547 CWE-571 Ekspresi 'lapisan' selalu benar. footprintpreviewgraphicsitem.cpp 177
Karena kondisi di kedua
jika pernyataan selalu benar, cabang
lain tidak pernah puas.
Periksa pointer yang terlupakan
extern int ZEXPORT unzGetGlobalComment ( unzFile file, char * szComment, uLong uSizeBuf) { .... if (uReadThis>0) { *szComment='\0'; if (ZREAD64(s->z_filefunc,s->filestream,szComment,uReadThis)!=uReadThis) return UNZ_ERRNO; } if ((szComment != NULL) && (uSizeBuf > s->gi.size_comment)) *(szComment+s->gi.size_comment)='\0'; .... }
Peringatan PVS-Studio:
V595 CWE-476 Pointer 'szComment' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 2068, 2073. unzip.c 2068
Jika
uReadThis> 0 , maka pointer
szComment mengalami
penurunan nilai. Ini berbahaya karena pointer ini mungkin nol. Penganalisa membuat kesimpulan berdasarkan fakta bahwa lebih lanjut pointer ini diperiksa untuk
NULL kesetaraan.
Anggota kelas yang tidak diinisialisasi
template <class T> class Edge { public: using VertexType = Vector2<T>; Edge(const VertexType &p1, const VertexType &p2, T w=-1) : p1(p1), p2(p2), weight(w) {};
Peringatan PVS-Studio:
V730 CWE-457 Tidak semua anggota kelas diinisialisasi di dalam konstruktor. Pertimbangkan untuk memeriksa: isBad. edge.h 14
Semua konstruktor, kecuali yang pertama, menginisialisasi bidang kelas
isBad . Kemungkinan besar, konstruktor pertama secara tidak sengaja lupa melakukan inisialisasi ini. Akibatnya, konstruktor pertama membuat objek diinisialisasi tidak lengkap, yang dapat menyebabkan perilaku program yang tidak ditentukan.
Ada 11
pemicu diagnostik
V730 lainnya. Namun, karena kita tidak terbiasa dengan kode, sulit untuk mengatakan apakah peringatan ini menunjukkan masalah nyata. Saya pikir peringatan ini paling baik dipelajari oleh penulis proyek.
Kebocoran memori
template <typename ElementType> void ProjectLibrary::loadElements(....) { .... ElementType* element = new ElementType(elementDir, false);
Peringatan PVS-Studio:
V773 CWE-401 Pengecualian dilemparkan tanpa melepaskan penunjuk 'elemen'. Kebocoran memori dimungkinkan. projectlibrary.cpp 245
Jika elemen sudah ada dalam daftar, pengecualian akan dilempar. Tapi ini tidak menghancurkan objek yang dibuat sebelumnya, penunjuk yang disimpan dalam variabel
elemen .
Jenis pengecualian tidak valid
bool CmdRemoveSelectedSchematicItems::performExecute() { .... throw new LogicError(__FILE__, __LINE__); .... }
Peringatan PVS-Studio:
V1022 CWE-755 Pengecualian dilemparkan oleh pointer. Pertimbangkan untuk melemparkannya dengan nilai saja. cmdremoveselectedschematicitems.cpp 143
Penganalisa mendeteksi pengecualian yang dilemparkan oleh pointer. Paling sering adalah kebiasaan untuk melempar pengecualian dengan nilai, dan menangkap dengan referensi. Melempar pointer dapat menyebabkan pengecualian tidak ditangkap, karena akan ditangkap oleh referensi. Juga, menggunakan pointer memaksa pencegat untuk memanggil operator
hapus untuk menghancurkan objek yang dibuat sehingga kebocoran memori tidak terjadi.
Secara umum, operator
baru ditulis di sini secara tidak sengaja dan harus dihapus. Bahwa ini adalah kesalahan dikonfirmasi oleh fakta bahwa di semua tempat lain dikatakan:
throw LogicError(__FILE__, __LINE__);
Penggunaan dynamic_cast berbahaya
void GraphicsView::handleMouseWheelEvent( QGraphicsSceneWheelEvent* event) noexcept { if (event->modifiers().testFlag(Qt::ShiftModifier)) .... } bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... handleMouseWheelEvent(dynamic_cast<QGraphicsSceneWheelEvent*>(event)); .... }
Peringatan PVS-Studio:
V522 CWE-628 Dereferencing dari 'pointer' null pointer mungkin terjadi. Pointer null potensial dilewatkan ke fungsi 'handleMouseWheelEvent'. Periksa argumen pertama. Periksa baris: 143, 252. graphicsview.cpp 143
Pointer yang dihasilkan dari operator
dynamic_cast dilewatkan ke fungsi
handleMouseWheelEvent . Di dalamnya, penunjuk ini dereferensi tanpa verifikasi sebelumnya.
Ini berbahaya karena operator
dynamic_cast dapat mengembalikan
nullptr . Ternyata kode ini tidak lebih baik daripada hanya menggunakan
static_cast yang lebih cepat.
Pemeriksaan penunjuk eksplisit harus ditambahkan ke kode ini sebelum digunakan.
Juga, kode semacam ini sangat umum:
bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... QGraphicsSceneMouseEvent* e = dynamic_cast<QGraphicsSceneMouseEvent*>(event); Q_ASSERT(e); if (e->button() == Qt::MiddleButton) .... }
Peringatan PVS-Studio:
V522 CWE-690 Mungkin ada
referensi negatif tentang penunjuk nol potensial 'e'. graphicsview.cpp 206
Pointer diperiksa menggunakan makro
Q_ASSERT . Berikut ini uraiannya:
Mencetak pesan peringatan yang berisi nama file kode sumber dan nomor baris jika pengujian salah.
Q_ASSERT () berguna untuk menguji pra dan pasca kondisi selama pengembangan. Tidak ada artinya jika QT_NO_DEBUG ditentukan saat kompilasi.
|
Q_ASSERT adalah cara yang buruk untuk memeriksa pointer sebelum digunakan. Sebagai aturan, dalam versi Rilis
QT_NO_DEBUG tidak ditentukan. Saya tidak tahu bagaimana keadaan di proyek LibrePCB. Namun, jika
QT_NO_DEBUG didefinisikan dalam Rilis, maka ini adalah solusi yang aneh dan tidak standar.
Jika makro berkembang menjadi void, ternyata tidak ada verifikasi. Dan kemudian tidak jelas mengapa menggunakan
dynamic_cast sama sekali. Mengapa tidak menggunakan
static_cast ?
Secara umum, kode ini tidak berbau dan layak ditinjau dari semua fragmen kode yang serupa. Dan ada banyak dari mereka.
Saya menghitung 82 kasus serupa!Kesimpulan
Secara umum, proyek LibrePCB menurut kami berkualitas tinggi. Namun demikian, kami merekomendasikan bahwa penulis proyek mempersenjatai diri dengan alat PVS-Studio dan secara independen melakukan Peninjauan Kode pada bagian kode yang ditunjukkan oleh penganalisa. Kami siap memberi mereka lisensi gratis selama sebulan untuk analisis penuh proyek. Selain itu, mereka dapat menggunakan opsi lisensi penganalisis gratis, karena kode proyek terbuka dan diposting di situs web GitHub. Kami akan segera menulis tentang opsi lisensi ini.
Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Andrey Karpov, Svyatoslav Razmyslov.
Memeriksa LibrePCB dengan PVS-Studio Inside a Docker Container .