New Year Check .NET Core Libraries（CoreFX）

約1年前、MicrosoftはCoreCLRやCoreFXなどのプロジェクトのオープンソースコードを投稿しました。 最近まで、最後のプロジェクトはC ++ではなくC＃で作成されていたため、私たちにとって興味深いものではありませんでした。 しかし、C＃プログラミング言語のプロジェクトをサポートするPVS-Studio 6.00の新しいバージョンのリリースで、CoreFXに戻って記事を書くことにしました。

はじめに

.NET Coreは、.NET Frameworkのサブセットを含むライブラリとランタイムのモジュール実装です。 .NET Coreは、「CoreFX」と呼ばれる一連のライブラリと、最適化された小さな作業環境「CoreCLR」で構成されています。

.NET Coreはオープンソースであり、GitHubで入手できます。

• .NETコアライブラリ（CoreFX）
• .NET Core共通言語ランタイム（CoreCLR）

これらは、高品質のソースコードを含む大規模なマイクロソフト製品ですが、コードの不審な部分はまだ発見できます。

CoreCLRチェックについては、記事「 PVS-Studio：CoreCLRからの25の疑わしいコードフラグメント 」を参照してください。

この記事で説明するCoreFXプロジェクトは、C＃！をサポートするPVS-Studio 6.00静的アナライザーを使用してテストされました。

検証結果

開いているプロジェクトのチェックに関する記事の準備中に、静的アナライザーが提供するすべての警告とはまったく異なる情報を提供します。 したがって、プロジェクトの作成者が独自に分析を実行し、アナライザーによって発行されたすべてのメッセージを調査することをお勧めします。

見つかった最も危険な場所

V3027変数 'start.BaseMapping'は、同じ論理式のヌルに対して検証される前に、論理式で使用されました。 Mappings.cs 598

