👩🏼‍🌾 👩🏾‍🤝‍👩🏼 🤘🏼 Dockerコンテナー内でPVS-Studioを使用したLibrePCBプロジェクトの検証 🚵🏾 🚡 🕵🏾

これは、PVS-Studio静的コードアナライザーを使用してオープンLibrePCBプロジェクトをチームがどのようにテストしたかに関する古典的な記事です。ただし、検証はDockerコンテナー内で実行されたという点で興味深い記事です。コンテナを使用する場合、この記事がアナライザを開発プロセスに統合する別の簡単な方法を示すことを期待しています。

LibrePCB

LibrePCBは、電子回路とプリント基板を設計するための無料ソフトウェアです。プログラムコードはC ++で記述され、Qt5はグラフィカルインターフェイスの構築に使用されます。最近、このアプリケーションの最初の公式リリースが行われ、独自のファイル形式（* .lp、* .lplib）が安定化されました。 Linux、macOS、およびWindows用に準備されたバイナリパッケージ。

LibrePCBは、CおよびC ++で約300,000行の空でないコードのみを含む小さなプロジェクトです。同時に、空でない行の25％はコメントです。ところで、これはコメントの大きな割合です。ほとんどの場合、これはプロジェクトに多くの小さなファイルがあり、その大部分がプロジェクト情報とライセンスからの見出しコメントで占められているという事実によるものです。 GitHubサイトコード： LibrePCB

このプロジェクトは私たちにとって興味深く思えたので、チェックすることにしました。しかし、テスト結果はもはやそれほど面白くありませんでした。はい、本当の間違いがありました。しかし、記事の読者に確かに伝える必要のある特別なものはありませんでした。おそらく、私たちは記事を書かず、エラーをプロジェクトの開発者に送ることに限定しました。ただし、興味深い点は、プロジェクトがDockerイメージ内でテストされたため、記事を書く価値があると判断したことです。

Docker

Dockerは、オペレーティングシステムレベルの仮想化環境でのアプリケーションの展開と管理を自動化するためのソフトウェアです。これにより、アプリケーションをそのすべての環境と依存関係とともにコンテナに「パック」できます。このテクノロジーは5年ほど前のものであり、多くの企業がプロジェクトインフラストラクチャにDockerを実装してきましたが、オープンソースの世界では最近までこれはあまり目立ちませんでした。

当社は、オープンソースプロジェクトと緊密に連携し、独自のPVS-Studio静的アナライザーを使用してソースコードをチェックします。現在、 300以上のプロジェクトが検証されています。このアクティビティで最も難しいことは、常に他の人のプロジェクトを編集することでしたが、Dockerコンテナの導入によりこのプロセスが大幅に簡素化されました。

Dockerでオープンソースプロジェクトをテストした最初の経験は、 Azure Service Fabricでした。そこで、開発者はソースファイルディレクトリをコンテナにインストールし、アナライザの統合はコンテナで実行されているスクリプトの1つを編集することに制限されていました。

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))

LibrePCBプロジェクトの違いは、イメージとプロジェクトをビルドするためのDockerfileをすぐに提供したことです。それは私たちにとってさらに便利であることが判明しました。 Dockerファイルの中で興味のある部分を次に示します。

 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://..../LibrePCB.git /opt/LibrePCB \ && cd /opt/LibrePCB .... # build and install librepcb RUN /opt/LibrePCB/dev/docker/make_librepcb.sh ....

イメージのアセンブリ中にプロジェクトのコンパイルとインストールは行いません。したがって、プロジェクトの作成者がプロジェクトの正常なアセンブリを保証する画像を収集しました。

コンテナの起動後、アナライザーがインストールされ、プロジェクトをビルドおよび分析するために次のコマンドが実行されました。

 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

ちなみに、すべてのアクションはWindows 10で実行されました。すべての一般的なオペレーティングシステムの開発者がこの方向で開発していることは非常に喜ばしいことです。残念ながら、Windowsを備えたコンテナはそれほど便利ではありません。特にソフトウェアを簡単にインストールすることは不可能だからです。

見つかったバグ

PVS-Studio静的コードアナライザーを使用して発見したエラーの説明を含む古典的なセクションになりました。ちなみに、この機会を利用して、最近組み込みシステムのコード分析をサポートする方向でアナライザーを開発していることを思い出したいと思います。読者の一部が読み飛ばしたかもしれない記事をいくつかご紹介します。

タイプミス

 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) // <= .... }

PVS-Studio警告： V501 CWE-571「&&」演算子の左側と右側に同じ副次式「symbVarItemUuid」があります。 symbolpreviewgraphicsitem.cpp 74

