👩🏻‍🔬 🧒🏾 🏐 静的コードアナライザーを実装する理想的な方法 👶🏿 🧣 👩🏻‍⚕️

静的分析ツールを使用する際の主な問題の1つは、誤検知の処理です。アナライザー設定を使用したり、コードを変更したりして、誤検知を排除する方法は多数あります。私はWindowsプロジェクト用の小さなApple IIエミュレーターを取り上げ、PVS-Studio静的アナライザーレポートを実際に使用する方法を示しました。エラーを修正し、誤検知を抑制する方法の例を示します。

はじめに

プロジェクトに静的分析手法を導入する理想的なプロセスを説明します。これは、アナライザーのすべての誤ったアラームと実際のエラーを排除して、アナライザーが警告を生成しないようにすることで構成されています。これは、Unreal Engine 4プロジェクトで作業するときに使用したアプローチです。

実際には、理想的なアプローチはほとんど不可能です。したがって、大規模なプロジェクトでは、代替アプローチを使用するのが賢明です。現在使用可能なすべての警告を非表示にして、新規または変更されたコードに関連するメッセージのみを表示できます。このため、PVS-Studioアナライザーには、特別なデータベースに情報を保存する特別なメカニズムがあります。詳細については、記事をご覧ください： 10メガバイトを超えるソースコードを使用して静的分析をプロジェクトに実装する方法

そのため、すべてのメッセージを非表示にして、新しいコードの品質を慎重に監視できます。新しいコードでエラーを見つけると、静的解析手法の威力と利点をすぐに理解できます。また、空き時間がある場合は、非表示の警告に戻り、コードに必要な変更を徐々に加えることができます。

しかし、完璧な幸せの世界に戻りましょう。時間があり、PVS-Studioアナライザーが生成する警告を安全に解決できると想像してください。

この記事では、アナライザーの警告を処理する方法を示します。そして、最初のチェックから警告が0になるウィンドウまで、完全に進みます。

それが私が小さなプロジェクトを選んだ理由です。私はもっと取ることができますが、それから記事を書くのにうんざりし、あなたはそれを読むでしょう。しかし、あなたはまだ疲れています。小規模なプロジェクトであっても記事は大きくなりますが、注意してください。コードアナライザーをより有効に活用するのに役立ちます。

実験用マウスは、 Windowsプロジェクト用のApple IIエミュレーターでした。選択は偶然彼に完全に落ちました。したがって、私たちはそれにこだわらないでしょう。どのプロジェクトを採用するかは気にしませんでした。主なことは小さくすることでしたが、同時に何か面白いことがありました。

プロジェクトの特徴：

ソースコードサイズ：3メガバイト。
コードの行数：85700。
分析時間（8プロセッサーコア）：30秒。

最初の打ち上げ

アナライザーの最初の起動後、次の警告が表示されます。

図1。 Windowsプロジェクト用のApple IIエミュレーターのPVS-Studioアナライザーの最初の起動時に発行される警告。

図1. Windowsプロジェクト用のApple IIエミュレーターのPVS-Studioアナライザーの最初の起動時に発行される警告。

この記事では、一般分析（GA）に関連するレベル1およびレベル2の警告についてのみ説明します。レベル3で「勝つ」ことは可能ですが、記事は引きずられます。レベル3を非常にすばやく確認し、いくつか説明しますが、何も修正しません。

微小最適化（OP）：今では面白くない。

64ビット診断について：プロジェクトには64ビット構成がありません。面白くない。

プロジェクトを確認した後、すべての警告をコードでソートしました。これは、「コード」列をクリックして実行できます（図N2を参照）。

図2。 PVS-Studioメッセージウィンドウ。メッセージは診断番号でソートされます。

図2. PVS-Studioメッセージウィンドウ。 メッセージは診断番号でソートされます。

コードの並べ替えにより、メッセージングが容易になります。同じタイプのメッセージのグループが発生します。 1つのメッセージが発生する理由を理解したら、残りを簡単かつ迅速に処理できます。

ご注意 読者は、なぜこのソートをすぐに行わないのか疑問に思うかもしれません。実際、ユーザーが分析プロセスでメッセージを見ることができるようにしたいのです。それらをソートすると、分析プロセスの新しいメッセージは最後に追加されず、リスト内の別の場所に追加されます。その結果、メッセージは「ジャンプ」します。このような「ぴくぴくした」リストで作業することは不可能です。

アナライザーメッセージの操作

