Visual C ++ 2012ライブラリで見つかったエラー

プログラムのエラーを検出する方法の1つは、静的コード分析です。 この方法論の人気が高まっていることを嬉しく思います。 これは、静的分析が多くの機能の1つであるVisual Studioによって大幅に促進されます。 この機能は簡単に試して、定期的に使用を開始できます。 静的コード分析が好きだと気づいた人は、C / C ++ / C ++ 11言語用のプロフェッショナルなPVS-Studioアナライザーを提供できることを嬉しく思います。

はじめに

Visual Studio開発環境では、静的コード分析が可能です。 この分析は非常に便利で使いやすいです。 ただし、Visual Studioは膨大な数の機能を実行することを理解しておく必要があります。 これは、1つの機能が常に専用のツールを失うことを意味します。 リファクタリングおよびカラーリング機能は、Visual Assistと比較して失われます。 統合された画像編集の機能は、Adobe PhotoshopやCorelDRAWよりも当然悪いものです。 状況は、静的コード分析の機能と似ています。



私たちは理論化しません。 練習に移りましょう。 Visual Studio 2012フォルダーにある興味深いPVS-Studioアナライザーの表示を見てみましょう。



実際、Visual Studioに含まれているソースファイルを確認する予定はありませんでした。 すべてが偶然に起こりました。 Visual Studio 2012では、新しいC ++ 11言語標準のサポートにより、多くのヘッダーファイルが変更されました。 タスクは、PVS-Studioアナライザーに含まれるヘッダーファイルを処理できることを確認することでした。



思いがけないことに、header * .hファイルでいくつかのエラーに気づきました。 Visual Studio 2012に含まれるファイルをさらに詳しく調べることにしました。つまり、次のようなフォルダーです。

• プログラムファイル（x86）\ Microsoft Visual Studio 11.0 \ VC \ include
• プログラムファイル（x86）\ Microsoft Visual Studio 11.0 \ VC \ crt
• プログラムファイル（x86）\ Microsoft Visual Studio 11.0 \ VC \ atlmfc

ライブラリを構築するためのプロジェクトまたはメイクファイルがないため、完全なチェックは機能しませんでした。 そのため、ライブラリコードのわずかな部分のみをチェックすることができました。 検証が不完全であっても、結果は非常に興味深いものです。



Visual C ++のライブラリ内でPVS-Studioアナライザーが見つけたものを知りましょう。 ご覧のとおり、これらのエラーはすべて、Visual C ++自体に組み込まれたアナライザーを介してリークされました。

見つかった疑わしいスポットの一部

以下のすべてのコードスニペットにエラーが含まれているとは主張しません。 PVS-Studioアナライザーによって提案されたリストから、最も疑わしいと思われる場所を選択しました。

奇妙なサイクル

この疑わしいコードが最初に見つかりました。 彼は研究の継続の推進力を務めた。