古典的なタイプミス：変数symbVarItemUuidは連続して2回チェックされます。上記と同様のチェックがあり、それを見ると、チェックする2番目の変数がsymbVarUuidであることが明らかになります。

次のタイプミスのコードスニペット：

 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); // <= eMaxPair->OutIdx = Unassigned; } DeleteFromAEL(eMaxPair); .... }

PVS-Studio警告： V778 CWE-682 2つの同様のコードフラグメントが見つかりました。おそらく、これはタイプミスであり、「e」の代わりに「eMaxPair」変数を使用する必要があります。 clipper.cpp 2999

ほとんどの場合、コードはCopy-Pasteを使用して記述されています。テキストの2番目のブロックの見落としのため、 e-> TopをeMaxPair-> Topに置き換えるのを忘れていました。

追加のチェック

 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; }

PVS-Studio警告： V547 CWE-571式「コンテンツ」は常に真です。 html.c 162

これはおそらくまだエラーではなく、単に冗長なコードです。 コンテンツポインターを再確認する必要はありません。ゼロの場合、関数はすぐに作業を終了します。

同様の状況：

 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); } .... }

PVS-Studio 警告： V547 CWE-571式 'e-> OutIdx> = 0'は常にtrueです。 clipper.cpp 2983

再確認（e-> OutIdx> = 0）は意味がありません。しかし、おそらくこれは間違いです。たとえば、変数e-> Topをチェックする必要があると仮定できます。ただし、これは単なる予言です。私たちはプロジェクトコードに精通しておらず、エラーを冗長コードと区別できません:)。