ソリューションには3つのプロジェクトがあります（図N2の[ソリューションエクスプローラー]ウィンドウに表示されます）。それらのうちの2つ、zlibとzip_libは、チェックするのに興味がありません。したがって、それらは分析から除外する必要があります。実際には、zlibはすでにデフォルトで例外に追加されているため、zip_libを除外するだけで十分です。これはPVS-Studioの設定ウィンドウで行います（[ ファイルをチェックしない]セクション）：

図N3。チェックからzip_libを除外します。

図N3。 チェックからzip_libを除外しました。

不要なプロジェクトを事前に除外しました。ただし、これは確認後に簡単に行えます。そして、これのための設定に行く必要はありません。コンテキストメニューには、ファイルまたは特定のディレクトリに関連するすべてのメッセージを簡単に非表示にできるアイテムがあります。とても便利です。記事「 PVS-Studio for Visual C ++ 」に精通することをお勧めします。ツールの効果的な使用に役立つこの機能およびその他の機能について説明します。

これで、メッセージの処理を開始する準備が整いました。番号V501の診断から始めて、さらに進んでいきましょう。合計で、32 + 49 = 81件の投稿を検討します。これはかなりたくさんあります。したがって、いくつかの場所では詳細に説明し、いくつかの場所では簡単に説明します。

マクロ偽アラームxxxxxREG

最初の6つのメッセージは、ADDXXREG、ADCHLREG、SBCHLREG、SBCHLREGの複雑なマクロによるものです。それらが開かれると、冗長な構造が発生し、アナライザは、たとえば次のメッセージを生成します。

V501 '^'演算子の左右に同じ部分式があります：（tmp >> 8）^ reg_ixh ^ reg_ixh z80.cpp 3444

ADDXXREGマクロは非常に大きく、他のマクロで構成されています。したがって、記事では引用しません。

XOR操作は、変数reg_ixhを使用して2回実行されることが重要です。したがって、式は（tmp >> 8）に簡略化できます。ただし、ここで間違いはありません。特定のマクロ引数を置換するときに、過剰な式であることが判明しました。

ADDXXREG（reg_ixh、reg_ixl、reg_ixh、reg_ixl、15、2）;

これらは誤検知であり、排除する必要があります。それらに関連するすべての警告を抑制します。これを行うために、これらのマクロが宣言されているヘッダーファイルに、次のコメントを追加しました。

//-V：ADDXXREG：501
//-V：ADCHLREG：501
//-V：SBCHLREG：501
//-V：SBCHLREG：501

この警告抑制メカニズムの詳細については、ドキュメントの対応するセクションを参照してください。

原則として、1つのコメントで対応できます。すべてのマクロの名前には「REG」が付いています。したがって、コメントを1つ書くことができます：//-V：REG：501。「REG」が発生する行のV501警告を抑制します。ただし、上記のマクロに関連しない有用なメッセージを誤って非表示にする可能性があるため、これは悪いことです。検索にブラケットを追加することで、状況を少し改善できます。//-V：REG（：501。

sprintf（）関数パラメーターのエラー

sprintf( sText, "%s %s = %s\n" , g_aTokens[ TOKEN_COMMENT_EOL ].sToken , g_aParameters[ PARAM_CATEGORY ].m_sName , g_aParameters[ eCategory ] );

警告：V510「sprintf」関数は、クラスタイプ変数を5番目の実引数として受け取ることは想定されていません。 debug.cpp 2300

実際、5番目の実引数はCommand_t型の構造体です。どうやら、g_aParameters [eCategory] .m_sNameを引数として使用する必要があります。コードに適切な変更を加えました。

悪臭ゼロメモリ（）

次のメッセージは、1つの配列がいっぱいではないことを示しています。V512「memset」関数を呼び出すと、バッファー「pHDD-> hd_buf」のアンダーフローが発生します。 harddisk.cpp 491

 BYTE hd_buf[HD_BLOCK_SIZE+1]; // Why +1? ZeroMemory(pHDD->hd_buf, HD_BLOCK_SIZE);

最後のバイトはリセットされません。これが間違いかどうかはわかりません。コメントに注意してください。おそらく、開発者自身でさえ、配列のサイズと、それを完全にゼロにするかどうかを知りません。

このコードは無臭であると言われています。必ずしも間違っているわけではありませんが、疑わしく、将来的に間違いを引き起こす可能性があります。

コメントでこの警告を非表示にします。ファイルを自分で変更するか、メニュー項目「選択したメッセージを誤警報としてマークする」を使用できます。

図3.コメントトをコードに追加して警告を抑制します。

図3.コメントをコードに追加して警告を抑制します。

このアイテムを選択すると、アナライザーはコードにコメントを挿入します。

 ZeroMemory(pHDD->hd_buf, HD_BLOCK_SIZE); //-V512

