Infer.NETコードにはどのようなエラーが隠されていますか？

Microsoftのプロジェクトのソースコードの公開は、それらを検証する正当な理由です。 今回も例外ではなく、今日はInfer.NETコードで見つかった疑わしい場所を見ていきます。 注釈を付けて、要点を理解しましょう！

プロジェクトとアナライザーについて少し

Infer.NETは、Microsoftの専門家によって開発された機械学習システムです。 プロジェクトのソースコードは最近GitHubで利用可能になりました。これが検証の理由でした。 プロジェクトの詳細については、たとえばこちらをご覧ください 。

プロジェクトは、PVS-Studioバージョン6.26静的アナライザーを使用してチェックされました。 PVS-Studioは、Windows、Linux、macOSのC \ C ++ \ C＃（およびまもなくJava）のコードのエラーを探していることを思い出させてください。 C＃コードは現在Windowsでのみ分析されています。 アナライザーをダウンロードして、プロジェクトで試すことができます。

チェック自体は非常に簡単で、問題はありませんでした。 以前は、GitHubからプロジェクトをアンロードし、必要なパッケージ（依存関係）を復元し、プロジェクトが正常にビルドされたことを確認しました。 これは、アナライザーが完全な分析に必要なすべての情報にアクセスできるようにするために必要です。 数回クリックするだけで組み立てた後、Visual StudioのPVS-Studioプラグインを使用してソリューションの分析を開始しました。

ところで、これはPVS-Studioを使用してテストしたMicrosoftの最初のプロジェクトではありません。他にもRoslyn 、 MSBuild 、 PowerShell 、 CoreFXなどがありました 。

ご注意 あなたまたはあなたの知人がJavaコードの分析に興味がある場合は、「Java用のアナライザーが欲しい」を選択することでサポートに連絡できます。 アナライザーのパブリックベータ版はありませんが、すぐに利用可能になるはずです。 秘密の実験室のどこかで（壁を通して）、彼らは積極的に作業しています。

しかし、十分な抽象的な話-コードの問題を見てみましょう。

これはバグですか、それとも機能ですか？

自分でエラーを見つけようとすることをお勧めします-完全に解決可能なタスクです。 正直なところ、「 2017年のC ++プロジェクトのトップ10エラー 」という記事にあったことの精神に冗談はありません。 そのため、コードスニペットの後に提供されるアナライザーの警告を読むのを急がないでください。

