Microsoftプロジェクトのテストの継続：PowerShell分析

マイクロソフトにとって、ソフトウェア製品のソースコードを開くことは最近「良い伝統」になりました。 ここでは、CoreFX、.Net Compiler Platform（Roslyn）、コードコントラクト、MSBuild、およびその他のプロジェクトについて思い出すことができます。 PVS-Studio静的アナライザーの開発者である私たちにとって、これはよく知られているプロジェクトを確認し、発見されたエラーについて開発者を含む人々に伝え、アナライザーをテストする機会です。 今日は、別のMicrosoftプロジェクト-PowerShellで見つかったエラーについて説明します。

Powerhell

PowerShellはMicrosoftのクロスプラットフォームプロジェクトであり、コマンドラインインターフェイスと付属のスクリプト言語を備えたシェルで構成されています。 Windows PowerShellは、Microsoft .NET Framework上に構築され、統合されています 。 さらに、PowerShellはCOM 、 WMI、 ADSIへの便利なアクセスを提供するだけでなく、通常のコマンドラインコマンドを実行して、管理者がローカルおよびリモートシステムでさまざまなタスクを実行できる単一の環境を作成できます。



プロジェクトコードはGitHubのリポジトリからダウンロードできます 。

PVS-Studio

プロジェクトリポジトリの統計によると、コードの約93％はC＃プログラミング言語を使用して記述されています。

検証には、PVS-Studio静的コードアナライザーを使用しました。 開発中のバージョンが使用されました。 つまり、これはPVS-Studio 6.08ではなく、PVS-Studio 6.09でもありません。 このアプローチにより、アナライザの最新バージョンをテストするための幅広いアプローチが可能になり、可能であれば、見つかった欠陥を修正することができます。 もちろん、これはマルチレベルテストシステムの使用を無効にするものではありません（Linuxバージョンの開発に関する記事の 7つのテスト方法を参照）が、アナライザーをテストする別の方法として機能します。



アナライザーの現在のバージョンはここからダウンロードできます 。

分析準備

アナライザーが更新され、プロジェクトコードがロードされます。 しかし、時には、分析のためにプロジェクトを準備するプロセスで、つまりアセンブリの段階で困難が発生することがあります。 チェックする前に、プロジェクトを組み立てることをお勧めします。 なぜこれが重要なのですか？ そのため、アナライザーはより多くの情報を利用できるようになるため、より詳細な分析を行うことが可能になります。



アナライザーを使用する最も使いやすい（便利な）方法は、Visual Studio開発環境からプロジェクトをチェックすることです。 高速、簡単、便利です。 しかし、その後、不快なニュアンスが浮上しました。



判明したように、開発者自身は、Visual Studio開発環境を使用してプロジェクトをビルドすることを推奨していません。GitHubに直接記述されています。「Visual StudioからPowerShellソリューションをビルドすることはお勧めしません。」



しかし、Visual Studioでビルドしてその下からチェックする誘惑は大きすぎたので、試してみることにしました。 結果を次の図に示します。

図1. Visual Studioを使用したプロジェクトのコンパイルエラー。



不快です。 それは私にとって何を意味しましたか？ そのように、プロジェクトのアナライザーのすべての機能を明らかにすることはできません。 ここではいくつかのシナリオが可能です。



シナリオ1.組み立てられていないプロジェクトの検証。



プロジェクトを組み立てようとしました。 行きませんか？ さて、そのまま確認しましょう。



このアプローチの利点は何ですか？ アセンブリに煩わされる必要はありません。問題が何であるか、それを解決する方法、またはアセンブリされたプロジェクトをかわしてチェックする方法を理解する必要はありません。 これにより時間を節約できます。十分な長さの輸送後もプロジェクトを組み立てることができ、時間を無駄にするという保証はありません。



この決定の欠点も明らかです。 まず、これは劣った分析です。 一部のエラーはアナライザーから外れます。 おそらく、いくつかの誤検知が表示されます。 第二に、この場合、組み立てられたプロジェクトでは何らかの方法で変更される可能性があるため、偽/肯定応答の統計を提供することは意味がありません。



それにもかかわらず、このシナリオでも、十分なエラーを見つけて、それに関する記事を書くことができます。



シナリオ2.プロジェクトを理解して組み立てます。



長所と短所は上記の説明とは逆です。 はい、アセンブリにより多くの時間を費やす必要がありますが、望ましい結果が得られるという事実ではありません。 しかし、成功した場合は、ソースコードをより徹底的に検証し、他の興味深いエラーを見つけることができます。



ここで何をすべきかについて明確なアドバイスはありません。誰もが自分で決定します。



