VirtualDubチェック

ちょうど今、私は座ってPVS-Studioを使用してVirtualDubプロジェクトをチェックしました。 選択はランダムでした。 最も重要なことは、PVS-Studioコードアナライザーの開発状況を示すために、さまざまなプロジェクトを定期的にチェック/ダブルチェックすることだと思います。 そして、どのプロジェクトをテストするかはそれほど重要ではありません。 間違いはどこにでもあります。 2011年にVirtualDubプロジェクトを既にテストしましたが、興味深いものはほとんど見つかりませんでした。 だから、2年後に物事がどうなるかを見ることにしました。







VirtualDub WebサイトからVirtualDub-1.10.3-src.7zアーカイブをダウンロードしました。 分析には、 PVS-Studioバージョン5.10を使用しました。 分析には約1時間かかりましたので、厳密に判断しないでください。 確かに、私は何かを見逃した。 そしてその逆に、正しいコードは疑わしいと考えることができます。 VirtualDubプロジェクトをサポートする人には、私のレポートに頼らずに、独立したチェックを行うようお願いします。 私たちは常にオープンソースコミュニティに会いに行き 、登録キーを割り当てる準備ができています。



また、エイブリィ・リーにこの記事を理解して扱うようお願いしたいです。 前回、彼は私の記事の1つでVirtualDubについて非常に痛々しいほど言及しました。 私はまったく望んでいませんでしたし、いくつかのプログラムがバグだと言いたくはありません。 ソフトウェアのバグはどこにでもあります。 私の目標は、静的コード分析が提供できる利点を示すことです。 これにより、オープンソースプロジェクトの信頼性が少し向上します。 これは素晴らしい。



もちろん、1回限りのチェックは無効です。 しかし、残念ながら、私はそれについて何もできません。 静的分析ツールを定期的に使用するかどうかは、開発者次第です。 私は、通常の使用の利点が何であるかを説明しようとするだけです。 このトピックに関する興味深いメモの1つは、 Leo Tolstoyと静的コード分析です。



ただし、この記事は静的分析を使用するための方法論ではなく、エラーに関するものです。 VirtualDubで見つかった新しい興味深いPVS-Studioアナライザーを見てみましょう。

仮想デストラクタ

C ++プログラミング言語では、ポリモーフィックベースクラスのデストラクタを仮想として宣言する必要があります。 これは、対応する基本クラスへのポインターを介して、派生クラスのオブジェクトの正しい破棄を保証する唯一の方法です。



私は誰もがそれを知っていることを知っています。 ただし、これはデストラクタ仮想の宣言を忘れることを止めません。



VirtualDubにはVDDialogBaseW32クラスがあります。

class VDDialogBaseW32 { .... ~VDDialogBaseW32(); .... virtual INT_PTR DlgProc(....) = 0; virtual bool PreNCDestroy(); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ご覧のとおり、仮想関数が含まれています。 ただし、デストラクタは仮想として宣言されていません。 当然、彼のクラスは彼から継承されます。 たとえば、VDDialogAudioFilterFormatConvConfig：

 class VDDialogAudioFilterFormatConvConfig : public VDDialogBaseW32 { .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

オブジェクト破棄エラーは次のようになります。

