PVS-StudioでIronPythonとIronRubyを確認する

ごく最近、C＃プロジェクトのチェックをサポートするPVS-Studioアナライザーの新しいバージョンをリリースしました。 製品の開発はリリース時点で中断されましたが、アナライザーのテストに従事していました。 私の実験のプロジェクトとして、IronPythonとIronRubyを使いました。 そして、これらのプロジェクトがチェックされたので、小さな記事レポートを書くことにしました。

IronPythonおよびIronRuby

IronPythonとIronRubyは、両方とも.NETとPythonおよびRubyプログラミング言語のPython実装です。 これらのプロジェクトのソースコードは、このリンクの GitHubで入手できます。 DLRソースコードも含まれています。 .NET Framework 4.0以降、DLRはその一部であり、IronPythonとIronRubyが使用しています。 それにもかかわらず、彼女がいたので、私はまだ古いバージョンのDLRをチェックしました。

プロジェクトの検証について

したがって、コード全体は、DLR、IronPython、IronRubyの3つの大きな部分で構成され、1630 * .csファイルが含まれています。 検証のために、私たちのウェブサイトからダウンロードできるPVS-Studio 6.00を使用しました。 決定の検証には1分以上かかりました。 その結果、分析装置は、最初の34個の警告、2番目の15個の警告、および3番目のレベルの280個の警告を発行しました。



最初のレベルでは、34の警告のうち、19が本当の間違いであることが判明しました-かなり良い結果で、6箇所が疑わしいと思われます-あなたはそれらに注意を払う必要があります。 残りの9つのメッセージは誤検知であり、そのうちの半分はアナライザー自体を編集することで除去できますが、近いうちに行います。



2番目と3番目のレベルのエラーと疑わしい場所は、大幅に少なくなりました。

見つかったバグ

PVS-Studioで見つかった実際のエラーの例を見てみましょう。



例1および2。不注意。