template <class T> class ATL_NO_VTABLE CUtlProps : public CUtlPropsBase { .... HRESULT GetIndexOfPropertyInSet(....) { .... for(ULONG ul=0; ul<m_pUPropSet[*piCurSet].cUPropInfo; ul++) { if( dwPropertyId == pUPropInfo[ul].dwPropId ) *piCurPropId = ul; return S_OK; } return S_FALSE; } .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V612ループ内の無条件の「戻り」。 atldb.h 4829



ループの本体は1回だけ実行されます。 エラーは説明を必要としません。 ほとんどの場合、目的の値が見つかったら「return」ステートメントを呼び出す必要があります。 この場合、コードは次のようになります。

 for(ULONG ul=0; ul<m_pUPropSet[*piCurSet].cUPropInfo; ul++) { if( dwPropertyId == pUPropInfo[ul].dwPropId ) { *piCurPropId = ul; return S_OK; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

疑わしい投影

読みにくい例に謝ります。 三項演算子の状態に注意してください。

 // TEMPLATE FUNCTION proj _TMPLT(_Ty) inline _CMPLX(_Ty) proj(const _CMPLX(_Ty)& _Left) { // return complex projection return (_CMPLX(_Ty)( _CTR(_Ty)::_Isinf(real(_Left)) || _CTR(_Ty)::_Isinf(real(_Left)) ? _CTR(_Ty)::_Infv(real(_Left)) : real(_Left), imag(_Left) < 0 ? -(_Ty)0 : (_Ty)0)); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V501 「||」の左側と右側に同一のサブ式「_Ctraits <_Ty> :: _ Isinf（real（_Left））」があります 演算子。 xcomplex 780



この条件では、式「_CTR（_Ty）:: _ Isinf（real（_Left））」が2回繰り返されます。 ここにエラーがあるかどうか、そしてコードを修正する方法を言うのは難しいです。 ただし、この機能は明らかに注目に値します。

不要なチェック

 template<typename BaseType, bool t_bMFCDLL = false> class CSimpleStringT { .... void Append(_In_reads_(nLength) PCXSTR pszSrc, _In_ int nLength) { .... UINT nOldLength = GetLength(); if (nOldLength < 0) { // protects from underflow nOldLength = 0; } .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V547式 'nOldLength <0'は常にfalseです。 符号なしの型の値が<0になることはありませんatlsimpstr.h 420



ここに間違いはありません。 コードから判断すると、行の長さが負になることはありません。 CSimpleStringTクラスには、対応するチェックがあります。 変数nOldLengthが符号なしの型であるという事実は、何も破壊しません。 とにかく、長さは常に正です。 これは単なる冗長コードです。

誤ったライン形成

 template <class T> class CHtmlEditCtrlBase { .... HRESULT SetDefaultComposeSettings( LPCSTR szFontName=NULL, .....) const { CString strBuffer; .... strBuffer.Format(_T("%d,%d,%d,%d,%s,%s,%s"), bBold ? 1 : 0, bItalic ? 1 : 0, bUnderline ? 1 : 0, nFontSize, szFontColor, szBgColor, szFontName); .... } };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V576形式が正しくありません 。 'Format'関数の8番目の実引数を確認することを検討してください。 wchar_t型シンボルの文字列へのポインターが必要です。 afxhtml.h 826



このコードは、UNICODEプログラムで誤ったメッセージを生成します。 'Format（）'関数は、8番目の引数がLPCTSTR型であると想定しています。 ただし、変数 'szFontName'は常にLPCSTR型です。

マイナスポート

 typedef WORD ATL_URL_PORT; class CUrl { ATL_URL_PORT m_nPortNumber; .... inline BOOL Parse(_In_z_ LPCTSTR lpszUrl) { .... //get the port number m_nPortNumber = (ATL_URL_PORT) _ttoi(tmpBuf); if (m_nPortNumber < 0) goto error; .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V547式 'm_nPortNumber <0'は常にfalseです。 符号なしの型の値が<0になることはありませんatlutil.h 2775



ポート番号がゼロより小さいことを確認しても機能しません。 変数「m_nPortNumber」の符号なしタイプは「WORD」です。 タイプ「WORD」は「unsigned short」です。

未定義の動作

次のマクロは、Visual C ++ヘッダーファイルにあります。

 #define DXVABitMask(__n) (~((~0) << __n))
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

どこで使用されても、あいまいな動作が発生します。 もちろん、Visual C ++開発者は、この構造が安全かどうかをよく知っています。 おそらく、Visual C ++は常に負の数のシフトを同じ方法で処理するという自信があります。 正式には、負の数をシフトすると、未定義の動作が発生します。 このトピックの詳細については、「 フォードを知らない、水に入らないでください。パート3 」を参照してください。

64ビットモードでの誤った操作

この64ビットエラーパターンについては、64ビットC / C ++アプリケーションの開発に関するレッスンで詳しく説明します。 間違いが何であるかを理解するために、 レッスン12に慣れることをお勧めします。

 class CWnd : public CCmdTarget { .... virtual void WinHelp(DWORD_PTR dwData, UINT nCmd = HELP_CONTEXT); .... }; class CFrameWnd : public CWnd { .... }; class CFrameWndEx : public CFrameWnd { .... virtual void WinHelp(DWORD dwData, UINT nCmd = HELP_CONTEXT); .... };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V301予期しない関数のオーバーロード動作。 派生クラス「CFrameWndEx」および基本クラス「CFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxframewndex.h 154



「WinHelp」関数が「CFrameWndEx」クラスで誤って宣言されています。 最初の引数のタイプは「DWORD_PTR」でなければなりません。 同様のエラーは、他のいくつかのクラスでも見られます。

• V301予期しない関数のオーバーロード動作。 派生クラス「CMDIFrameWndEx」および基本クラス「CFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxmdiframewndex.h 237
• V301予期しない関数のオーバーロード動作。 派生クラス「CMDIFrameWndEx」および基本クラス「CMDIFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxmdiframewndex.h 237
• V301予期しない関数のオーバーロード動作。 派生クラス「COleIPFrameWndEx」および基本クラス「CFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxoleipframewndex.h 130
• V301予期しない関数のオーバーロード動作。 派生クラス「COleIPFrameWndEx」および基本クラス「COleIPFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxoleipframewndex.h 130
• V301予期しない関数のオーバーロード動作。 派生クラス「COleDocIPFrameWndEx」および基本クラス「CFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxoledocipframewndex.h 129
• V301予期しない関数のオーバーロード動作。 派生クラス「COleDocIPFrameWndEx」および基本クラス「COleIPFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxoledocipframewndex.h 129
• V301予期しない関数のオーバーロード動作。 派生クラス「COleDocIPFrameWndEx」および基本クラス「COleDocIPFrameWnd」の関数「WinHelpW」の最初の引数を参照してください。 afxoledocipframewndex.h 129

先頭のポインターが使用され、NULLと比較されます

そのような場所はかなり多く見つかりました。 特定の各ケースが危険であるかどうかを分析することは非常に疲れます。 ライブラリの作成者は、これではるかにうまくいくでしょう。 いくつかの例を挙げます。

 BOOL CDockablePane::PreTranslateMessage(MSG* pMsg) { .... CBaseTabbedPane* pParentBar = GetParentTabbedPane(); CPaneFrameWnd* pParentMiniFrame = pParentBar->GetParentMiniFrame(); if (pParentBar != NULL && (pParentBar->IsTracked() || pParentMiniFrame != NULL && pParentMiniFrame->IsCaptured() ) ) .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V595 nullptrに対して検証される前に、「pParentBar」ポインターが使用されました。 行を確認してください：2840、2841。afxdockablepane.cpp 2840



最初に、「pParentBar」ポインターを使用してGetParentMiniFrame（）関数を呼び出します。 ゼータは、このポインターがNULLである可能性があると突然疑われ、対応するチェックが実行されます。

 AFX_CS_STATUS CDockingManager::DeterminePaneAndStatus(....) { .... CDockablePane* pDockingBar = DYNAMIC_DOWNCAST(CDockablePane, *ppTargetBar); if (!pDockingBar->IsFloating() && (pDockingBar->GetCurrentAlignment() & dwEnabledAlignment) == 0) { return CS_NOTHING; } if (pDockingBar != NULL) { return pDockingBar->GetDockingStatus( pt, nSensitivity); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V595 nullptrに対して検証される前に、「pDockingBar」ポインターが使用されました。 行を確認してください：582、587。afxdockingmanager.cpp 582



最初は、ポインター「pDockingBar」がアクティブに使用され、その後突然NULLと比較されます。



最後の例：

 void CFrameImpl::AddDefaultButtonsToCustomizePane(....) { .... for (POSITION posCurr = lstOrigButtons.GetHeadPosition(); posCurr != NULL; i++) { CMFCToolBarButton* pButtonCurr = (CMFCToolBarButton*)lstOrigButtons.GetNext(posCurr); UINT uiID = pButtonCurr->m_nID; if ((pButtonCurr == NULL) || (pButtonCurr->m_nStyle & TBBS_SEPARATOR) || (....) { continue; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V595 nullptrに対して検証される前に、「pButtonCurr」ポインターが使用されました。 行を確認してください：1412、1414。afxframeimpl.cpp 1412



クラス「m_nID」のメンバーに気軽に連絡してください。 そして、条件 'pButtonCurr'が0になる可能性があることがわかります。

破壊されたオブジェクトを使用する

 CString m_strBrowseFolderTitle; void CMFCEditBrowseCtrl::OnBrowse() { .... LPCTSTR lpszTitle = m_strBrowseFolderTitle != _T("") ? m_strBrowseFolderTitle : (LPCTSTR)NULL; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V623 「？：」演算子の検査を検討してください。 一時オブジェクトが作成され、その後破棄されます。 afxeditbrowsectrl.cpp 308



三項演算子は、異なる型の値を返すことはできません。 したがって、タイプCStringのオブジェクトは、「（LPCTSTR）NULL」から暗黙的に作成されます。 次に、この空の文字列から暗黙的にバッファへのポインタが取得されます。 問題は、CStringのような一時オブジェクトが破壊されることです。 その結果、「lpszTitle」ポインターの値が無効になり、操作できなくなります。 ここでは、このエラーパターンの詳細な説明を読むことができます。

時間の問題

 UINT CMFCPopupMenuBar::m_uiPopupTimerDelay = (UINT) -1; .... void CMFCPopupMenuBar::OnChangeHot(int iHot) { .... SetTimer(AFX_TIMER_ID_MENUBAR_REMOVE, max(0, m_uiPopupTimerDelay - 1), NULL); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V547式 '（0）>（m_uiPopupTimerDelay-1）'は常にfalseです。 符号なしの型の値が<0になることはありません。afxpopupmenubar.cpp 968



値 '-1'は特別なフラグとして使用されます。 「max」マクロを使用して、プログラマはm_uiPopupTimerDelay変数の負の値から保護することを望んでいました。 変数には符号なしの型があるため、これは機能しません。 常にゼロ以上です。 正しいコードは次のようになります。

 SetTimer(AFX_TIMER_ID_MENUBAR_REMOVE, m_uiPopupTimerDelay == (UINT)-1 ? 0 : m_uiPopupTimerDelay - 1, NULL);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

同様のエラーはこちらです：

• V547式 '（0）>（m_uiPopupTimerDelay-1）'は常にfalseです。 符号なしの型の値が<0になることはありません。afxribbonpanelmenu.cpp 880

無意味な線

 BOOL CMFCTasksPaneTask::SetACCData(CWnd* pParent, CAccessibilityData& data) { .... data.m_nAccHit = 1; data.m_strAccDefAction = _T("Press"); data.m_rectAccLocation = m_rect; pParent->ClientToScreen(&data.m_rectAccLocation); data.m_ptAccHit; return TRUE; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V607所有者のない式「data.m_ptAccHit」。 afxtaskspane.cpp 96



「data.m_ptAccHit;」とは何ですか？ おそらくここで、彼らはこの変数に値を割り当てるのを忘れていたのでしょうか？

別の0が必要な場合があります

 BOOL CMFCTasksPane::GetMRUFileName(....) { .... const int MAX_NAME_LEN = 512; TCHAR lpcszBuffer [MAX_NAME_LEN + 1]; memset(lpcszBuffer, 0, MAX_NAME_LEN * sizeof(TCHAR)); if (GetFileTitle((*pRecentFileList)[nIndex], lpcszBuffer, MAX_NAME_LEN) == 0) { strName = lpcszBuffer; return TRUE; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V512 「memset」関数の呼び出しにより、バッファー「lpcszBuffer」のアンダーフローが発生します。 afxtaskspane.cpp 2626



このコードは、終端がゼロではない文字列を返す可能性があるという疑いがあります。 おそらく、配列の最後の要素もリセットする必要があります。

 memset(lpcszBuffer, 0, (MAX_NAME_LEN + 1) * sizeof(TCHAR));
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

奇妙な「if」

 void CMFCVisualManagerOfficeXP::OnDrawBarGripper(....) { .... if (bHorz) { rectFill.DeflateRect(4, 0); } else { rectFill.DeflateRect(4, 0); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V523 「then」ステートメントは「else」ステートメントと同等です。 afxvisualmanagerofficexp.cpp 264

危険なクラスsingle_link_registry

クラス「single_link_registry」を使用すると、すべての例外を正しく処理した場合でも、アプリケーションが予期せず終了する場合があります。 クラス「single_link_registry」のデストラクタを見てみましょう。

 virtual ~single_link_registry() { // It is an error to delete link registry with links // still present if (count() != 0) { throw invalid_operation( "Deleting link registry before removing all the links"); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V509デストラクタ内の「スロー」演算子は、try..catchブロック内に配置する必要があります。 デストラクタ内で例外を発生させることは違法です。 agents.h 759



このデストラクタは例外をスローする場合があります。 これは悪い考えです。 プログラムで例外が発生すると、デストラクタの呼び出しによるオブジェクトの破壊が始まります。 「single_link_registry」デストラクタでエラーが発生すると、別の例外がスローされます。 デストラクタでは処理されません。 その結果、C ++ライブラリは、terminate（）関数を呼び出してプログラムを直ちにクラッシュさせます。



同様の悪いデストラクタ：

• V509デストラクタ内の「スロー」演算子は、try..catchブロック内に配置する必要があります。 デストラクタ内で例外を発生させることは違法です。 concrt.h 4747
• V509デストラクタ内の「スロー」演算子は、try..catchブロック内に配置する必要があります。 デストラクタ内で例外を発生させることは違法です。 agents.h 934
• V509デストラクタ内の「スロー」演算子は、try..catchブロック内に配置する必要があります。 デストラクタ内で例外を発生させることは違法です。 taskcollection.cpp 880

別の奇妙なループ

 void CPreviewView::OnPreviewClose() { .... while (m_pToolBar && m_pToolBar->m_pInPlaceOwner) { COleIPFrameWnd *pInPlaceFrame = DYNAMIC_DOWNCAST(COleIPFrameWnd, pParent); if (!pInPlaceFrame) break; CDocument *pViewDoc = GetDocument(); if (!pViewDoc) break; // in place items must have a server document. COleServerDoc *pDoc = DYNAMIC_DOWNCAST(COleServerDoc, pViewDoc); if (!pDoc) break; // destroy our toolbar m_pToolBar->DestroyWindow(); m_pToolBar = NULL; pInPlaceFrame->SetPreviewMode(FALSE); // restore toolbars pDoc->OnDocWindowActivate(TRUE); break; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V612ループ内の無条件の「ブレーク」。 viewprev.cpp 476



ループには「継続」ステートメントはありません。 ループの最後は「break」です。 これは非常に奇妙です。 サイクルは常に1回だけ実行されます。 ここにエラーがあるか、「while」を「if」に置き換える方が良いでしょう。

奇妙な定数

リストには興味深いものではないコードに関する他のマイナーなコメントがあります。 何が危機にclearしているのかが明確になるように、1つの例を挙げます。



afxdrawmanager.cppファイルでは、何らかの理由で、Pi番号に定数が設定されています。

 const double AFX_PI = 3.1415926;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

V624定数3.1415926が使用されています。 結果の値は不正確になる可能性があります。 <math.h>のM_PI定数の使用を検討してください。 afxdrawmanager.cpp 22



これは間違いではなく、定数の精度で十分です。 しかし、M_PI定数を使用しない理由は明確ではありません。M_PI定数ははるかに正確に設定されています。

 #define M_PI 3.14159265358979323846
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Visual C ++開発者の呼び出し

残念ながら、Visual C ++の一部であるライブラリを構築するためのプロジェクトまたはメイクファイルはありません。 したがって、分析は非常に表面的です。 何かを見つけて、それについて書きました。 他に疑わしい場所がないとは思わないでください:)。



PVS-Studioを使用してライブラリをチェックする方がはるかに便利であると確信しています。 必要に応じて、メイクファイルへの統合を促し、支援する準備ができています。 また、何が間違いで何がそうでないかを理解するのも簡単です。

結論

Visual Studio 2012では、C / C ++コードの静的分析があります。 しかし、これはこれで十分という意味ではありません。 これは最初のステップにすぎません。 これは、コードの品質を改善するための新しいテクノロジーを簡単かつ安価に試すチャンスです。 そして、気に入ったら、私たちのところに来てPVS-Studioを購入してください。 このツールは、欠陥とより集中的に戦います。 彼はこのために投獄されています。 私たちはそれでお金を稼ぎます。つまり、非常に積極的に開発しています。



Visual C ++ライブラリでエラーが見つかりましたが、静的分析があります。 Clangコンパイラにはバグが見つかりましたが、静的分析があります。 私たちを取得し、私たちはあなたのプロジェクトのエラーを定期的に見つけます。 Visual Studio 2005、2008、2010、2012には優れたインターンがあり、バックグラウンドでエラーを探すことができます。