アセンブリに苦しんでいた私は、プロジェクトを「現状のまま」チェックすることにしました。 記事を書くために、このオプションはまったく受け入れられます。



ご注意 プロジェクトはVisual Studioからビルドされていないという事実にもかかわらず、ルートにあるスクリプト（ build.sh ）を介して非常に穏やかにビルドされます。



注2.開発者の1人（このことに感謝します）は、* .slnファイルは作業の利便性のために必要であるが、アセンブリには必要ではないことを示唆しました。 これは、分析シナリオの正しい選択を支持するもう1つの議論です。

分析結果

部分式の重複



警告V3001で不審なスポットが検出されないプロジェクトには、メダルを与える必要があります。 その場合、PowerShellはメダルなしで残されていたはずです。その理由は次のとおりです。

internal Version BaseMinimumVersion { get; set; } internal Version BaseMaximumVersion { get; set; } protected override void ProcessRecord() { if (BaseMaximumVersion != null && BaseMaximumVersion != null && BaseMaximumVersion < BaseMinimumVersion) { string message = StringUtil.Format( Modules.MinimumVersionAndMaximumVersionInvalidRange, BaseMinimumVersion, BaseMaximumVersion); throw new PSArgumentOutOfRangeException(message); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3001 「&&」演算子の左側と右側に同一のサブ式「BaseMaximumVersion！= Null」があります。 System.Management.Automation ImportModuleCommand.cs 1663



GitHubのコードにリンクします 。



コードスニペットからわかるように、 BaseMaximumVersionリンクはnullの不等式について2回チェックされましたが、 代わりにBaseMinimumVersionリンクをチェックする必要があることは明らかです。 状況の組み合わせが成功しているため、このエラーは長期間にわたって現れない場合があります。 ただし、エラーが発生すると、 BaseMinimumVersionリンクはnullになるため、 BaseMinimumVersionに関する情報は例外で使用されるメッセージのテキストには含まれません。 その結果、いくつかの有用な情報が失われます。



この記事を書いたときに、コードの書式設定を修正したため、エラーに気付きやすくなりました。 プロジェクトコードでは、条件全体が1行に書き込まれます。 これは、優れたコード設計がいかに重要であるかを思い出させるものです。 コードの読み取りと理解を容易にするだけでなく、エラーをすばやく見つけるのにも役立ちます。

 internal static class RemoteDataNameStrings { .... internal const string MinRunspaces = "MinRunspaces"; internal const string MaxRunspaces = "MaxRunspaces"; .... } internal void ExecuteConnect(....) { .... if ( connectRunspacePoolObject.Data .Properties[RemoteDataNameStrings.MinRunspaces] != null && connectRunspacePoolObject.Data .Properties[RemoteDataNameStrings.MinRunspaces] != null ) { try { clientRequestedMinRunspaces = RemotingDecoder.GetMinRunspaces( connectRunspacePoolObject.Data); clientRequestedMaxRunspaces = RemotingDecoder.GetMaxRunspaces( connectRunspacePoolObject.Data); clientRequestedRunspaceCount = true; } .... } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3001 '&&'演算子の左右に同じ部分式があります。 System.Management.Automationサーバーremotesession.cs 633



GitHubのコードにリンクします 。



タイプミスにより、同じチェックが再度2回実行されました。 ほとんどの場合、2番目のケースでは、静的クラスRemoteDataNameStringsの定数フィールドMaxRunspacesを使用する必要がありました。



メソッドによって返される未使用の値



メソッドの戻り値が使用されないという事実に特徴的なエラーがあります。 原因と結果は大きく異なる可能性があります。 プログラマーはStringオブジェクトが不変であることを忘れてしまい、行編集メソッドは現在のシンクを変更するのではなく、結果として新しいシンクを返します。 同様のケースは、操作の結果が新しいコレクションである場合のLINQの使用です。 ここでも同様のエラーが発生しました。

 private CatchClauseAst CatchBlockRule(.... ref List<TypeConstraintAst> errorAsts) { .... if (errorAsts == null) { errorAsts = exceptionTypes; } else { errorAsts.Concat(exceptionTypes); // <= } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3010関数 'Concat'の戻り値を使用する必要があります。 System.Management.Automation Parser.cs 4973



GitHubのコードにリンクします 。



すぐに、 errorAstsパラメーターが refキーワードと共に使用されるという事実に注意を喚起したいと思います。これは、メソッドの本体のリンクを変更することを意味します。 コードのロジックは単純です。errorAstsリンクがnullの場合、別のコレクションへのリンクを割り当てます。それ以外の場合は、 exceptionTypesコレクションの要素を既存のコレクションに追加します。 確かに、オーバーレイは2番目の部分で出てきました。 Concatメソッドは、既存のコレクションを変更せずに新しいコレクションを返します。 したがって、 errorAstsコレクションは変更されずに残り、新しいコレクション（ errorAstsおよびexceptionTypes要素を含む）は無視されます。



この問題はいくつかの方法で解決できます。

• ListクラスのAddRangeメソッドを使用すると、現在のリストに新しい要素が追加されます。
• ToListメソッドの呼び出しを忘れずに、 Concatメソッドの結果を使用して、目的の型にキャストします。

as 演算子を使用したキャスト後の間違ったリンクの確認



金メダル診断ルールV3019 ！ 確かに私は言うつもりはありませんが、私が記事を書いたほとんどすべてのC＃-プロジェクトで、このエラーが発生しました。 通常の読者は、 as演算子を使用してキャストした後、 nullのリンクをチェックするかどうかを慎重にダブルチェックする必要があるというルールをすでに知っているはずです。

 internal List<Job> GetJobsForComputer(String computerName) { .... foreach (Job j in ChildJobs) { PSRemotingChildJob child = j as PSRemotingChildJob; if (j == null) continue; if (String.Equals(child.Runspace .ConnectionInfo .ComputerName, computerName, StringComparison.OrdinalIgnoreCase)) { returnJobList.Add(child); } } return returnJobList; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3019 「as」キーワードを使用した型変換後、おそらく誤った変数がnullと比較されます。 変数 'j'、 'child'を確認してください。 System.Management.Automation Job.cs 1876



GitHubのコードにリンクします 。



jをPSRemotingChildJob型にキャストした結果は子リンクに書き込まれます 。つまり、そこにnullを書き込むことができます（元のリンクがnullの場合、またはキャストが失敗した場合）。 ただし、元の参照jについてはnullの不等式がチェックされ、さ​​らに低い場合は、 子オブジェクトのRunspaceプロパティへの参照です。 したがって、 j！= Nullおよびchild == nullの場合、 j == nullのチェックは役に立たず、派生リンクのインスタンスメンバーにアクセスするときにNullReferenceException型の例外がスローされます。



このような場所をさらに2つ見つけました。

• V3019「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数 'j'、 'child'を確認してください。 System.Management.Automation Job.cs 1900
• V3019「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数 'j'、 'child'を確認してください。 System.Management.Automation Job.cs 1923

操作の順序が正しくありません

 private void CopyFileFromRemoteSession(....) { .... ArrayList remoteFileStreams = GetRemoteSourceAlternateStreams(ps, sourceFileFullName); if ((remoteFileStreams.Count > 0) && (remoteFileStreams != null)) .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3027変数 ' remoteFileStreams 'は、同じ論理式でnullに対して検証される前に、論理式で使用されました。 System.Management.Automation FileSystemProvider.cs 4126



GitHubのコードにリンクします 。



運がよければコードは正常に動作しますが、運がよければ-null参照を逆参照しようとすると、 NullReferenceException型の例外がスローされます。 remoteFileStreams！= Null部分式は、この式で役割を果たさず、例外からも保護しません。 明らかに、適切な操作のためには、部分式を交換する必要があります。



まあ、私たちは皆人間であり、間違いを犯します。 そして、これらのエラーを見つけるにはアナライザーが必要です。



ヌル参照の逆参照の可能性

 internal bool SafeForExport() { return DisplayEntry.SafeForExport() && ItemSelectionCondition == null || ItemSelectionCondition.SafeForExport(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3080 null参照解除の可能性があります。 「ItemSelectionCondition」の検査を検討してください。 System.Management.Automation displayDescriptionData_List.cs 352



GitHubのコードにリンクします 。



NullReferenceException型の潜在的な例外。 部分式ItemSelectionCondition.SafeForExport（）は、最初の部分式の結果がfalseの場合にのみ評価されます 。 DisplayEntry.SafeForExport（）がfalseを返し、 ItemSelectionCondition == nullを返す場合、2番目の部分式-ItemSelectionCondition.SafeForExport（）が評価され、null参照の逆参照の問題が発生します（その結果、例外が生成されます）。



同様のコードがもう一度発生しました。 関連する警告： V3080 null参照解除の可能性。 「EntrySelectedBy」の検査を検討してください。 System.Management.Automation displayDescriptionData_Wide.cs 247



別のケース。

 internal Collection<ProviderInfo> GetProvider( PSSnapinQualifiedName providerName) { .... if (providerName == null) { ProviderNotFoundException e = new ProviderNotFoundException( providerName.ToString(), SessionStateCategory.CmdletProvider, "ProviderNotFound", SessionStateStrings.ProviderNotFound); throw e; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3080 null参照解除の可能性があります。 「providerName」の検査を検討してください。 System.Management.Automation SessionStateProviderAPIs.cs 1004



GitHubのコードにリンクします 。



このようなコードが見つかる場合があります。あるタイプの例外をスローしたかったのですが、別のタイプでした。 なんで？ この場合、 providerName参照がnullであることが確認されますが、以下では、例外オブジェクトが作成されると、同じリンクでToStringインスタンスメソッドが呼び出されます。 結果は、計画どおりProviderNotFoundExceptionではなく、 NullReferenceException型の例外になります。



別の同様のコードが一致しました。 関連する警告： V3080 null参照解除の可能性。 「ジョブ」の検査を検討してください。 System.Management.Automation PowerShellETWTracer.cs 1088



nullを チェックする前に参照を使用する

 internal ComplexViewEntry GenerateView(...., FormattingCommandLineParameters inputParameters) { _complexSpecificParameters = (ComplexSpecificParameters)inputParameters.shapeParameters; int maxDepth = _complexSpecificParameters.maxDepth; .... if (inputParameters != null) mshParameterList = inputParameters.mshParameterList; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3095 nullに対して検証される前に、「inputParameters」オブジェクトが使用されました。 行を確認してください：430、436。System.Management.Automation FormatViewGenerator_Complex.cs 430



GitHubのコードにリンクします 。



inputParameters！= Nullをチェックすると、チェックされる参照がnullになる可能性があります 。 フィールドmshParameterListにアクセスするときにNullReferenceExceptionを取得しないように再保証されます。 すべてが正しいです。 ここに、同じオブジェクトの別のインスタンスフィールドshapeParametersに既にアピールしているものがあります。 参照が最初にnullだった場合、これら2つの操作の間のinputParametersは変更されないため、 nullの不等式をチェックしても例外が発生することはありません。



同様のケース：

 public CommandMetadata(CommandMetadata other) { .... _parameters = new Dictionary<string, ParameterMetadata>( other.Parameters.Count, StringComparer.OrdinalIgnoreCase); // deep copy if (other.Parameters != null) .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3095 「other.Parameters」オブジェクトは、nullに対して検証される前に使用されました。 行を確認してください：189、192。System.Management.Automation CommandMetadata.cs 189



GitHubのコードにリンクします 。



他のオブジェクトのParametersプロパティがnullではないことを確認しますが、上記の数行はインスタンスプロパティCountを参照しています。 ここで何かが明らかに間違っています。



未使用のコンストラクターパラメーター



新しい診断ルールの結果が表示された直後に表示されると便利です。 そのため、 V3117の診断で発生しました 。

 private void PopulateProperties( Exception exception, object targetObject, string fullyQualifiedErrorId, ErrorCategory errorCategory, string errorCategory_Activity, string errorCategory_Reason, string errorCategory_TargetName, string errorCategory_TargetType, string errorCategory_Message, string errorDetails_Message, string errorDetails_RecommendedAction, string errorDetails_ScriptStackTrace) { .... } internal ErrorRecord( Exception exception, object targetObject, string fullyQualifiedErrorId, ErrorCategory errorCategory, string errorCategory_Activity, string errorCategory_Reason, string errorCategory_TargetName, string errorCategory_TargetType, string errorCategory_Message, string errorDetails_Message, string errorDetails_RecommendedAction) { PopulateProperties( exception, targetObject, fullyQualifiedErrorId, errorCategory, errorCategory_Activity, errorCategory_Reason, errorCategory_TargetName, errorCategory_TargetType, errorDetails_Message, errorDetails_Message, errorDetails_RecommendedAction, null); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio 警告 ： V3117コンストラクターパラメーター 'errorCategory_Message'は使用されません。 System.Management.Automation ErrorPackage.cs 1125



» GitHubのコードへのリンク 。



ErrorRecordコンストラクターは、フィールドおよびその他のアクションを初期化するPopulatePropertiesメソッドを呼び出します。 アナライザーは、コンストラクターのパラメーターの1つであるerrorCategory_Messageが本体で使用されていないことを警告します。 実際、 PopulatePropertiesメソッドの呼び出しをよく見ると、 errorDetails_Message引数がメソッドに2回渡されていますが、 errorCategory_Messageは渡されていないことがわかります。 PopulatePropertiesメソッドのパラメーターを調べて、エラーがあることを確認します。



常に偽である条件



複雑な診断を実装し、興味深いエラーを見つけるのに役立つPVS-Studioの機能の1つは、いわゆる「仮想値」です。これにより、プログラム実行の特定の段階で変数が取りうる値の範囲を見つけることができます。 詳細については、 「仮想値計算を使用したエラーの検索」の記事から入手できます。 このメカニズムに基づいて、 V3022やV3063などの診断ルールが構築されます。 彼らの助けを借りて、多くの場合、興味深いエラーを検出することが可能です。 今回はこれが起こったので、見つかったエラーの1つを検討することを提案します。

 public enum RunspacePoolState { BeforeOpen = 0, Opening = 1, Opened = 2, Closed = 3, Closing = 4, Broken = 5, Disconnecting = 6, Disconnected = 7, Connecting = 8, } internal virtual int GetAvailableRunspaces() { .... if (stateInfo.State == RunspacePoolState.Opened) { .... return (pool.Count + unUsedCapacity); } else if (stateInfo.State != RunspacePoolState.BeforeOpen && stateInfo.State != RunspacePoolState.Opening) { throw new InvalidOperationException( HostInterfaceExceptionsStrings.RunspacePoolNotOpened); } else if (stateInfo.State == RunspacePoolState.Disconnected) { throw new InvalidOperationException( RunspacePoolStrings.CannotWhileDisconnected); } else { return maxPoolSz; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio 警告 ： V3022式 'stateInfo.State == RunspacePoolState.Disconnected'は常にfalseです。 System.Management.Automation RunspacePoolInternal.cs 581



» GitHubのコードへのリンク 。



アナライザーは、式stateInfo.State == RunspacePoolState.Disconnectedが常にfalseであると主張します。 そうですか？ もちろん、そうでなければ、なぜこのコードを書くのでしょう！



開発者は前の状態でミスをしました。 実際には、 stateInfo.State == RunspacePoolState.Disconnectedの場合、前のifステートメントが常に実行されます。 エラーを修正するには、最後の2つのif （ else if ） ステートメントを交換します。

もっとミス？

はい、アナライザーが疑わしいと判断した場所は他にもたくさんありました。 定期的に記事を読んでいる人は、多くの場合、すべてのエラーを書き出すわけではないことを知っています。 モノの検証記事のサイズには達しなかったかもしれませんが、執筆のための資料はまだあります。 すべての警告に対する最大の関心は開発者から生じるはずですが、残りの読者には、最も興味深い疑わしい場所を見せようとするだけです。

「開発者に伝えましたか？」

奇妙なことに、私たちはまだ定期的にこの質問をされています。 発見されたエラーについては常に開発者に通知しますが、今回はもう少し先に進むことにしました。



私はGitterを通じて個人的に開発者の一人（こんにちは、Sergey！）と話をしました。 このソリューションの利点は明らかです-見つかったエラーについて議論し、アナライザーに関するフィードバックを取得し、記事で何かを修正できます。 人々が静的解析の利点を理解するのは素晴らしいことです。 開発者は、アナライザーによって検出されたコードフラグメントが実際にエラーであることに注意し、感謝し、エラーは今後修正されると述べました。 そして、私は、リポジトリ内のこれらのコードフラグメントへのリンクを提供することで、彼らを少し助けることにしました。 アナライザーの使用について少し話しました。 静的分析を定期的に使用する必要があることを人々が理解しているのは素晴らしいことです。 そうなることを望み、アナライザーを開発プロセスに導入します。



これが相互に有益な協力です。

おわりに

予想どおり、アナライザーは多くの不審な場所を検出できました。 そして、ポイントは、誰かが間違ったコードを書いたり、十分な資格を持っていないということではありません（もちろん、これは起こりますが、この場合はそうではないと思います）-人的要因は非難することです。 これが人間の本質であり、誰もが間違っています。 静的分析ツールは、コード内のエラーを示すことにより、この欠点を補おうとします。 したがって、それらを定期的に使用することが、最良のコードへの道です。 そして、100回聞くよりも1回見る方が良いので、自分でPVS-Studioを試すことをお勧めします。

他のMicrosoftプロジェクトの分析

C ++

• CNTK検証 ;
• ChakraCoreを確認してください 。
• CoreCLRを確認してください 。
• Windows 8ドライバーサンプルを確認してください 。
• Microsoft Word 1.1aの検証 。
• Visual C ++ライブラリの確認： 1、2 ;
• カサブランカを確認してください 。

C＃

• CoreFXを確認してください 。
• .Net Compiler Platform（Roslyn）の確認 ;
• コード契約の検証 ;
• MSBuildを確認してください 。
• WPFサンプルを確認します 。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：セルゲイヴァシリエフ。 マイクロソフトのプロジェクト：PowerShellの分析を引き続き確認します 。