private bool Enter(RangeExpression/*!*/ node, bool isCondition) { .... if (!isCondition && litBegin != null && litEnd != null && litBegin.Value is int && litBegin.Value is int) { _result = MakeNode(NodeKind.lit, new Range( (int)litBegin.Value, (int)litEnd.Value, node.IsExclusive)); } else { .... } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3001 「&&」演算子の左右には、「litBegin.Value is int」と同じ副次式があります。 IronRubyParseTreeOps.cs 277



条件は、litEnd.Valueもチェックするのではなく、litBegin.Valueがint型であることを二重にチェックします。



同じ式の同様のチェックが、たとえば次の2つの場所にあります。

 private static PythonTuple ReduceProtocol2( CodeContext/*!*/ context, object self) { .... if (self is PythonDictionary || self is PythonDictionary) { dictIterator = PythonOps.Invoke(context, self, "iteritems", ArrayUtils.EmptyObjects); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3001 「||」の左右に「self is PythonDictionary」と同じ副次式があります 演算子。 IronPython ObjectOps.cs 452



例3.同じ式。

 protected override MSAst.Expression VisitTry( MSAst.TryExpression node) { .... if (newHandlers != null || newFinally != null) { node = Ast.MakeTry(node.Type, node.Body, newFinally != null ? newFinally : node.Finally, node.Fault, newHandlers != null ? newHandlers : newHandlers ); } return node; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio： V3012 「？：」演算子は、その条件式に関係なく、常に1つの同じ値newHandlersを返します。 DebugInfoRewriter.cs 252



ここでは、条件式の両方のブランチでnewHandlersが使用されています。 newHandlersがnullの場合、node.Handlersを使用することになっていた。



例4および5。不注意。

 public static bool HasValue(RubyContext/*!*/ context, object/*!*/ self, object value) { var strValue = value as MutableString; if (value == null) { return false; } var clrStrValue = strValue.ConvertToString(); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

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



これは、「as」演算子を使用してキャストした後、キャストの結果ではなくソースオブジェクトが不注意でチェックされ、未検証の参照が使用される場合によくある間違いです。



別の同様のケースを次に示します。

 private static RubyRegex/*!*/ ConstructRubyRegexp( RubyConstructor/*!*/ ctor, Node/*!*/ node) { ScalarNode scalar = node as ScalarNode; if (node == null) { throw RubyExceptions.CreateTypeError( "Can only create regex from scalar node"); } Match match = _regexPattern.Match(scalar.Value); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

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



例6.コピーペースト。

 private void LoadNewObj(CodeContext/*!*/ context) { PythonTuple args = PopStack() as PythonTuple; if (args == null) { throw PythonOps.TypeError("expected second argument, got {0}", DynamicHelpers.GetPythonType(args)); } PythonType cls = PopStack() as PythonType; if (args == null) { throw PythonOps.TypeError("expected first argument, got {0}", DynamicHelpers.GetPythonType(args)); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3021同一の条件式を持つ2つの「if」ステートメントがあります。 最初の「if」ステートメントにはメソッドの戻り値が含まれます。 これは、2番目の「if」ステートメントが無意味であることを意味します。 cPickle.cs 2194



上記のコードスニペットでは、2つの条件とGetPythonType（）関数の呼び出しはまったく同じです。 明らかに、2番目の条件は最初の条件をコピーすることによって取得されましたが、著者は変数の名前を変更するのを忘れていました。 プロジェクトには、同様の状況がいくつかあります。



例7.同じ条件。

 public static int Compare(SourceLocation left, SourceLocation right) { if (left < right) return -1; if (right > left) return 1; return 0; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3021同一の条件式を持つ2つの「if」ステートメントがあります。 最初の「if」ステートメントにはメソッドの戻り値が含まれます。 これは、2番目の「if」ステートメントが無意味であることを意味します。 SourceLocation.cs 156



この方法は非常に簡単で、間違いを犯す場所はないようです。 それでも、2番目の条件では、何らかの理由で左右のパラメーターが交換されたため、アナライザーが検出したものと同じものが両方の条件でチェックされます。



コードの正しいバージョン：

 public static int Compare(SourceLocation left, SourceLocation right) { if (left < right) return -1; if (left > right) return 1; return 0; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

例8.過度の状態。

 private void WriteSingleQuoted(string text, bool split) { .... while (ending <= text.Length) { c = '\0'; if (ending < text.Length) { c = text[ending]; } if (spaces) { if (c == 0 || c != 32) { .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3023 'c == 0の検査を検討してください|| c！= 32 '式。 表現が過剰であるか、誤植が含まれています。 Emitter.cs 308



まず、変数「c」にデフォルト値「\ 0」が割り当てられます。 次に、文字列全体がまだ処理されていない場合、「c」は次の文字の値を取得します。 そして最後に、デフォルト値が変数「c」に残っているかどうか、またはスペース以外のものがそこに読み込まれたかどうかがチェックされます。 実際、ゼロはすでに32（スペースコード）とは異なるため、ゼロとの比較はここでは不要です。 このコードはエラーにつながることはありませんが、理解するのが難しくなるため、ゼロとの比較は破棄される可能性があります。 アナライザーは、このプロジェクトでさらにいくつかの同様の追加チェックを見つけました。



例9および10。無効なフォーマット文字列。



String.Format関数を使用する際の一般的な問題は、String.Formatに渡されるパラメーターの数について、フォーマット文字列の数とパラメーター番号がコンパイラーによってチェックされないことです。 その結果、誤った文字列が生成されるか、FormatExceptionがスローされます。 いくつかの例を見てみましょう。

 public T Current { get { try { return (T)enumerable.Current; } catch (InvalidCastException iex) { throw new InvalidCastException(string.Format( "Error in IEnumeratorOfTWrapper.Current. Could not cast: {0} in {0}", typeof(T).ToString(), enumerable.Current.GetType().ToString()), iex); } } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio： V3025の形式が正しくありません 。 「フォーマット」関数を呼び出すときに、異なる数のフォーマット項目が予想されます。 予想：1.現在：2. ConversionWrappers.cs 235



この例では、最後のパラメーターは使用されず、代わりに値typeof（T）.ToString（）が2回表示されます。

 private static void DumpGenericParameters( MetadataTableView genericParams, MetadataRecord owner) { foreach (GenericParamDef gp in genericParams) { _output.WriteLine(" generic parameter #{0}: {1}", gp.Index, gp.Name, gp.Attributes); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio： V3025の形式が正しくありません 。 「WriteLine」関数の呼び出し中に、異なる数のフォーマット項目が予期されます。 予想：2.現在：3. Program.cs 268



また、このコードフラグメントでは、フォーマット文字列が必要とするパラメーターよりも1つ多くのパラメーターがWriteLine関数に渡されます。



例11.アクセス後のnullの確認。

 public static MutableString ChompInPlace(....) { MutableString result = InternalChomp(self, separator); if (result.Equals(self) || result == null) { self.RequireNotFrozen(); return null; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

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



この状態では、nullチェックを交換してEqualsメソッドを呼び出す必要があります。 現在のところ、アプリケーションはNullReferenceExceptionでクラッシュする可能性があります。



例12.同期の問題。

 class DictThreadGlobalState { public int DoneCount; .... } private static void RunThreadTest(DictThreadGlobalState globalState) { .... globalState.DoneEvent.Reset(); globalState.Event.Set(); while (globalState.DoneCount != 0) { // wait for threads to get back to finish } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3032この式で待機することは、コンパイラがいくつかの変数を最適化する可能性があるため、信頼できません。 これを回避するには、揮発性変数または同期プリミティブを使用します。 EngineTest.cs 2558



このコードには、常に発生するとは限らないエラーが含まれていますが、ランタイム、.NET Frameworkのバージョン、システム内のプロセッサ数、またはその他の実装の詳細によって異なります。 このようなエラーを見つけるのは非常に困難です。 その理由は、DoneCount変数がvolatile宣言されていない場合、コンパイラーはそれが1つのスレッドでのみ使用されていると考えているためです。たとえば、この変数はループ内で変更されないので、キャッシュから常に送信および送信できます。 しかし、私たちの場合、その値は別のスレッドで変化するため、そのような場合、変数を使用してスレッドを同期する場合、volatileとして宣言する必要があります。 詳細については、 MSDNをご覧ください。



例13.二重割り当て

 private static Dictionary<string, EncodingInfoWrapper> MakeCodecsDict() { .... switch (normalizedName) { case "iso_8859_1": d["8859"] = d["latin_1"] = d["latin1"] = d["iso 8859_1"] = d["iso8859_1"] = d["cp819"] = d["819"] = d["latin"] = d["latin1"] = d["l1"] = encs[i]; break; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3005 'd ["latin1"]'変数はそれ自体に割り当てられます。 StringOps.cs 1905



このコードは、値を変数d ["latin1"]に2回割り当てます。 ほとんどの場合、これは単なるコードであり、エラーではありません。 しかし、彼らはいくつかのコードページを忘れたのかもしれません。 いずれにせよ、一見の価値があります。



例14.符号なし変数とゼロの比較

 public static int __hash__(UInt64 x) { int total = unchecked((int) (((uint)x) + (uint)(x >> 32))); if (x < 0) { return unchecked(-total); } return total; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio 警告 ： V3022式 'x <0'は常にfalseです。 符号なしの型の値は、常に0以上です。IntOps.Generated.cs1967



おそらく、「x」をゼロと比較するのではなく、「合計」を比較する必要があったのでしょう。「x」を使用して何らかのアクションを実行し、特殊なケースをチェックするのは奇妙だからです。 はい。「合計」には符号付きタイプがあるため、「合計<0」という比較はより論理的に見えます。



例15.同一のチェック。

 public void ReflectTypes(Type[]/*!*/ allTypes) { .... def.Super = null; if (cls != null && def.Extends != typeof(BasicObject) && !def.Extends.IsInterface) { if (cls != null && cls.Inherits != null) { def.Super = new TypeRef(cls.Inherits); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3030定期チェック。 'cls！= Null'条件は、373行目で既に検証されています。LibraryDef.cs 374



ここでは、両方の条件で、変数「cls」のヌルがチェックされます。 ほとんどの場合、著者は最初の条件で 'def'のnullをチェックしたかったのは、そこですぐにExtendsプロパティにアクセスするためです。 しかし、条件の直前にnullが「def.Super」に書き込まれるため、これも必要ではありません。つまり、「def」はもはやnullではありません。 一般に、これは単なる追加のチェックです。



例16.コピーと貼り付け。



私はわずか280個の第3レベルのエラーに到達しました。 これらの大部分は、2つの関数の本体が一致するという警告と、浮動小数点数の比較に関する警告です。 私はここで何か深刻なものを見つけることができるとは思わなかったので、エラーをざっと調べ始めましたが、それでもそれを見つけました。

 public static bool IsPositiveOne(BigDecimal x) { return IsOne(x) && IsPositive(x); } public static bool IsNegativeOne(BigDecimal x) { return IsOne(x) && IsPositive(x); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3013 「IsPositiveOne」関数の本体が「IsNegativeOne」関数の本体と完全に同等であることは奇妙です（351、行355）。 BigDecimal.cs 351



これは本当に本当の間違いであり、コードをある関数から別の関数にコピーした結果です。 コードの正しいバージョンは次のようになります。

 public static bool IsNegativeOne(BigDecimal x) { return IsOne(x) && IsNegative(x); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

例17 NaNの奇妙なテスト。

 public static bool Equals(float x, float y) { if (x == y) { return !Single.IsNaN(x); } return x == y; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V3024奇妙な正確な比較：x == y。 定義された精度の比較を使用することを検討してください：Math.Abs​​（A-B）<Epsilon。 FloatOps.cs 1048



NaNの特別なチェックのポイントが何であるか理解できませんでした。 条件（x == y）が満たされる場合、NaNはそれ自体を含む他の値と等しくないため、「x」と「y」の両方がNaNと異なることを意味します。 つまり、最初の戻り値は常にtrueを返します。 NaNを確認することは不要なようです。

おわりに

プロジェクト検証の結果に基づいて、アナライザーの作業に満足しました。最初に、数十の実際のエラーが見つかったため、プロジェクトコードが改善されるからです。 そして次に、排除できるいくつかの誤検知を特定し、それによって製品を同時に改善しました。 したがって、PVS-Studioのデモバージョンをダウンロードし、コードを確認することをお勧めします。





この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：イリヤイワノフ。 PVS-Studioを使用したIronPythonおよびIronRubyの分析 。