memcpy（）関数呼び出しでの誤検出

 unsigned char random[ 256 + 4 ]; memcpy( &memmain[ iByte ], random, 256 );

memcpy（）関数は、「ランダム」バッファーの一部のみをコピーします。アナライザは、これを疑わしいとみなし、正直に報告します。この場合、彼は間違っており、間違いはありません。前の例のように、コメントを使用して警告を抑制しました。あまり美しくはありませんが、他の人のコードで何をすべきかわかりません。

追加のアクション

 nAddress_ = 0; nAddress_ = (unsigned)*(LPBYTE)(mem + nStack); nStack++; nAddress_ += ((unsigned)*(LPBYTE)(mem + nStack)) << 8;

警告：V519「nAddress_」変数には連続して2回値が割り当てられます。おそらくこれは間違いです。行を確認してください：568、569。debugger_assembler.cpp 569

アナライザーは、変数nAddress_にさまざまな値が連続して数回割り当てられていることに気付きました。ここには間違いはなく、余分なコードがあります。変数に値0が割り当てられている最初の行を削除しました。警告を取り除くもう1つのオプションは、2番目の割り当てを「+ =」に置き換えることです。

同様の状況は、さらに2つのファイルで確認できます。

video.cppファイル（3310行および3315行を参照）。余分な操作「pSrc + = nLen;」を削除しました。

Debug.cppファイル（行5867および5868を参照）。置換：

 char *p = sLine; p = strstr( sLine, ":" );

に

 char *p = strstr( sLine, ":" );

私はこれらの断片については語りません。

switchステートメントのエラー

次のV519診断は、すでに深刻なエラーを示しています。間違いは古典的なものであり、誰もがそれについて知っていますが、私たちはさまざまなプログラムで何度も何度もそれを満たします。

 switch( c ) { case '\\': eThis = PS_ESCAPE; case '%': eThis = PS_TYPE; break; default: sText[ nLen++ ] = c; break; }

警告：V519「p」変数には、連続して2回値が割り当てられます。おそらくこれは間違いです。チェック行：5867、5868。debug.cpp 5868

「eThis = PS_ESCAPE;」の後、「break」ステートメントはありません。このため、「eThis」変数の値はすぐにPS_STYPEに変わります。これは明らかな間違いです。それを修正するために、「break」ステートメントを追加しました。

エラー：常に偽の状態

 inline static ULONG ConvertZ80TStatesTo6502Cycles(UINT uTStates) { return (uTStates < 0) ? 0 : (ULONG) ((double)uTStates / uZ80ClockMultiplier); }

警告：V547式 'uTStates <0'は常にfalseです。符号なしの型の値が<0になることはありません。z80.cpp5507

プログラマは、負の値が関数に渡された場合から身を守りたいと考えていました。ただし、「uTStates」変数は符号なしであるため、保護は機能しません。

タイプ 'INT'に明示的なキャストを追加しました。

 return ((INT)uTStates < 0) ? 0 : (ULONG) ((double)uTStates / uZ80ClockMultiplier);

アナライザーの過度の注意力

次の関数では、配列が範囲外になる可能性があることをアナライザは心配しています。

 void SetCurrentImageDir(const char* pszImageDir) { strcpy(g_sCurrentDir, pszImageDir); int nLen = strlen( g_sCurrentDir ); if( g_sCurrentDir[ nLen - 1 ] != '\\' ) .... }

警告：V557アレイアンダーランが発生する可能性があります。 'nLen-1'インデックスの値は-1に達する可能性があります。 applewin.cpp 553

空の文字列を関数に渡すと、その長さはゼロになります。次に、配列のオーバーフローが発生します：g_sCurrentDir [0-1]。

アナライザーは、そのような状況が可能かどうかを知りません。したがって、念のため、警告します。

そのような状況が可能かどうかはわかりません。可能であれば、アナライザーはエラーを検出しました。そうでない場合、これは誤検知です。

これを誤検知と見なすことにしました。ただし、警告を抑制するコメントを追加するよりも、コードを改善する方が便利です。したがって、関数に追加のチェックを追加します。

 if (nLen == 0) return;

理論的には、配列が範囲外になる場所がもう1つあります。しかし、私たちは記事を濃い参照に変えないようにしなければなりません。したがって、この警告については説明せず、コメントを使用してそれを抑制します。同じファイルを参照してください（556行目）。