 INT_PTR CALLBACK VDDialogBaseW32::StaticDlgProc(....) { VDDialogBaseW32 *pThis = (VDDialogBaseW32 *)GetWindowLongPtr(hwnd, DWLP_USER); .... delete pThis; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する警告：V599「VDDialogBaseW32」クラスには仮想関数が含まれていますが、デストラクタは仮想デストラクタとして宣言されていません。 VirtualDub gui.cpp 997



ご覧のとおり、基本クラスへのポインターを使用してオブジェクトを破棄します。 このようなオブジェクトの削除は、未定義のプログラムの動作につながります。



VDMPEGAudioPolyphaseFilterクラスにも同様の不幸があります。

あいまいな動作の詳細

仮想デストラクタに関連するエラーに関する質問はありません。 より滑りやすいトピックはシフト操作です。 以下にそのような例を示します。

 void AVIVideoGIFOutputStream::write(....) { { .... for(int i=0; i<palsize; ++i) dict[i].mPrevAndLastChar = (-1 << 16) + i; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これは、10年以上にわたって正常に使用されている完全に信頼できるコードであると、好きなだけ言うことができます。 ただし、とにかく、ここではプログラムの不定の動作を扱っています。 このような構造について規格が言っていることは次のとおりです。



シフト演算子<<および>>は、左から右にグループ化します。



シフト式<<加算式



シフト式>>加算式



オペランドは整数またはスコープなしの列挙型である必要があり、整数のプロモーションが実行されます。



1.結果の型は、昇格した左オペランドの型です。 右のオペランドが負の場合、または昇格した左のオペランドのビット長以上の場合、動作は未定義です。



2. E1 << E2の値は、E1を左にシフトしたE2ビット位置です。 空きビットはゼロで埋められます。 E1に符号なしの型がある場合、結果の値はE1 * 2 ^ E2で、結果の型で表現可能な最大値よりも1多いモジュロです。 それ以外の場合、E1に符号付きの型と負でない値があり、E1 * 2 ^ E2が結果の型で表現できる場合、それが結果の値です。 それ以外の場合、動作は未定義です。



3. E1 >> E2の値は、E1を右シフトしたE2ビット位置です。 E1に符号なしの型がある場合、またはE1に符号付きの型と負でない値がある場合、結果の値はE1 / 2 ^ E2の商の整数部分です 。 E1に符号付きタイプと負の値がある場合、結果の値は実装定義です。



コードが機能するのは運です。 最適化のために新しいコンパイラまたは他のキーをマスターするとき、プログラムはその動作を予期せず変更する場合があります。 シフトの詳細と、コードを編集する価値があるかどうかについては、「 フォードを知らないで、水に入らないでください。パート3 」の記事を参照してください 。



以下は、PVS-StudioアナライザーがVirtualDubプロジェクトで未定義の動作または未指定の動作を検出した場所の完全なリストです 。

タイプミス

 static ModuleInfo *CrashGetModules(void *&ptr) { .... while(*pszHeap++); if (pszHeap[-1]=='.') period = pszHeap-1; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio診断メッセージ：V529奇数のセミコロン ';' 「while」演算子の後。 VirtualDub crash.cpp 462



「while」の後のセミコロンに注意してください。 ここでは、エラーまたはコードの形式が正しくありません。 これはまさに間違いだと思います。 ループ「while（* pszHeap ++）;」は行末に移動し、その結果、変数「pszHeap」は端末ゼロの後にメモリを指します 。 「if（pszHeap [-1] == '。'）」をチェックしても意味がありません。 アドレスpszHeap [-1]には常に終端ゼロがあります。



文字列を扱うときは、別のタイプミスを考慮してください。

 void VDBackfaceService::Execute(...., char *s) { .... if (*s == '"') { while(*s && *s != '"') ++s; } else { .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V637 2つの正反対の条件が発生しました。 2番目の条件は常にfalseです。 行を確認：183、184。VirtualDub backface.cpp 183



このコードは、引用符内のすべてをスキップする必要があります。 少なくともそういう風に思えます。 ただし、条件（* s && * s！= '"'）はすぐにfalseです。おそらく、コードは次のようになっているはずです。

 if (*s == '"') { ++s; while(*s && *s != '"') ++s; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

new演算子は、メモリの割り当てに失敗すると例外をスローします。

古いコードでは、多くの場合、新しい演算子が返されたかどうかのチェックに遭遇します。 次のようになります。

 int *p = new int[10]; if (!p) return false;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

C ++言語標準をサポートする最新のコンパイラは、メモリ割り当てが失敗した場合に例外をスローする必要があります。 「new」演算子が例外をスローしないようにできますが、現在検討中のケースとは関係ありません。



したがって、（！P）が不要であるかどうかを確認します。 一般に、このようなコードは無害です。 追加のチェック。 心配する必要はありません。

ただし、古いコードは不快な結果を招く可能性があります。 VirtualDubプロジェクトからのフラグメントを検討してください。

 void HexEditor::Find(HWND hwndParent) { .... int *next = new int[nFindLength+1]; char *searchbuffer = new char[65536]; char *revstring = new char[nFindLength]; .... if (!next || !searchbuffer || !revstring) { delete[] next; delete[] searchbuffer; delete[] revstring; return; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PV-Studioが発行する診断メッセージ：V668メモリが「新しい」演算子を使用して割り当てられたため、「次の」ポインターをnullに対してテストしても意味がありません。 メモリ割り当てエラーの場合、例外が生成されます。 VirtualDub hexviewer.cpp 2012



文字列「char * revstring = new char [nFindLength];」で例外が発生すると、メモリリークが発生します。 delete []ステートメントは呼び出されません。 重大な間違いではありませんが、それについて言及する価値はあります。



ここでは、VirtualDubで「new」演算子を呼び出した後にポインターがチェックされるすべての場所がリストされます。

破壊されたオブジェクトへのリンク

 vdlist_iterator& operator--(int) { vdlist_iterator tmp(*this); mp = mp->mListNodePrev; return tmp; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V558関数は、一時ローカルオブジェクトへの参照を返します：tmp。 VirtualDub vdstl.h 460



関数が正しく実装されていません。 ローカルの「tmp」オブジェクトへの参照を返します。 関数を終了すると、すでに破棄されます。 このリンクを使用すると、未定義の動作が発生します。



ところで、近くにある++演算子は正しく実装されています。

最初に使用し、次にチェックします

さまざまなプログラムでは、ポインターが最初に間接参照されたときにエラーが発生することが多く、その場合のみNULLと比較されます。 NULLポインターの等価性は偶発的なまれな状況であるため、このようなエラーは非常に長い間現れません。 これらの欠点はVirtualDubコードに存在します。 例：

 LRESULT YUVCodec::DecompressGetFormat(BITMAPINFO *lpbiInput, BITMAPINFO *lpbiOutput) { BITMAPINFOHEADER *bmihInput = &lpbiInput->bmiHeader; BITMAPINFOHEADER *bmihOutput = &lpbiOutput->bmiHeader; LRESULT res; if (!lpbiOutput) return sizeof(BITMAPINFOHEADER); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio診断メッセージ：V595 nullptrに対して検証される前に、「lpbiOutput」ポインターが使用されました。 行を確認してください：82、85。VirtualDub yuvcodec.cpp 82



最初に、lpbiOutputポインターが逆参照されます。 次に、「if（！LpbiOutput）」がチェックされます。 このようなエラーは通常、リファクタリング中に発生します。 新しいコードは、必要なチェックの前に挿入されます。 上記のコードを修正するには、アクションのシーケンスを変更する必要があります。

 LRESULT YUVCodec::DecompressGetFormat(BITMAPINFO *lpbiInput, BITMAPINFO *lpbiOutput) { if (!lpbiOutput) return sizeof(BITMAPINFOHEADER); BITMAPINFOHEADER *bmihInput = &lpbiInput->bmiHeader; BITMAPINFOHEADER *bmihOutput = &lpbiOutput->bmiHeader; LRESULT res; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

アナライザーがV595警告を発行する他の場所は、 ここにリストされています 。

HRESULTタイプを使用する

 VDPosition AVIReadTunnelStream::TimeToPosition(VDTime timeInUs) { AVISTREAMINFO asi; if (AVIStreamInfo(pas, &asi, sizeof asi)) return 0; return VDRoundToInt64(timeInUs * (double)asi.dwRate / (double)asi.dwScale * (1.0 / 1000000.0)); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V545このような「if」演算子の条件式は、HRESULT型の値「AVIStreamInfoA（pas、＆asi、sizeof asi）」に対して正しくありません。 代わりにSUCCEEDEDまたはFAILEDマクロを使用する必要があります。 VirtualDub avireadhandlertunnelw32.cpp 230



AVIStreamInfo（）関数は、HRESULT型の値を返します。 このタイプは「bool」として解釈できません。 HRESULT型の変数に格納されている情報は、かなり複雑な構造を持っています。 HRESULT値を確認するには、「WinError.h」で宣言されているSUCCEEDEDまたはFAILEDマクロを使用する必要があります。 これらのマクロの仕組みは次のとおりです。

 #define FAILED(hr) (((HRESULT)(hr)) < 0) #define SUCCEEDED(hr) (((HRESULT)(hr)) >= 0)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

正しいコードは次のとおりです。

 if (FAILED(AVIStreamInfo(pas, &asi, sizeof asi)))
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioは、次の行にも同様の警告を表示します。

• avireadhandlertunnelw32.cpp 238
• avireadhandlertunnelw32.cpp 335
• inputfileavi.cpp 440
• context_d3d11.cpp 959

マジックナンバー

数字を使用して文字列の長さを設定することはお勧めできません。 文字を数えるときに間違いを犯すのは非常に簡単です。 例：

 bool VDOpenGLBinding::Attach(....) { .... if (!memcmp(start, "GL_EXT_blend_subtract", 20)) .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V512「memcmp」関数の呼び出しは、バッファー「GL_EXT_blend_subtract」のアンダーフローを引き起こします。 リザopengl.cpp 393



文字列「GL_EXT_blend_subtract」の長さは20文字ではなく、21文字です。 エラーは重大ではありません。 実際には、衝突は発生しません。 ただし、このようなマジックナンバーは避けてください。 文字列の長さをカウントするには、特別なマクロを使用することをお勧めします。 例：

 #define LiteralStrLen(S) (sizeof(S) / sizeof(S[0]) - 1)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

C ++では、より安全なテンプレート関数を作成できます。

 template <typename T, size_t N> char (&ArraySizeHelper(T (&array)[N]))[N]; template <typename T, size_t N> size_t LiteralStrLen(T (&array)[N]) { return sizeof(ArraySizeHelper(array)) - 1; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

2番目のオプションの利点は、単純なポインターを誤って引数として渡すことができないことです。 この手法については、記事「 PVS-Studio vs Chromium 」で詳しく説明されています。

絶対パス

 VDDbgHelpDynamicLoaderW32::VDDbgHelpDynamicLoaderW32() { hmodDbgHelp = LoadLibrary( "c:\\program files\\debugging tools for windows\\dbghelp"); if (!hmodDbgHelp) { hmodDbgHelp = LoadLibrary("c:\\program files (x86)\\...... .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V631「LoadLibraryA」関数呼び出しの検査を検討してください。 ファイルまたはディレクトリへの絶対パスを定義することは、スタイルが悪いと見なされます。 VirtualDub leaks.cpp 67、69



このコードが悪い理由は明らかだと思います。 もちろん、コードはデバッグに関連付けられており、エンドユーザーに何らかの形で悪影響を与えることはほとんどありません。 しかし、とにかく、Program Filesへの正しいパスを取得する方が良いでしょう。

無効な引数

 sint64 rva; void tool_lookup(....) { .... printf("%08I64x %s + %x [%s:%d]\n", addr, sym->name, addr-sym->rva, fn, line); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V576 Incorrect format。 'printf'関数の4番目の実引数を確認することを検討してください。 引数は32ビット以下であると予想されます。 あすかlookup.cpp 56



変数 'rva'は64ビット型です。 これは、8バイトがスタックにプッシュされることを意味します。 printf（）関数は、 可変個の引数を持つ関数です。 処理するデータのタイプは、フォーマット文字列を使用して指定されます。 この場合、変数 'rva'は32ビット変数（ "％x"）として処理されます。



このエラーが失敗につながるかどうかは、コンパイラが引数の転送を整理する方法とプラットフォームのビット深度に依存します。 たとえば、 Win64では、すべての整数型が最初に64ビット型にキャストされてから、スタックにプッシュされます。 変数が必要以上にスタック上のスペースを占有するという問題はそうではありません。



ただし、変数 'rva'がINT_MAXより大きい値を格納する場合、その値はとにかく正しく印刷されません。



アナライザーは同様の警告をここに表示します：

• dubstatus.cpp 360
• lookup.cpp 58

間違った比較

 void VDVideoCompressorVCM::GetState(vdfastvector<uint8>& data) { DWORD res; .... res = ICGetState(hic, data.data(), size); .... if (res < 0) throw MyICError("Video compression", res); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V547式 'res <0'は常にfalseです。 符号なしの型の値が0未満になることはありません。Rizaw32videocodecpack.cpp 828



変数 'res'には、署名されていないDWORD型があります。 これは、式「res <0」が常に「false」に等しいことを意味します。



同様のチェックはこちらです：w32videocodec.cpp 284。



別の同様のエラーを検討してください。

 #define ICERR_CUSTOM -400L static const char *GetVCMErrorString(uint32 icErr) { .... if (icErr <= ICERR_CUSTOM) err = "A codec-specific error occurred."; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V605式icErr <=-400Lの検証を検討してください。 符号なしの値は、数値-400と比較されます。 システムerror_win32.cpp 54



変数「icErr」のタイプは「unsigned」です。 したがって、比較の前に、数値「-400」は暗黙的に「符号なし」に変換されます。 値「-400」は4294966896になります。したがって、比較（icErr <= -400）は（icErr <= 4294966896）と同等です。 どうやら、これはプログラマが望んでいたことではありません。

その他の奇妙な

 void AVIOutputFile::finalize() { .... if (stream.mChunkCount && hdr.dwScale && stream.mChunkCount) .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V501「&&」演算子の左右に同じサブ式「stream.mChunkCount」があります。 VirtualDub avioutputfile.cpp 761



変数 'stream.mChunkCount'は2回チェックされます。 1つのチェックが不要であるか、他のチェックを忘れました。

 void VDVideoCompressorVCM::Start(const void *inputFormat, uint32 inputFormatSize, const void *outputFormat, uint32 outputFormatSize, const VDFraction& frameRate, VDPosition frameCount) { this->hic = hic; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V570「this-> hic」変数はそれ自体に割り当てられます。 リザw32videocodecpack.cpp 253

 void VDDialogAudioConversionW32::RecomputeBandwidth() { .... if (IsDlgButtonChecked(mhdlg, IDC_PRECISION_NOCHANGE)) { if (mbSourcePrecisionKnown && mbSource16Bit) bps *= 2; else bps = 0; } if (IsDlgButtonChecked(mhdlg, IDC_PRECISION_16BIT)) bps *= 2; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioによって発行される診断メッセージ：V646アプリケーションのロジックの検査を検討してください。 「else」キーワードが欠落している可能性があります。 VirtualDub optdlg.cpp 120



コードが正しくフォーマットされていない可能性があります。 または、キーワード「else」が忘れられている可能性があります。

 bool VDCaptureDriverScreen::Init(VDGUIHandle hParent) { .... mbAudioHardwarePresent = false; mbAudioHardwarePresent = true; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioが発行する診断メッセージ：V519 'mbAudioHardwarePresent'変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認してください：274、275。VDCapture cap_screen.cpp 275

おわりに

ご覧のように、一度だけ実行したとしても、静的分析は有用です。 しかし、定期的に実行する方がはるかに便利です。 結局のところ、コンパイラの警告（警告）、プログラマーはリリース前に2回以上含まれていますが、それらは常に使用しています。 同じ状況が静的分析ツールにもあります。 常に使用することにより、エラーをすばやく修正できます。 PVS-Studioは、より興味深い警告を生成するコンパイラの一種のアドオンと見なすことができます。 最適なオプションは、 増分コード分析を使用することです。 変更されたファイルをコンパイルすると、すぐに新しいエラーが見つかります。