internal void SetSequence() { if (TypeDesc.IsRoot) return; StructMapping start = this; // find first mapping that does not have the sequence set while (!start.BaseMapping.IsSequence && //<== start.BaseMapping != null && //<==??? !start.BaseMapping.TypeDesc.IsRoot) start = start.BaseMapping; .... } 

コードに重大な論理エラーがあります！ ループの本体では、「start」という名前のオブジェクトが各反復で変更され、オブジェクトが特定の状態にある間にループが実行されます。 ただし、「start.BaseMapping！= Null」という条件のチェックは「start.BaseMapping.IsSequence」を呼び出した後に実行され、これによりnull参照の逆参照が発生する可能性があります。

V3019 「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数「comparand」、「comparedCredentialKey」を確認してください。 CredentialCache.cs 4007

 public override bool Equals(object comparand) { CredentialHostKey comparedCredentialKey = comparand as CredentialHostKey; if (comparand == null) { // This covers also the compared == null case return false; } bool equals = string.Equals(AuthenticationType, comparedCredentialKey.AuthenticationType, .... .... } 

オブジェクトは、任意のタイプまたはヌルにすることができます。 nullが到着した場合、このケースは正しく処理されます。 「CredentialHostKey」タイプに変換できないタイプのオブジェクトである場合、「comparedCredentialKey.AuthenticationType」にアクセスするとエラーが発生します。 変数「comparedCredentialKey」はnullの場合があります。

ほとんどの場合、彼らは次のように書きたいと思っていました。

 CredentialHostKey comparedCredentialKey = comparand as CredentialHostKey; if (comparedCredentialKey == null) { return false; } 

コード内の同様の場所：

• V3019「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数「comparand」、「comparedCredentialKey」を確認してください。 CredentialCache.cs 497

V3008 「HResult」変数には、連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：169、166。WebSocketException.cs 169

 private void SetErrorCodeOnError(int nativeError) { if (!Succeeded(nativeError)) { HResult = nativeError; } HResult = nativeError; //<==??? } 

何らかの理由で、条件に関係なく、HResult変数は常に同じ値を取ります。 ほとんどの場合、関数は別の方法で実装する必要があります。

V3008 「ResPrec」変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認してください：1735、1731。SQLDecimal.cs 1735

 public static SqlDecimal operator /(SqlDecimal x, SqlDecimal y) { int ResPrec; .... ResPrec = ResScale + x.m_bPrec + y.m_bPrec + 1; //<== MinScale = Math.Min(ResScale, s_cNumeDivScaleMin); ResInteger = Math.Min(ResInteger, s_NUMERIC_MAX_PRECISION); ResPrec = ResInteger + ResScale; //<== if (ResPrec > s_NUMERIC_MAX_PRECISION) ResPrec = s_NUMERIC_MAX_PRECISION; .... } 

変数「ResPrec」の値が特定の式を使用して計算されることは非常に疑わしいですが、その後、別の値で単純にこすられます。

V3020ループ内の無条件の「戻り」。 Enumerable.cs 517

 public override bool MoveNext() { switch (state) { case 1: _enumerator = _source.GetEnumerator(); state = 2; goto case 2; case 2: while (_enumerator.MoveNext()) { current = _selector(_enumerator.Current); return true; } Dispose(); break; } return false; } 

奇妙なことに、「while」ループの本体では、関数は条件なしで終了します。 コードにエラーがある可能性があります。

別の同様のループ：

• V3020ループ内の無条件の「戻り」。 JsonDataContract.cs 128

V3008 「prefix」変数には、連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認：953、952。XmlSerializationWriter.cs 953

 protected void WriteAttribute(string localName, string ns, ....) { .... string prefix = localName.Substring(0, colon); prefix = _w.LookupPrefix(ns); _w.WriteStartAttribute(prefix, localName.Substring(colon + 1), ns); .... } 

長い「コロン」の「localName」の部分文字列は変数「prefix」に保存され、この値は別の値に固定されます。 さらにコードでは、「localName」の残りの部分文字列が使用され、最初の部分が失われていることが明らかです。 非常に疑わしいコード。

V3030定期的なチェック。 'baseTableRowCounts == null'条件は、68行目で既に検証されています。MetadataAggregator.cs70

 private MetadataAggregator(....) { .... if (baseTableRowCounts == null) //<== { if (baseReader == null) { throw new ArgumentNullException("deltaReaders"); } if (baseReader.GetTableRowCount(TableIndex.EncMap) != 0) { throw new ArgumentException("....", "baseReader"); } CalculateBaseCounts(baseReader, out baseTableRowCounts, //<== out baseHeapSizes); } else { if (baseTableRowCounts == null) //<==??? { throw new ArgumentNullException("baseTableRowCounts"); } .... } .... } 

アナライザーは、すでにテストされた状態を検出しました。 コードスニペットを見ると、「else」の最後のチェック-「baseTableRowCounts == null」は意味がありません。 しかし、コードの上では、変数 "baseTableRowCounts"がnullの場合、関数CalculateBaseCounts（）を呼び出して値を変更しようとしていることがわかります。 この関数の後、ほとんどの場合、追加のチェック「baseTableRowCounts == null」では十分ではありません。 つまり ほとんどの場合、コードは次のようになります。

 private MetadataAggregator(....) { .... if (baseTableRowCounts == null) { if (baseReader == null) { throw new ArgumentNullException("deltaReaders"); } if (baseReader.GetTableRowCount(TableIndex.EncMap) != 0) { throw new ArgumentException("....", "baseReader"); } CalculateBaseCounts(baseReader, out baseTableRowCounts, out baseHeapSizes); if (baseTableRowCounts == null) { throw new ArgumentNullException("baseTableRowCounts"); } } else { .... } .... } 

その他の警告

V3022式 'readercount> = 0'は常にtrueです。 符号なしの型の値は常に> 0です。ReaderWriterLockSlim.cs977

 private void ExitAndWakeUpAppropriateWaitersPreferringWriters() { .... uint readercount = GetNumReaders(); .... if (readercount == 1 && _numWriteUpgradeWaiters > 0) { .... } else if (readercount == 0 && _numWriteWaiters > 0) { ExitMyLock(); _writeEvent.Set(); } else if (readercount >= 0) { .... } else ExitMyLock(); .... } 

変数 "readercount"には符号なしの型があるため、条件 "readercount> = 0"は意味がありません。 おそらく、それは以前は符号付きタイプでしたが、最後の「else」のExitMyLOck（）関数では、少なくとも実行される可能性がありました。 現在、このコードは制御されません。 この場所を書き換える必要があります。

V3014 「for」演算子内で誤った変数がインクリメントされている可能性があります。 「i」の検討を検討してください。 RegexCharClass.cs 1094

 private void Canonicalize() { .... for (i = 1, j = 0; ; i++) { for (last = _rangelist[j]._last; ; i++) { if (i == _rangelist.Count || last == LastChar) { done = true; break; } if ((CurrentRange = _rangelist[i])._first > last + 1) break; if (last < CurrentRange._last) last = CurrentRange._last; } _rangelist[j] = new SingleRange(_rangelist[j]._first, last); j++; if (done) break; if (j < i) _rangelist[j] = _rangelist[i]; } _rangelist.RemoveRange(j, _rangelist.Count - j); .... } 

アナライザーは、あるサイクルのカウンターの変化を別のサイクルで検出しました。 この関数にエラーがあるかどうかを言うのは困難ですが、スペルはあまり明確ではありません。 配列にアクセスするときにインデックスのどこかで間違いを犯す可能性があります。 このようなコードでは、1つのカウンターの変化を数サイクルで監視することは困難です。

V3004 「then」ステートメントは「else」ステートメントと同等です。 XmlSerializationWriterILGen.cs 1213

 private void WriteMember(...., TypeDesc memberTypeDesc, ....) { .... if (memberTypeDesc.IsArray) { LocalBuilder localI = ilg.DeclareOrGetLocal(typeof(Int32), iVar); ilg.For(localI, 0, ilg.GetLocal(aVar)); } else { LocalBuilder localI = ilg.DeclareOrGetLocal(typeof(Int32), iVar); ilg.For(localI, 0, ilg.GetLocal(aVar)); } .... } 

何にも影響しない条件 1つのコードが常に実行されます。 古典的なコピーアンドペースト。

V3004 「then」ステートメントは「else」ステートメントと同等です。 SqlUtil.cs 93

 internal static void ContinueTask(....) { .... if (connectionToDoom != null || connectionToAbort != null) { try { onSuccess(); } catch (Exception e) { completion.SetException(e); } } else { // no connection to doom - reliability section not required try { onSuccess(); } catch (Exception e) { completion.SetException(e); } } .... } 

条件には同じコードがたくさんありますが、コメントは状況が異なると言っています。

おわりに

以下は、Microsoftがテストした別のプロジェクトです。 そのようなボリュームの場合、プロジェクトにはかなり高品質のコードが含まれていますが、プログラマーはまだ間違いを犯す可能性があります。 この記事はレビューであり、レポートで受信されたすべてのアナライザー警告が含まれているわけではありません。

2つの非常に重要な状況がコードの品質に寄与しています。

1. プロジェクトの定期的な静的分析。1回限りではありません。
2. 対応するコードフラグメントの作成者によるアナライザー警告の表示

この記事をお楽しみください。 C / C ++およびC＃の興味深いオープンプロジェクトのチェックで読者を喜ばせ続けることをお約束します。

ご清聴ありがとうございました。 そして新年のあなたのためのコード！


英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください：Svyatoslav Razmyslov。 .NETコアライブラリ（CoreFX）のクリスマス分析 。