そして別のケース：

 QString SExpression::toString(int indent) const { .... if (child.isLineBreak() && nextChildIsLineBreak) { if (child.isLineBreak() && (i > 0) && mChildren.at(i - 1).isLineBreak()) { // too many line breaks ;) } else { str += '\n'; } } .... }

PVS-Studio警告： V571 CWE-571繰り返しチェック。「child.isLineBreak（）」条件は、208行目で既に検証されています。sexpression.cpp209

論理エラー

 void FootprintPreviewGraphicsItem::paint(....) noexcept { .... for (const Circle& circle : mFootprint.getCircles()) { layer = mLayerProvider.getLayer(*circle.getLayerName()); if (!layer) continue; // <= if (layer) { // <= pen = QPen(....); painter->setPen(pen); } else painter->setPen(Qt::NoPen); .... } .... }

PVS-Studio警告： V547 CWE-571式「レイヤー」は常に真です。フットプリントpreviewgraphicsitem.cpp 177

2番目のifステートメントの条件は常にtrueであるため、 elseブランチは満たされません。

忘れられたポインターチェック

 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'; .... }

PVS-Studio警告： V595 CWE-476 nullptrに対して検証される前に、 'szComment'ポインターが使用されました。行を確認してください：2068、2073。unzip.c 2068

uReadThis> 0の場合、 szCommentポインターは逆参照されます。このポインターはヌルである可能性があるため、これは危険です。アナライザは、このポインタがさらにNULLであるかどうかがチェックされるという事実に基づいて、このような結論を出します。

初期化されていないクラスメンバー

 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) {}; // <= Edge(const Edge &e) : p1(e.p1), p2(e.p2), weight(e.weight), isBad(false) {}; Edge() : p1(0,0), p2(0,0), weight(0), isBad(false) {} VertexType p1; VertexType p2; T weight=0; bool isBad; };

PVS-Studio警告： V730 CWE-457クラスのすべてのメンバーがコンストラクター内で初期化されるわけではありません。検査を検討してください：isBad。 edge.h 14

最初のコンストラクタを除くすべてのコンストラクタは、 isBadクラスのフィールドを初期化します。ほとんどの場合、最初のコンストラクターは、誤ってこの初期化を忘れてしまいました。結果として、最初のコンストラクターは不完全に初期化されたオブジェクトを作成し、未定義のプログラムの動作を引き起こす可能性があります。

さらに11 個のV730診断トリガーがあります。ただし、コードに精通していないため、これらの警告が実際の問題を示しているかどうかを判断するのは困難です。これらの警告は、プロジェクトの作者によって最もよく研究されていると思います。

メモリリーク

 template <typename ElementType> void ProjectLibrary::loadElements(....) { .... ElementType* element = new ElementType(elementDir, false); // can throw if (elementList.contains(element->getUuid())) { throw RuntimeError( __FILE__, __LINE__, QString(tr("There are multiple library elements with the same " "UUID in the directory \"%1\"")) .arg(subdirPath.toNative())); } .... }

PVS-Studio警告： V773 CWE-401「エレメント」ポインターを解放せずに例外がスローされました。メモリリークが発生する可能性があります。 projectlibrary.cpp 245

要素がすでにリストにある場合、例外がスローされます。ただし、これにより、以前に作成されたオブジェクト（要素変数に格納されているポインター）は破棄されません。

無効な例外タイプ

 bool CmdRemoveSelectedSchematicItems::performExecute() { .... throw new LogicError(__FILE__, __LINE__); .... }

PVS-Studio警告： V1022 CWE-755ポインターによって例外がスローされました。代わりに値でスローすることを検討してください。 cmdremoveselectedschematicitems.cpp 143

アナライザーは、ポインターによってスローされた例外を検出しました。ほとんどの場合、値によって例外をスローし、参照によってキャッチするのが習慣です。ポインタをスローすると、参照によってキャッチされるため、例外がキャッチされない可能性があります。また、ポインターを使用すると、インターセプターは削除演算子を呼び出して、作成されたオブジェクトを破棄し、メモリリークが発生しないようにします。

一般に、 新しい演算子は偶然ここに書き込まれ、削除する必要があります。これが間違いであることは、他のすべての場所で次のように述べられているという事実によって確認されます。

 throw LogicError(__FILE__, __LINE__);

dynamic_castの危険な使用

 void GraphicsView::handleMouseWheelEvent( QGraphicsSceneWheelEvent* event) noexcept { if (event->modifiers().testFlag(Qt::ShiftModifier)) .... } bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... handleMouseWheelEvent(dynamic_cast<QGraphicsSceneWheelEvent*>(event)); .... }

PVS-Studio警告： V522 CWE-628 nullポインター「イベント」の逆参照が行われる場合があります。潜在的なヌルポインターは、「handleMouseWheelEvent」関数に渡されます。最初の引数を調べます。行を確認：143、252。graphicsview.cpp 143

dynamic_cast演算子の結果のポインターは、 handleMouseWheelEvent関数に渡されます。その中で、このポインタは事前の検証なしに逆参照されます。

dynamic_cast演算子がnullptrを返す可能性があるため、これは危険です。このコードは、より高速なstatic_castを使用するよりも優れていることがわかります。

使用する前に、このコードに明示的なポインターチェックを追加する必要があります。

また、この種のコードは非常に一般的です：

 bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... QGraphicsSceneMouseEvent* e = dynamic_cast<QGraphicsSceneMouseEvent*>(event); Q_ASSERT(e); if (e->button() == Qt::MiddleButton) .... }

PVS-Studio警告： V522 CWE-690潜在的なヌルポインター「e」の逆参照がある可能性があります。 graphicsview.cpp 206

ポインターは、 Q_ASSERTマクロを使用してチェックされます。彼の説明は次のとおりです。

testがfalseの場合、ソースコードファイル名と行番号を含む警告メッセージを出力します。

Q_ASSERT（）は、開発中の事前および事後条件のテストに役立ちます。コンパイル中にQT_NO_DEBUGが定義されていた場合は何もしません。

Q_ASSERTは、使用前にポインターをチェックするのに悪い方法です。原則として、リリースバージョンではQT_NO_DEBUGは定義されていません。 LibrePCBプロジェクトの状況はわかりません。ただし、リリースでQT_NO_DEBUGが定義されている場合、これは奇妙で非標準的なソリューションです。

マクロがボイドに展開される場合、検証がないことがわかります。そして、 dynamic_castを使用する理由がまったく明確ではありません。なぜstatic_castを使用しないのですか？

一般に、このコードは無臭であり、すべての同様のコードフラグメントを確認する価値があります。そして、ところで、それらの多くがあります。 82件の同様のケースを数えました！

おわりに

一般的に、LibrePCBプロジェクトは高品質のように思えました。それでも、プロジェクトの作成者はPVS-Studioツールを使用して、アナライザーによって示されたコードセクションのコードレビューを独自に実施することをお勧めします。プロジェクトの完全な分析のために、1か月間無料のライセンスを提供する準備ができています。さらに、プロジェクトコードが開いており、GitHub Webサイトに投稿されているため、無料のアナライザライセンスオプションを使用できます。このライセンスオプションについては近日中にお知らせします。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov、Svyatoslav Razmyslov。 Dockerコンテナ内のPVS-StudioでLibrePCBを確認する。

Dockerコンテナー内でPVS-Studioを使用したLibrePCBプロジェクトの検証