private void MergeParallelTransitions() { .... if ( transition1.DestinationStateIndex == transition2.DestinationStateIndex && transition1.Group == transition2.Group) { if (transition1.IsEpsilon && transition2.IsEpsilon) { .... } else if (!transition1.IsEpsilon && !transition2.IsEpsilon) { .... if (double.IsInfinity(transition1.Weight.Value) && double.IsInfinity(transition1.Weight.Value)) { newElementDistribution.SetToSum( 1.0, transition1.ElementDistribution, 1.0, transition2.ElementDistribution); } else { newElementDistribution.SetToSum( transition1.Weight.Value, transition1.ElementDistribution, transition2.Weight.Value, transition2.ElementDistribution); } .... } 

PVS-Studio警告 ： V3001 「&&」演算子の左右には、同一の副次式「double.IsInfinity（transition1.Weight.Value）」があります。 ランタイムAutomaton.Simplification.cs 479

コードスニペットからわかるように、このメソッドは変数のペア-transition1とtransition2で機能しています。 同様の名前を使用することは正当な場合もありますが、この場合、名前のどこかで誤ってミスをする可能性が高くなることを覚えておく価値があります。

これは、無限大（ double.IsInfinity ）の数をチェックするときに起こったことです。 エラーのため、同じ変数の値を2回チェックしました-transition1.Weight.Value 。 2番目の部分式のチェックされた値は、変数transition2.Weight.Valueでした。

別の同様の疑わしいコード。

 internal MethodBase ToMethodInternal(IMethodReference imr) { .... bf |= BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance; .... } 

PVS-Studio警告 ： V3001 「|」の左側と右側に同一の副次式「BindingFlags.Public」があります 演算子。 コンパイラCodeBuilder.cs 194

bf変数の値を形成するとき、 BindingFlags.Public列挙要素が2回使用されます。 このコードには追加のフラグ操作が含まれているか、 BindingFlags.Publicの 2回目の使用の代わりに、異なる列挙値が必要です。

ところで、ソースコードでは、このコードは1行で記述されています。 テーブルスタイル（ここのような）でフォーマットされている場合、問題の検出が容易になるように思えます。

続けましょう。 メソッド全体を持ち帰り、エラー（またはエラー）を自分で見つけることをお勧めします。

 private void ForEachPrefix(IExpression expr, Action<IExpression> action) { // This method must be kept consistent with GetTargets. if (expr is IArrayIndexerExpression) ForEachPrefix(((IArrayIndexerExpression)expr).Target, action); else if (expr is IAddressOutExpression) ForEachPrefix(((IAddressOutExpression)expr).Expression, action); else if (expr is IPropertyReferenceExpression) ForEachPrefix(((IPropertyReferenceExpression)expr).Target, action); else if (expr is IFieldReferenceExpression) { IExpression target = ((IFieldReferenceExpression)expr).Target; if (!(target is IThisReferenceExpression)) ForEachPrefix(target, action); } else if (expr is ICastExpression) ForEachPrefix(((ICastExpression)expr).Expression, action); else if (expr is IPropertyIndexerExpression) ForEachPrefix(((IPropertyIndexerExpression)expr).Target, action); else if (expr is IEventReferenceExpression) ForEachPrefix(((IEventReferenceExpression)expr).Target, action); else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action); else if (expr is IMethodInvokeExpression) ForEachPrefix(((IMethodInvokeExpression)expr).Method, action); else if (expr is IMethodReferenceExpression) ForEachPrefix(((IMethodReferenceExpression)expr).Target, action); else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action); else if (expr is IDelegateInvokeExpression) ForEachPrefix(((IDelegateInvokeExpression)expr).Target, action); action(expr); } 

見つけた？ 確認中です！

PVS-Studioの警告 ：

• V3003 「if（A）{...} else if（A）{...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 行を確認してください：1719、1727。コンパイラCodeRecognizer.cs 1719
• V3003 「if（A）{...} else if（A）{...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 チェック行：1721、1729。コンパイラCodeRecognizer.cs 1721

問題をより明確にするために、コードを少し単純化します。

 private void ForEachPrefix(IExpression expr, Action<IExpression> action) { if (....) .... else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action); .... else if (expr is IUnaryExpression) ForEachPrefix(((IUnaryExpression)expr).Expression, action); else if (expr is IAddressReferenceExpression) ForEachPrefix(((IAddressReferenceExpression)expr).Expression, action) .... } 

条件式は複製され、複数のifステートメントのブランチが作成されます。 おそらく、このコードはコピーアンドペーストメソッドを使用して記述されているため、問題が発生しました。 次の理由から、重複のブランチは実行されないことがわかりました 。

• 条件式が真の場合 、対応するペアの最初のifステートメントの本体が実行されます。
• 最初のケースで条件式がfalseの場合、2番目の条件式はfalseになります。

then-ブランチには同じアクションが含まれているため、今では紛らわしい冗長コードのように見えます。 これは別の種類の問題である可能性があります-テイクではなく、他のチェックを実行する必要があります。

続けます。

 public int Compare(Pair<int, int> x, Pair<int, int> y) { if (x.First < y.First) { if (x.Second >= y.Second) { // y strictly contains x return 1; } else { // No containment - order by left bound return 1; } } else if (x.First > y.First) { if (x.Second <= y.Second) { // x strictly contains y return -1; } else { // No containment - order by left bound return -1; } } .... } 

PVS-Studioの警告 ：

• V3004 「then」ステートメントは「else」ステートメントと同等です。 ランタイムRegexpTreeBuilder.cs 1080
• V3004 「then」ステートメントは「else」ステートメントと同等です。 ランタイムRegexpTreeBuilder.cs 1093

コードには、 thenブランチとelseブランチの同一のボディを持つ2つの条件ステートメントが含まれているため、非常に疑わしく見えます 。 どちらの場合も、おそらく異なる値を返す価値があります。 または、これが想定される動作である場合、冗長な条件ステートメントを削除すると便利です。

興味深いサイクルがありました。 以下の例：

 private static Set<StochasticityPattern> IntersectPatterns(IEnumerable<StochasticityPattern> patterns) { Set<StochasticityPattern> result = new Set<StochasticityPattern>(); result.AddRange(patterns); bool changed; do { int count = result.Count; AddIntersections(result); changed = (result.Count != count); break; } while (changed); return result; } 

PVS-Studio警告 ： V3020ループ内の無条件の「ブレーク」。 コンパイラDefaultFactorManager.cs 474

無条件のbreakステートメントにより、ループが1回だけ実行され、 変更された制御変数は使用されません。 一般的に、コードは奇妙で疑わしいように見えます。

同じメソッド（正確なコピー）が別のクラスで見つかりました。 対応するアナライザー警告： V3020ループ内の無条件の「ブレーク」。 Visualizers.Windows FactorManagerView.cs 350

ところで、メソッドはループ内で無条件のcontinueステートメントに遭遇しました（アナライザーは同じ診断でそれを見つけました）が、これは特別な一時的な解決策であることを確認するコメントが上にありました：

 // TEMPORARY continue; 

無条件の休憩声明の近くにそのようなコメントがなかったことを思い出します。

続けましょう。

 internal static DependencyInformation GetDependencyInfo(....) { .... IExpression resultIndex = null; .... if (resultIndex != null) { if (parameter.IsDefined( typeof(SkipIfMatchingIndexIsUniformAttribute), false)) { if (resultIndex == null) throw new InferCompilerException( parameter.Name + " has SkipIfMatchingIndexIsUniformAttribute but " + StringUtil.MethodNameToString(method) + " has no resultIndex parameter"); .... } .... } .... } 

PVS-Studio 警告 ： V3022式 'resultIndex == null'は常にfalseです。 コンパイラFactorManager.cs 382

すぐに、宣言と上記の検証の間で、 resultIndex変数の値が変わる可能性があることに注意してください。 ただし、 resultIndex！= NullとresultIndex == nullのチェックの間に、値を変更することはできません。 したがって、式resultIndex == nullの結果は常にfalseになります 。これは、例外がスローされないことを意味します。

問題を見つけるために、私の提案なしに自分でエラーを見つけることに興味があることを願っていますが、念のため、もう一度やり直すことを提案します。 メソッドコードは小さいので、全体を説明します。

 public static Tuple<int, string> ComputeMovieGenre(int offset, string feature) { string[] genres = feature.Split('|'); if (genres.Length < 1 && genres.Length > 3) { throw new ArgumentException(string.Format( "Movies should have between 1 and 3 genres; given {0}.", genres.Length)); } double value = 1.0 / genres.Length; var result = new StringBuilder( string.Format( "{0}:{1}", offset + MovieGenreBuckets[genres[0]], value)); for (int i = 1; i < genres.Length; ++i) { result.Append( string.Format( "|{0}:{1}", offset + MovieGenreBuckets[genres[i].Trim()], value)); } return new Tuple<int, string>(MovieGenreBucketCount, result.ToString()); } 

ここで何が起こるか見てみましょう。 入力文字列は、文字「|」で解析されます。 配列の長さが期待どおりでない場合、例外をスローする必要があります。 ちょっと待って ... genres.Length <1 && genres.Length> 3 ？ 式に必要な値の範囲（ [int.MinValue..1）と（3..int.MaxValue] ）の両方にすぐに該当する数値はないため、式の結果は常にfalseになります 。 したがって、このチェックでは何も保護されず、予期される例外はスローされません。

これはアナライザーが警告するものです： V3022式 'genres.Length <1 && genres.Length> 3'は常にfalseです。 おそらく '||' ここで演算子を使用する必要があります。 Evaluator Features.cs 242

疑わしい核分裂操作が発生しました。

 public static void CreateTrueThetaAndPhi(....) { .... double expectedRepeatOfTopicInDoc = averageDocLength / numUniqueTopicsPerDoc; .... int cnt = Poisson.Sample(expectedRepeatOfTopicInDoc); .... } 

PVS-Studio警告 ： V3041式は暗黙的に 'int'型から 'double'型にキャストされました。 分数部分の損失を避けるために、明示的な型キャストの使用を検討してください。 例：double A =（double）（X）/ Y;。 LDA Utilities.cs 74

これはここでは疑わしい：整数除算が実行され（変数averageDocLengthとnumUniqueTopicsPerDocはint型）、結果はdouble型の変数に書き込まれます。 質問が請う：それは特別に行われたのか、それとも実数の除算がまだ暗示されていたのか？ 変数expectedRepeatOfTopicInDocがint型であった場合、これにより考えられる質問が解決されます。

他の場所では、引数が疑わしい変数expectedRepeatOfTopicInDocであるPoisson.Sampleメソッドが、たとえば以下で説明するように使用されます。

 int numUniqueWordsPerTopic = Poisson.Sample((double)averageWordsPerTopic); 

averageWordsPerTopicはint型で、使用場所ですでにdoubleに変換されています。

そして、ここに別の使用場所があります。

 double expectedRepeatOfWordInTopic = ((double)numDocs) * averageDocLength / numUniqueWordsPerTopic; .... int cnt = Poisson.Sample(expectedRepeatOfWordInTopic); 

変数の名前は元の例と同じであることに注意してください。実数の除算のみを使用して、 expectedRepeatOfWordInTopicを初期化します（明示的にnumDocsをdoubleにキャストします ）。

一般に、アナライザーが警告を発行した開始場所を調べる価値があります。

しかし、これを修正する必要があるかどうか、およびコードの作成者にはどのように（もっとよく知っている）許可するかについての考察は、さらに先に進みましょう。 次の不審な部門へ。

 public static NonconjugateGaussian BAverageLogarithm(....) { .... double v_opt = 2 / 3 * (Math.Log(mx * mz / Ex2 / 2) - m); if (v_opt != v) { .... } .... } 

PVS-Studio警告 ： V3041式は暗黙的に 'int'型から 'double'型にキャストされました。 分数部分の損失を避けるために、明示的な型キャストの使用を検討してください。 例：double A =（double）（X）/ Y;。 ランタイムProductExp.cs 137

アナライザーは、整数除算の疑わしい操作を再び検出しました。 2と3は整数の数値リテラルであり、式2/3の結果は0になります 。 その結果、式全体は次の形式を取ります。

 double v_opt = 0 * expr; 

同意します、少し奇妙です。 何回かこの警告に戻り、何らかのキャッチを見つけようとして、記事に追加しようとはしませんでした。 このメソッドには数学とさまざまな公式があります（率直に言って、私は本当に分解したくありませんでした）。 さらに、この記事で書いた警告については可能な限り懐疑的であり、それらをよりよく研究した後にのみ説明します。

しかし、それから私は夜明けになりました-なぜ2/3と書かれた0の形式の乗数が必要なのですか？ とにかくこの場所は一見の価値があります。

 public static void WriteAttribute(TextWriter writer, string name, object defaultValue, object value, Func<object, string> converter = null) { if ( defaultValue == null && value == null || value.Equals(defaultValue)) { return; } string stringValue = converter == null ? value.ToString() : converter(value); writer.Write($"{name}=\"{stringValue}\" "); } 

PVS-Studio警告 ： V3080 null参照解除の可能性があります。 「値」の検査を検討してください。 コンパイラWriteHelpers.cs 78

条件に基づいたアナライザーのかなり公平な主張。 値== nullの場合、式value.Equals（defaultValue）でnull参照の逆参照が発生する可能性があります 。 この式は||演算子の右側のオペランドであるため、計算するには左側のオペランドがfalseである必要があり、このためにdefaultValue \ value変数の少なくとも1つがnullでないことで十分です。 その結果、 defaultValue！= Null 、および値== nullの場合 ：

• defaultValue == null- > false ;
• defaultValue == null && value == null- > false ; （ 値のチェックは行われませんでした）
• value.Equals（defaultValue） -> NullReferenceException 、 値はnullであるため 。

同様のケースを見てみましょう：

 public FeatureParameterDistribution( GaussianMatrix traitFeatureWeightDistribution, GaussianArray biasFeatureWeightDistribution) { Debug.Assert( (traitFeatureWeightDistribution == null && biasFeatureWeightDistribution == null) || traitFeatureWeightDistribution.All( w => w != null && w.Count == biasFeatureWeightDistribution.Count), "The provided distributions should be valid and consistent in the number of features."); .... } 

PVS-Studio警告 ： V3080 null参照解除の可能性があります。 「traitFeatureWeightDistribution」の検査を検討してください。 レコメンダーFeatureParameterDistribution.cs 65

余分な部分は捨てて、ブール値を計算するためのロジックのみを残します。これにより、把握しやすくなります。

 (traitFeatureWeightDistribution == null && biasFeatureWeightDistribution == null) || traitFeatureWeightDistribution.All( w => w != null && w.Count == biasFeatureWeightDistribution.Count) 

再び、||の右オペランド 左の計算結果がfalseの場合にのみ計算されます 。 traitFeatureWeightDistribution == nullおよびbiasFeatureWeightDistribution！= Nullの場合を含め、左側のオペランドをfalseにすることができます。 次に、||演算子の右側のオペランドが計算され、 traitFeatureWeightDistribution.Allの呼び出しによりArgumentNullExceptionが発生します。

別の興味深いコード：

 public static double GetQuantile(double probability, double[] quantiles) { .... int n = quantiles.Length; if (quantiles == null) throw new ArgumentNullException(nameof(quantiles)); if (n == 0) throw new ArgumentException("quantiles array is empty", nameof(quantiles)); .... } 

PVS-Studio警告 ： V3095 nullに対して検証される前に「分位点」オブジェクトが使用されました。 行を確認してください：91、92。ランタイムOuterQuantiles.cs 91

quantiles.Lengthプロパティに最初にアクセスし 、次に変位値の null がチェックされることに注意してください。 その結果、 quantiles == nullの場合、メソッドは例外をスローしますが、少し間違っています。 どうやら、彼らは場所の行を台無しにした。

以前のエラーの検出に自分で対処できた場合は、コーヒーを1杯入れて、次の方法でエラーを見つけて、その偉業を繰り返してみることをお勧めします。 少し面白くするために、メソッドコード全体を引用します。

（ フルサイズへのリンク ）

わかりました、わかりました、それは冗談でした（または成功しましたか？！）。 タスクを少し簡単にしましょう。

 if (sample.Precision < 0) { precisionIsBetween = true; lowerBound = -1.0 / v; upperBound = -mean.Precision; } else if (sample.Precision < -mean.Precision) { precisionIsBetween = true; lowerBound = 0; upperBound = -mean.Precision; } else { // in this case, the precision should NOT be in this interval. precisionIsBetween = false; lowerBound = -mean.Precision; lowerBound = -1.0 / v; } 

良くなった？ アナライザーは、このコードに対して次の警告を発行しました。V3008 「lowerBound」変数には、連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：324、323。ランタイムGaussianOp.cs 324

実際、最後のelseブランチでは、 lowerBound変数の値が連続して2回割り当てられます。 どうやら（そして上記のコードで判断すると）、変数upperBoundは割り当ての1つに関与している必要があります。

さらに続きます。

 private void WriteAucMatrix(....) { .... for (int c = 0; c < classLabelCount; c++) { int labelWidth = labels[c].Length; columnWidths[c + 1] = labelWidth > MaxLabelWidth ? MaxLabelWidth : labelWidth; for (int r = 0; r < classLabelCount; r++) { int countWidth = MaxValueWidth; if (countWidth > columnWidths[c + 1]) { columnWidths[c + 1] = countWidth; } } .... } 

PVS-Studio警告 ： V3081 「r」カウンターはネストされたループ内では使用されません。 「c」カウンターの使用状況を調べることを検討してください。 CommandLine ClassifierEvaluationModule.cs 459

内側のループのカウンター-r-は、このループの本体では使用されないことに注意してください。 このため、内側のループのすべての反復中に、同じ操作が同じ要素に対して実行されます。結局、インデックスは内側（ r ）ではなく外側のループ（ c ）のカウンターも使用します。

他に何が面白いと思われたのか見てみましょう。

 public RegexpFormattingSettings( bool putOptionalInSquareBrackets, bool showAnyElementAsQuestionMark, bool ignoreElementDistributionDetails, int truncationLength, bool escapeCharacters, bool useLazyQuantifier) { this.PutOptionalInSquareBrackets = putOptionalInSquareBrackets; this.ShowAnyElementAsQuestionMark = showAnyElementAsQuestionMark; this.IgnoreElementDistributionDetails = ignoreElementDistributionDetails; this.TruncationLength = truncationLength; this.EscapeCharacters = escapeCharacters; } 

PVS-Studio警告 ： V3117コンストラクターパラメーター 'useLazyQuantifier'は使用されません。 ランタイムRegexpFormattingSettings.cs 38

コンストラクターは1つのパラメーターuseLazyQuantifierを使用しません。 これは、対応する名前とタイプを持つプロパティがクラス-UseLazyQuantifierで定義されているという事実の背景に対して特に疑わしいように見えます。 どうやら、彼らは対応するパラメータを介してそれを初期化するのを忘れていました。

いくつかの潜在的に危険なイベントハンドラに会いました。 それらの1つの例を以下に示します。

 public class RecommenderRun { .... public event EventHandler Started; .... public void Execute() { // Report that the run has been started if (this.Started != null) { this.Started(this, EventArgs.Empty); } .... } .... } 

PVS-Studio警告 ： V3083イベント 'Started'の安全でない呼び出し、NullReferenceExceptionが発生する可能性があります。 イベントを呼び出す前に、ローカル変数にイベントを割り当てることを検討してください。 Evaluator RecommendederRun.cs 115

実際には、 nullの不等式のチェックとハンドラーの呼び出しの間で、イベントのサブスクライブを解除できます。また、 nullのチェックとハンドラーの呼び出しの間にイベントにサブスクライバーがない場合、 NullReferenceExceptionがスローされます。 このような問題を回避するには、たとえば、デリゲートチェーンへのリンクをローカル変数に保存するか、「?.」演算子を使用します ハンドラーを呼び出します。

上記のコードスニペットに加えて、このような場所が35個ありました。

ところで、 V3024には785 個の警告がありました。 演算子「！=」または「==」を使用して実数を比較すると、警告V3024が発行されます。 このような比較が必ずしも正しくない理由についてはここでは説明しません。これについての詳細はドキュメントに記載されており、 StackOverflowへのリンクもあります（これがこれです）。

式や計算が頻繁に見つかることを考えると、これらの警告は重要な場合もありますが、レベル3になります（すべてのプロジェクトに関連するものではないため）。

これらの警告が無関係であることが確実な場合は、 ほぼ1回クリックするだけでそれらを削除でき、アナライザー操作の総数を削減できます 。

おわりに

どういうわけか、私は長い間、プロジェクトのチェックに関する記事を書いていなかったことがわかり、このプロセスに再び触れることは非常に楽しいことでした。 また、この記事から新しい/有用な何かを学んだか、少なくとも興味を持って読んでほしいと思います。

開発者に問題領域の早期修正を望み、間違いを犯すことは普通ですが、私たちは人間であることを思い出させてください。 そのためには、人が見逃したものを見つけるために、静的アナライザーなどの追加のツールが必要ですよね？ とにかく-プロジェクトで頑張ってください、そして仕事に感謝します！

そして、静的アナライザーの最大の利点は、 通常の使用で達成されることを忘れないでください。

最高！

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：セルゲイヴァシリエフ。 Infer.NETコードにはどのようなエラーが潜んでいますか？