比較ではなく割り当て

 if ((bytenum == 3) && (byteval[1] = 0xAA)) {

警告：V560条件式の一部は常に真です：（byteval [1] = 0xAA）。 diskimagehelper.cpp 439

「=」ではなく「==」操作を実行したかったのは間違いありません。割り当てが必要な場合は、次のように記述する方がはるかに自然です。

 if (bytenum == 3) { byteval[1] = 0xAA;

したがって、これは間違いであり、修正する必要があります。

 if ((bytenum == 3) && (byteval[1] == 0xAA))

マクロ誤検知

 if ((TRACKS_MAX>TRACKS_STANDARD) && ....)

警告：V560条件式の一部は常に真です：（（35 + 5）> 35）。 diskimagehelper.cpp 548

マクロを展開すると、式（（35 + 5）> 35）が得られます。この表現は常に真実ですが、間違いではありません。

これは私が何をすべきかさえ知らない場合です。回路図では、コメントを使用して誤検出を抑制しています：//-V560。

追加変数

リファクタリングプロセスでは、「失われた」変数が残ることがあります。それらは何らかの形でコードで使用されていますが、実際には必要ありません。明らかに、これはbForeground変数で起こったこととまったく同じです。

 BOOL bForeground; .... bForeground = FALSE; .... if( bForeground ) dwCoopFlags |= DISCL_FOREGROUND; else dwCoopFlags |= DISCL_BACKGROUND; .... if( hr == DIERR_UNSUPPORTED && !bForeground && bExclusive )

さらに、 'bForeground'変数はどこでも変更または使用されません。これにより、次の警告が表示されます。V560条件式の一部が常にtrue :! BForeground。 mouseinterface.cpp 690

これは哲学の興味深い例です。それは誤検知ですか？人でさえ答えるのは難しいです。アナライザーは異常の検出に最適です。しかし、人間の観点からすると、このフラグメントは不完全なコードである可能性があります。そして、すべてが整然としています。

これは「匂いのあるコード」の別の例であると想定しています。「bForeground」変数をコードから削除しました。

未定義の動作

 *(mem+addr++) = (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4;

警告：V567未定義の動作。 'addr'変数は、シーケンスポイント間で2回使用されている間に変更されます。 cpu.cpp 564

式の計算方法は不明です。

おそらく変数 'addr'が最初に増加し、次に式の右側で使用されます。
そしてその逆かもしれません。

次のコードが正しいでしょう。

 *(mem+addr) = (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4; addr++;

wsprintf（）関数などを呼び出すときの無効な引数

間違った数の実際の引数をフォーマットされた出力関数に渡すことに関連するいくつかのエラーがあります。合計で、このようなエラーが10個見つかりましたが、1つだけを検討します。

 wsprintf( sText, TEXT("%s full speed Break on Opcode: None") , sAction , g_iDebugBreakOnOpcode , g_aOpcodes65C02[ g_iDebugBreakOnOpcode ].sMnemonic );

警告：V576の形式が正しくありません。「wsprintfA」関数の呼び出し中に、異なる数の実引数が予期されます。予想：3.現在：5. debug.cpp 939

文字列を形成するとき、最後の2つのパラメーターは考慮されません。部外者として、これらのパラメーターが不要である、またはフォーマット文字列に間違いがあると言うのは難しいです。

パラメータが不要であると判断し、削除しました。

以下のコードセクションでも同様の問題が発生しています。

予想：8.現在：9. debug.cpp 7377
予想：3.現在：4. debugger_help.cpp 1263
予想：3.現在：4. debugger_help.cpp 1265
予想：3.現在：4. debugger_help.cpp 1267
予想：3.現在：4. debugger_help.cpp 1282
予想：3.現在：4. debugger_help.cpp 1286
予想：3.現在：4. debugger_help.cpp 1288
予想：5.現在：4. debugger_help.cpp 1332
予想：3.現在：4. frame.cpp 691
予想：3.現在：4. frame.cpp 695

「％08X」を使用してポインタ値を出力する場所もいくつかあります。 32ビットシステムでは、これは実際に機能します。ただし、64ビットシステムでは、ポインターの一部のみが出力されます。正しい解決策は、「％p」を使用することです。関連するコードセクション：

ポインターの値を出力するには、「％p」を使用する必要があります。 tfe.cpp 507
ポインターの値を出力するには、「％p」を使用する必要があります。 tfe.cpp 507

二重比較の誤検知

アナライザーに障害はありませんでしたが、繰り返し条件で2つの誤検出が発生しました。 1つのケースを考えてみましょう：

 if (nAddress <= _6502_STACK_END) { sprintf( sText,"%04X: ", nAddress ); PrintTextCursorX( sText, rect ); } if (nAddress <= _6502_STACK_END) { DebuggerSetColorFG( DebuggerGetColor( FG_INFO_OPCODE )); sprintf(sText, " %02X",(unsigned)*(LPBYTE)(mem+nAddress)); PrintTextCursorX( sText, rect ); }

警告：V581互いに並んでいる「if」演算子の条件式は同一です。行を確認：2929、2935。debugger_display.cpp 2935

間違いはありません。プログラマーは、単に2つのアクションセットを分割しました。アナライザーの観点からすると、このようなコードは疑わしいものです。突然条件が異なるはずですか？それにもかかわらず、何かを誤検知で行う必要があります。 2つの条件ステートメントを1つにまとめることにしました。

 if (nAddress <= _6502_STACK_END) { sprintf( sText,"%04X: ", nAddress ); PrintTextCursorX( sText, rect ); DebuggerSetColorFG( DebuggerGetColor( FG_INFO_OPCODE )); sprintf(sText, " %02X",(unsigned)*(LPBYTE)(mem+nAddress)); PrintTextCursorX( sText, rect ); }

コードの可読性はこれに苦しむことはなかったと思いますが、途中で誤検出を取り除きました。

2番目のケースは類似しています：V581互いに並んでいる「if」演算子の条件式は同一です。行を確認してください：2237、2245。debugger_display.cpp 2245

写真12

図5.長い記事の途中で、読者が休憩できるように写真を挿入することをお勧めします。 挿入する記事の画像がわかりません。 したがって、ここではあなたのための猫です。

検証前のポインターの逆参照

合計で、アナライザーはこのトピックに関して3つの警告を発行しました。残念ながら、これらの場所にあるプログラムのテキストはかなり混乱しています。したがって、簡単にするために、実際のコードではなく、擬似コードを提供します。最初の2つの警告については、コードは次のようになります。

 int ZEXPORT unzGetGlobalComment(char *szComment) { .... if (A) { *szComment='\0'; return UNZ_ERRNO; } .... if ((szComment != NULL) && X) .... }

警告：V595 nullptrに対して検証される前に、 'szComment'ポインターが使用されました。チェック行：1553、1558。unzip.c 1553

ご覧のとおり、渡されたポインター 'szComment'はNULLになる場合があります。これは、チェックによって証明されます（szComment！= NULL）。

ただし、チェックを実行せずにポインターが大胆に逆参照されるコードのセクションがあります。これは危険です。実際には、 'szComment'が0の状況は決してない可能性があります。しかし、コードは危険であり、修正する必要があります。

類似：V595 nullptrに対して検証される前に、「pToken_」ポインターが使用されました。行を確認してください：811、823。debugger_parser.cpp 811

しかし、最後の3番目のケースでは、すべてがより複雑です。そのようなコードが間違っていて修正する必要があることを証明するのはもううんざりです。関数は短いので、全体を説明します

 bool ArgsGetValue ( Arg_t *pArg, WORD * pAddressValue_, const int nBase ) { TCHAR *pSrc = & (pArg->sArg[ 0 ]); TCHAR *pEnd = NULL; if (pArg && pAddressValue_) { *pAddressValue_ = (WORD)(_tcstoul( pSrc, &pEnd, nBase) & _6502_MEM_END); return true; } return false; }

警告：V595 nullptrに対して検証される前に、「pArg」ポインターが使用されました。行を確認してください：204、207。debugger_parser.cpp 204

ポインター「pArg」は、「if（pArg && pAddressValue_）」という条件から明らかなように、ゼロになる場合があります。ただし、ポインターがチェックされる前に、式で使用されます。

 TCHAR *pSrc = & (pArg->sArg[ 0 ]);

この式は、未定義のプログラムの動作につながります。 NULLポインターを逆参照することはできません。

多くの人は、このようなコードではメモリアクセスがなく、一部のアドレスのみが計算されることに反対しています。したがって、問題はありません。ただし、これは不明確な動作の解釈が狭すぎる。コンパイラの動作とコードの動作を推測する必要はありません。だからあなたは書くことができず、その理由を議論するのは意味がありません。

このようなコードの未定義の動作は、ゼロメモリアドレスへの参照だけではありません（実際には存在しない場合があります）。たとえば、コンパイラーは、チェック条件を「if（pAddressValue_）」に減らすことができます。「pArg-> xxx」という式があるため、ポインタは間違いなくnullではなく、チェックする必要はありません。

この問題をより詳細に議論する意味はありません。特別な記事を読むことをお勧めします： nullポインターの逆参照は未定義の動作につながります

コードは簡単に修正できます。 'if'内で変数の宣言を転送するだけで十分です。

恐ろしい表情

アナライザーは、次の式で混乱していました。

 if ((cx > 4) & (cx <= 13))

警告：V602 '（cx> 4）'式の検査を検討してください。「>」は「>>」に置き換えられる可能性があります。 debug.cpp 8933

アナライザーは、 '＆'演算子が使用されていることを確認します。オペランドは、適用された型 'bool'です。これは変です。通常、このような場合、論理演算子「&&」を使用するのが一般的です。

「＆」演算子は、ビット単位の演算によく使用されます。したがって、アナライザーは、ここでビットを使用したいという可能性を示唆しました。

 if ((cx >> 4) & (cx <= 13))

アナライザーが賢すぎて間違っています。ただし、プログラマーの責任もここにあります。このコードには匂いがあります。書く方がはるかに自然です：

 if (cx > 4 && cx <= 13)

マクロの不特定の動作と恐怖

負の値を右にシフトすると、結果がどうなるかは明確ではありません。コードの動作は別のコンパイラーで変更される可能性があるため、これを行わない方が良いです。

 const short SPKR_DATA_INIT = (short)0x8000; if (g_nSpeakerData == (SPKR_DATA_INIT >> 2))

警告：V610の動作は指定されていません。シフト演算子「>>」を確認してください。左のオペランド「SPKR_DATA_INIT」は負です。 speaker.cpp 450

終了：定数SPKR_DATA_INITをバンクレスとして宣言できます。確かに、コンパイラとアナライザが符号付き/符号なしの数値の比較に関する警告を受け取らないように、さらにいくつかの小さな変更を行う必要があります。

アナライザーは、このような危険な場所をさらに3つ見つけました。

左のオペランド「SPKR_DATA_INIT」は負です。 speaker.cpp 453
左のオペランド「〜0x180」は負です。 tfe.cpp 869
左のオペランド '〜0x100'は負です。 tfe.cpp 987

ところで、最後の2つの警告の編集を開始したときに、さらに2つのエラーが見つかりました。アナライザーは間接的な方法でそれらを見つけるのに役立ったと言えます。

マクロの使用方法は次のとおりです。

 SET_PP_16(TFE_PP_ADDR_SE_BUSST, busst & ~0x180);

長い文字列に展開されます。したがって、その一部のみを示します。

 ..... = (busst & ~0x180 >> 8) & 0xFF; .....

シフト演算子>>の優先度は、＆演算子の優先度よりも高くなっています。表：操作の優先順位を参照してください。

プログラマは、コードが次のように動作することを期待していました。

 ..... = ((busst & ~0x180) >> 8) & 0xFF; .....

しかし実際には、次のように機能します。

 ..... = (busst & (~0x180 >> 8)) & 0xFF; .....

これが、PVS-Studioアナライザーが警告する理由です：「左オペランド '〜0x180'は負です。」

これは、マクロを使用することの危険性です。

セキュリティホール

このプロジェクトでは、sprintf（）、wsprintf（）関数などを非常に危険な方法で使用しています。要するに、関数は次のように使用されます。

 sprintf(buf, STR);

STR文字列に「％s」などの制御文字が含まれている場合、結果は予測できません。

通常、このコードはセキュリティホールと見なされます（詳細を参照）。

ただし、エミュレータにとっては、これは重要ではないと思います。誰も彼を攻撃しません。ただし、このようなコード自体は危険です。プログラムのクラッシュや誤動作を簡単に引き起こす可能性があります。

正しいことは：sprintf（buf、 "％s"、STR）;

アナライザーは、多くの危険な関数呼び出しを検出しました。合計で、彼は21の警告を発行しました。

反対の条件

 // TO DO: Need way of determining if DirectX init failed if (soundtype != SOUND_WAVE) { if (soundtype == SOUND_WAVE) soundtype = SOUND_SMART;

警告：V637反対の2つの条件が発生しました。 2番目の条件は常にfalseです。行を確認してください：270、272。speaker.cpp 270

コメントから判断すると、コードは追加されていません。このコードをどうするかを言うのは難しいです。 2番目の無意味な「if」をコメントアウトすることにしました。

 if (soundtype != SOUND_WAVE) { //if (soundtype == SOUND_WAVE) // soundtype = SOUND_SMART;

悪いアライメントコード

コードは、両方のアクションがifステートメントに関連しているように見えます。

 { if ((Slot4 == CT_MockingboardC) || (Slot4 == CT_Phasor)) m_PropertySheetHelper.GetConfigNew().m_Slot[4] = CT_Empty; m_PropertySheetHelper.GetConfigNew().m_Slot[5] = CT_SAM; }

警告：V640コードの操作ロジックはそのフォーマットに対応していません。ステートメントは右側にインデントされますが、常に実行されます。中括弧が欠落している可能性があります。 pagesound.cpp 229

私の理解では、コードにエラーはありません。ただし、これは誤検知ではありません。そのようなコードについて警告するとき、アナライザーは絶対に正しいです。アライメントを修正してください：

 { if ((Slot4 == CT_MockingboardC) || (Slot4 == CT_Phasor)) m_PropertySheetHelper.GetConfigNew().m_Slot[4] = CT_Empty; m_PropertySheetHelper.GetConfigNew().m_Slot[5] = CT_SAM; }

strncat（）関数の不適切な動作

 strncat( sText, CHC_DEFAULT, CONSOLE_WIDTH ); strncat( sText, pHelp , CONSOLE_WIDTH );

警告：V645「strncat」関数呼び出しは、「sText」バッファオーバーフローを引き起こす可能性があります。境界には、バッファーのサイズを含めることはできませんが、保持できる文字数を含める必要があります。 debugger_help.cpp 753

関数の3番目の引数は、文字列に追加できる文字数です。したがって、そうすることは正しく安全です。

 strncat( sText, CHC_DEFAULT, sizeof(sText) - strlen(sText) - 1); strncat( sText, pHelp , sizeof(sText) - strlen(sText) - 1);

詳細については、 V645診断プログラムの説明を参照してください。

追加のチェック

非常に長い間、「new」演算子は、メモリを割り当てることができない場合に例外std :: bad_allocをスローしました。ただし、プログラムでは、不要なチェックを見つけることができます。

 BYTE* pNewImageBuffer = new BYTE [uNewImageSize]; _ASSERT(pNewImageBuffer); if (!pNewImageBuffer) return false;

警告：V668は、「new」演算子を使用してメモリが割り当てられたため、「pNewImageBuffer」ポインターをnullに対してテストする意味がありません。メモリ割り当てエラーの場合、例外が生成されます。 diskimagehelper.cpp 197

_ASSERTと検証は削除でき、削除する必要があります。ここでは意味がありません。

同様の状況：

mouseinterface.cpp 175
serialcomms.cpp 839
savestate.cpp 108
savestate.cpp 218
speech.cpp 40

自己宣言システムのタイプ

プロジェクトでは、一貫していくつかのデータ型を定義しています。

 typedef unsigned long ULONG; typedef void *LPVOID; typedef unsigned int UINT;

ここには明らかなエラーはありません。これは「臭いのあるコード」であると想定し、コメント//-V677を使用して警告を抑制します。

「ビッグツーの法則」に違反

たとえば、CConfigNeedingRestartクラスでoperator =が宣言されていますが、コピーコンストラクターはありません。これは「 Big Two Law 」に違反します。

クラスは十分に大きいので、ここではコードの断片を示しません。ひとこと

このクラスでは、すべてのフィールドは単純型であるため、独自の演算子=はまったく必要ありません。クラスは自動的に正常にコピーされます。

Disk_tクラスでは、状況は似ています。あちこちで演算子=を削除できます。

アナライザーの警告：

V690「CConfigNeedingRestart」クラスは「=」演算子を実装しますが、コピーコンストラクターがありません。そのようなクラスを使用するのは危険です。 config.h 7
V690「Disk_t」クラスは「=」演算子を実装しますが、コピーコンストラクターがありません。そのようなクラスを使用するのは危険です。 disk.cpp 74

タイプミス

 int nHeight=nHeight=g_aFontConfig[ FONT_CONSOLE ]._nFontHeight;

警告：V700は、「T foo = foo = ...」式の検査を検討してください。変数がそれ自体で初期化されるのは奇妙です。 debugger_display.cpp 1226

ただのタイプミス。に置き換え：

 int nHeight = g_aFontConfig[ FONT_CONSOLE ]._nFontHeight;

列挙に関するアナライザーの過度の懸念

'AppMode_e'列挙には、MODE_LOGO、MODE_PAUSED、MODE_RUNNING、MODE_DEBUG、MODE_STEPPINGという名前の定数が含まれています。

アナライザーは、すべてのスイッチがこのスイッチで使用されるわけではないことを心配しています（）：

 switch (g_nAppMode) { case MODE_PAUSED : _tcscat(.....); break; case MODE_STEPPING: _tcscat(.....); break; }

警告：V719 switchステートメントは、 'AppMode_e'列挙型のすべての値をカバーしません：MODE_DEBUG、MODE_LOGO、MODE_RUNNING。 frame.cpp 217

この例では、アナライザーを少し恥じています。経験的アルゴリズムが失敗しました。誤検知。修正するにはいくつかの方法があります。たとえば、デフォルトのブランチを追加できます。

 switch (g_nAppMode) { case MODE_PAUSED : _tcscat(.....); break; case MODE_STEPPING: _tcscat(.....); break; default: break; }

別の同様の誤検知：V719 switchステートメントは、 'AppMode_e'列挙のすべての値をカバーしません：MODE_DEBUG、MODE_LOGO。 frame.cpp 1210

私は、第3レベルの警告を簡単に確認することを約束しました。

（少なくとも最初の段階では）レベル3をまったく見ることはお勧めしません。多くの誤った、興味のない、または特定のメッセージがあります。これが状況です。

たとえば、ここにはかなりの数のV601警告があります。

 inline int IsDebugBreakpointHit() { if ( !g_bDebugNormalSpeedBreakpoints ) return false; return _IsDebugBreakpointHit(); }

警告：V601「false」値は暗黙的に整数型にキャストされます。debug.h 210

この関数は、タイプ 'int'を返します。ただし、「falseを返す」と表示されます。

アナライザーはこのコードで障害を見つけるのが正しいですが、実際には、これは実際のエラーによって非常にまれに隠されます。したがって、この警告はレベル3に低下しました。

特定の診断の例：

 double g_fClksPerSpkrSample; .... if ((double)g_nRemainderBufferSize != g_fClksPerSpkrSample)

警告：V550奇妙な正確な比較。fabs（A-B）> Epsilonのように、定義された精度の比較を使用する方がおそらく良いでしょう。speaker.cpp 197

このコードが有効であるかどうかは、アプリケーションと「double」型の変数に配置される値に大きく依存します。

多くのユーザーがこの診断を喜んでいます。他の人は、整数をdoubleに入れて、比較時に何をするかを知っていると言います。あなたは皆を喜ばせません。

エラー編集後に実行

すべてのメッセージ（レベル1および2）を修正したら、アナライザーを再起動できます。結果が期待されます-すべての警告が消えました（図6を参照）。

図6.レベル1およびレベル2のアラートはもうありません。

これは、小規模プロジェクトにのみ適用できる理想的なオプションです。それでも、アナライザーメッセージを操作するのに複雑なものは何もないことを示すことができたと思います。一部のメッセージは誤っていましたが、それらに問題はなく、それらはすべて排除されました。

まとめると

多くの場合、アナライザーが生成する誤検知の数を尋ねられます。答えはありません。このような統計を収集することは非常に困難であり、ほとんど効果はありません。誤検知の数は、プロジェクトによって大きく異なります。

データの解釈にはまだ問題があります。たとえば、プロジェクト全体でアクティブに使用されているマクロが失敗したため、誤検知の数は有用なメッセージの20倍になります。しかし、それは問題ではありません。このマクロで警告を抑制するだけで十分であり、誤ったメッセージの数はすぐに90％減少します。

次に答える難しさは、プログラマーが警告を分類するのが難しいことを考慮していないことです。これらのメッセージはエラーを検出しませんが、「臭いのあるコード」を明らかにします。このようなコードは修正する必要があります。現在は正常に動作していますが、将来的には欠陥を引き起こす可能性があるためです。この記事では、このような診断の例をいくつか示しました。

しかし、プログラマーはバイナリロジックに引き寄せられます。そして、彼らは答えを受け取ることを主張します：「これは間違ったメッセージですか？はい/いいえ？」記事を注意深く読んでいただければ、質問をそれほど厳密に提起しないことを願っています。

ご覧のとおり、誤検知の数について一般的に話すことは困難です。しかし、特定の小さなプロジェクトを取り上げる場合は、そのためだけに答えを出すことができます。

PVS-Studioアナライザーによって発行されたWindowsプロジェクト用のApple IIエミュレーターの診断メッセージの要約：

発行されたメッセージの総数（一般分析、第1レベルおよび第2レベル）：81
見つかったエラー：57
修正すべき「匂いのあるコード」の断片が見つかりました：9
誤検知：15

割合を要約するには：

エラーを示すメッセージ：70％
「ラップアラウンドコード」を示すメッセージ：11％
誤検知：19％

おわりに

プロジェクトでPVS-Studio静的アナライザーを試してください。デモバージョンは、http ：//www.viva64.com/en/pvs-studio-download/からダウンロードできます。

また、同僚や友人に静的アナライザーを試すことをお勧めします。Twitterまたは他のニュースフィードにメッセージを書いてください。ありがとう

PS私のTwitterを購読すれば、新しい記事をフォローし、一般的にC / C ++の世界からニュースを学ぶことができます：https : //twitter.com/Code_Analysis

ご清聴ありがとうございました。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey karpov。静的コードアナライザーをプロジェクトに統合する理想的な方法。

記事を読んで質問がありますか？

多くの場合、記事には同じ質問が寄せられます。ここで回答を集めました： PVS-Studioバージョン2015に関する記事の読者からの質問への回答。リストをご覧ください。

静的コードアナライザーを実装する理想的な方法