🥒 ✝️ 🧒🏾 2018年のC ++プロジェクトのトップ10エラー 🕯️ 💛 🤲🏿

2018年が終わってからすでに3か月が経過しています。多くの人にとって、それはほとんど気付かれずに飛びましたが、PVS-Studioの開発者である私たちにとっては、非常に飽和していることが判明しました。私たちは、大衆への静的解析の進歩のために大胆に戦い、大胆に戦い、C、C ++、C＃、およびJavaで書かれたオープンソースプロジェクトの新しいエラーを探しました。この記事で収集した10個の最も興味深いものです！

PVS-Studio静的コードアナライザーを使用して、興味深い場所を検索しました。上記の言語で記述されたコードのエラーおよび潜在的な脆弱性を検出できます。

自分でエラーを探すことに興味がある場合は、いつでも当社のアナライザーをダウンロードして試すことができます。学生や熱心なプログラマー向けのアナライザーの無料版、オープンソースプロジェクトの開発者向けの無料ライセンス、およびすべてすべての試用版を提供しています。おそらく、来年までにトップ10を獲得できるでしょうか？ :)

注：読者が自分で確認し、アナライザーの警告を見る前に、異常を自分で特定することをお勧めします。いくつの間違いを見つけることができますか？

10位

出典：そして再び宇宙へ：ユニコーンステラリウムが訪れた方法

このエラーは、Stellarium仮想プラネタリウムの確認中に発見されました。

上記のコードスニペットは、小さなものですが、ややトリッキーなエラーを含んでいます。

Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }

見つけた？

PVS-Studio警告： V603オブジェクトは作成されましたが、使用されていません。コンストラクタを呼び出す場合は、「this-> Plane :: Plane（....）」を使用する必要があります。 Plane.cpp 29

コードの作成者は、メインコンストラクターに埋め込まれた別のコンストラクターを使用して、オブジェクトのフィールドの一部を初期化することを望んでいました。確かに、彼は可視領域を離れたときに破壊される一時オブジェクトのみを作成できました。したがって、オブジェクトのいくつかのフィールドは初期化されません。

ネストされたコンストラクター呼び出しの代わりに、C ++ 11で導入された委任コンストラクターを使用する必要があります。たとえば、これを行うことができます：

 Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }

その後、すべての必須フィールドが正しく初期化されます。それは素晴らしいことではありませんか？

9位

ソース： Perl 5：マクロがエラーを隠す方法

9番目は、Perl 5ソースコードからの注目すべきマクロです。

同僚のSvyatoslavは、記事を書くためのエラーを収集して、マクロの使用に関してアナライザーによって発行された警告に遭遇しました。ここにあります：

 PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... }

問題が何であるかを知るために、スヴィアトスラフはさらに掘り下げました。彼はマクロの定義を開き、さらにいくつかのネストされたマクロが含まれていることを確認しましたが、その一部にはネストされたマクロも含まれていました。理解するのが非常に困難であったため、前処理済みのファイルを使用する必要がありました。しかし、悲しいかな、これは役に立ちませんでした。前のコード行の代わりに、Svyatoslavはこれを発見しました。

 (((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40));

警告PVS-Studio： V502 「？：」演算子は、予想とは異なる方法で動作する可能性があります。「？：」演算子の優先順位は、「&&」演算子よりも低くなっています。 pp_hot.c 3036

私の目でそのようなエラーを見つけるのは難しいと思います。正直なところ、私たちはこのコードを長い間黙想し、実際にここに間違いはないという結論に達しました。しかし、いずれにしても、これは読みにくいコードのかなり面白い例です。

マクロは悪であると言われています。もちろん、それらが不可欠であることが判明する場合もありますが、マクロを関数に置き換えることができる場合は、必ず実行する必要があります。

ネストされたマクロは特に困ります。理解が難しいだけでなく、予測できない結果をもたらす可能性もあります。マクロの作成者が誤ってそのようなマクロを間違えた場合、関数内で見つけるよりもはるかに困難になります。

8位

出典： Chromium：その他のエラー

次の例は、Chromiumプロジェクトの分析に関する一連の記事から引用したものです。彼女はWebRTCライブラリで自身をカバーしました。

 std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; }

PVS-Studio警告： V789 CWE-672範囲ベースのforループで使用される「formats」コンテナのイテレータは、「push_back」関数の呼び出し時に無効になります。 stereocodecfactory.cc 89

エラーは、範囲ベースのforループ内でフォーマットベクトルのサイズが変化することです。範囲ベースのループはイテレーターに基づいているため、このようなループ内のコンテナーのサイズを変更すると、これらのイテレーターが無効になる可能性があります。

明示的な反復子を使用してループを書き換えると、このエラーが続きます。したがって、明確にするために、次のコードを使用できます。

 for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } }

たとえば、 push_backメソッドを使用すると、ベクトルが解放される場合があり、イテレータは無効なメモリ領域を指します。

このようなエラーを回避するには、ルールを順守する必要があります。ループ内でコンテナのサイズを変更しないでください。条件はこのコンテナに関連付けられています。これは、範囲ベースのループおよび反復子を使用するループに適用されます。 StackOverflowのディスカッションで、どの操作がイテレータの無効化につながるかについて読むことができます。

7位

出典： Godot：静的コードアナライザーの定期的な使用

ビデオゲーム業界の最初の例は、Godotゲームエンジンで発見したコードのスニペットです。あなたの目でエラーを発見するために汗をかかなければならないかもしれませんが、私たちの洗練された読者はそれを処理できると確信しています：

 void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }

PVS-Studio警告： V621 CWE-835 「for」演算子の検査を検討してください。ループが誤って実行されるか、まったく実行されない可能性があります。 animation_blend_space_1d.cpp 113

サイクルの状態をさらに詳しく考えてみましょう。カウンター変数は、値blend_points_used-1で初期化されます。同時に、前の2つのチェック（ ERR_FAIL_CONDおよびif ）に基づいて、ループの実行時に、 blend_points_usedが常にp_at_indexよりも大きいことが明らかになります。したがって、ループ条件が常に真になるか、ループがまったく実行されません。

blend_points_usedが1 == p_at_indexの場合、ループは実行されません。

他のすべての場合、カウンタiはループの各反復で増加するため、チェックi> p_at_indexは常にtrueです。

サイクルは永遠に続くように見えるかもしれませんが、そうではありません。

まず、変数iの整数オーバーフローが発生しますが、これは未定義の動作です。したがって、これに頼る価値はありません。

iがunsigned int型であった場合、カウンターが可能な最大値に達した後、 i ++演算子はそれを0に変換します。この動作は標準で定義されており、「署名なしラッピング」と呼ばれます。ただし、このようなメカニズムを使用することも良い考えではないことに注意する必要があります。

そもそもそれは最初の場所でしたが、結局はまだ2つ目があります！実際には、整数オーバーフローに達することさえありません。アレイが海外に行く前。これは、アレイに割り当てられたブロック外のメモリ領域にアクセスしようとすることを意味します。そして、これも漠然とした行動です。古典的な例:)

このようなエラーを簡単に回避できるようにするため、いくつかの推奨事項のみを示します。

よりシンプルで直感的なコードを書く
より徹底的なコードレビューを行い、新たに記述されたコードに対してより多くのテストを記述します
静的アナライザーを使用します;）

6位

ソース： Amazon Lumberyard：The Cry of the Soul

gamedev業界の別の例、つまりAmazon Lumberyard AAAエンジンのソースコードから。

 void TranslateVariableNameByOperandType(....) { // Igor: yet another Qualcomm's special case // GLSL compiler thinks that -2147483648 is // an integer overflow which is not if (*((int*)(&psOperand->afImmediates[0])) == 2147483648) { bformata(glsl, "-2147483647-1"); } else { // Igor: this is expected to fix // paranoid compiler checks such as Qualcomm's if (*((unsigned int*)(&psOperand->afImmediates[0])) >= 2147483648) { bformata(glsl, "%d", *((int*)(&psOperand->afImmediates[0]))); } else { bformata(glsl, "%d", *((int*)(&psOperand->afImmediates[0]))); } } bcatcstr(glsl, ")"); .... }

PVS-Studio警告： V523 「then」ステートメントは「else」ステートメントと同等です。 toglsloperand.c 700

Amazon Lumberyardは、クロスプラットフォームエンジンとして開発されています。したがって、開発者はできるだけ多くのコンパイラをサポートしようとしています。プログラマーIgorは、コメントで伝えられているように、Qualcommコンパイラーに遭遇しました。

イゴールがタスクを完了し、コンパイラの「偏執狂的な」チェックに対処できたかどうかはわかりませんが、非常に奇妙なコードを残しました。 ifステートメントの thenブランチとelseブランチの両方にまったく同じコードが含まれているのは奇妙です。ほとんどの場合、このようなエラーは、粗雑なコピーペーストの結果として発生したものです。

ここで何がアドバイスできるかさえわかりません。そのため、Amazon Lumberyard開発者がバグの修正に成功し、プログラマーのイゴールに幸運を祈ります！

5位

出典：繰り返しになりますが、PVS-Studioアナライザーは人間よりも注意深いことが判明しました

次の例で興味深い話が起こりました。私の同僚のAndrei Karpovは、Qtフレームワークの次のテストに関する記事を準備していました。注目に値するエラーを書く過程で、彼はアナライザーからの警告に遭遇しましたが、それは間違っていると考えました。関連するコードスニペットと警告を次に示します。

 QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing) // <= V616 .... }

PVS-Studio警告： V616 CWE-480値が0の「CursorShowing」という名前の定数は、ビット単位の操作で使用されます。 qwindowscursor.cpp 669

つまり、PVS-Studioは、明らかにエラーがなかった場所で誓いました！ CursorShowing定数が0であることはできません。文字通り、その上の2、3行は1に初期化されるからです。

検証に不安定なバージョンのアナライザーが使用されたため、Andreiは警告の正確さを疑っていました。彼はこのコードのセクションを数回注意深く調べましたが、それでもエラーは見つかりませんでした。その結果、他の同僚が状況を修正できるように、彼はこの誤検知をバグトラッカーに書きました。

そして、詳細な分析によってのみ、PVS-Studioが再び人間よりも注意深いことが明らかになりました。値0x1は名前付き定数cursorShowingに割り当てられ、ビット定数演算「and」には名前付き定数CursorShowingが含まれます。最初は小文字で始まり、2番目は大文字で始まるため、これらは完全に異なる定数です。

QWindowsCursorクラスには実際にその名前の定数が含まれているため、コードは正常にコンパイルされます。彼女の定義は次のとおりです。

 class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... }

名前付き列挙型定数を明示的に割り当てない場合、デフォルトで初期化されます。 CursorShowingは列挙の最初の要素であるため、 0に設定されます。

このような間違いを防ぐために、エンティティにあまりにも似た名前を付けないでください。これらのエンティティが同じタイプであるか、暗黙的に相互にキャストできる場合は、特にこのルールに従う必要があります。実際、このような場合、目でエラーを検出することは事実上不可能であり、正しくないコードは正常にコンパイルされ、プロジェクト内で問題なく動作します。

4位

ソース：入力データを処理して、足で撮影します

トップ3のファイナリストに近づいていますが、FreeSWITCHプロジェクトのエラーは次々に発生します。

 static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */ break; } .... }

PVS-Studio警告： V1010 CWE-20未チェックの汚染データがインデックスで使用されています： 'strlen（command_buf）'。

アナライザーは、式strlen（command_buf）-1が未検証データを使用することを警告します。そして実際：C言語文字列（単一の文字-'\ 0'を含む）の観点からcommand_bufが空であることが判明した場合、 strlen（command_buf）は0を返します。この場合、 command_buf [-1]が呼び出され 、未定義の動作を表します。トラブル！

この間違いの正体は、それがなぜ起こるかでさえなく、 どのように起こるかです。このエラーは、自分で「タッチ」して再現できる楽しい例の1つです。 FreeSwitchを起動し、上記のコードセクションの実行につながるアクションを実行し、プログラムに入力用の空行を渡すことができます。

その結果、手首を軽くたたくと、動作中のプログラムが動作しないプログラムになります（いいえ、エレガントではありません）。このエラーを再現する方法の詳細は、上記のリンクのソース記事に記載されていますが、今のところは明確な結果を示します。

入力は何でも構いませんので、常に確認する必要があります。そうすれば、アナライザーは誓わず、プログラムの信頼性が高まります。

勝者に対処する時が来ました：決勝戦に移ります！

3位

出典： NCBI Genome Workbench：絶滅危Research研究

3人の受賞者は、NCBI Genome Workbenchプロジェクトのコード（遺伝データの研究と分析のためのツールセット）によって開かれます。ここで間違いを見つけるために遺伝子組み換えスーパーマンである必要はありませんが、かなりの数がこの可能性を認識しています。

 /** * Crypt a given password using schema required for NTLMv1 authentication * @param passwd clear text domain password * @param challenge challenge data given by server * @param flags NTLM flags from server side * @param answer buffer where to store crypted password */ void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } }

PVS-Studioの警告：

V597コンパイラーは、「ハッシュ」バッファーをフラッシュするために使用される「memset」関数呼び出しを削除できました。プライベートデータを消去するには、memset_s（）関数を使用する必要があります。 challenge.c 365
V597コンパイラは、「passwd_buf」バッファーのフラッシュに使用される「memset」関数呼び出しを削除できました。プライベートデータを消去するには、memset_s（）関数を使用する必要があります。 challenge.c 366

なんとか間違いを見つけましたか？もしそうなら、あなたは-よくやった！..まあ、またはまだ遺伝子組み換えスーパーマン。

実際、現代の最適化コンパイラは、アセンブルされたプログラムをより高速に動作させるために多くのことを行うことができます。特に、コンパイラは、 memsetに渡されたバッファが他のどこでも使用されていないことを追跡できます。

この場合、「不必要な」 memset呼び出しを削除し、削除するすべての権利を得ることができます。次に、重要なデータを保存するバッファをメモリに残して、攻撃者を喜ばせます。

このような背景に対して、文学的解説「安全性を身に付けて、つまらない」はさらに面白く見えます。このプロジェクトのために発行された警告の数が非常に少ないことから判断すると、開発者は注意を払って安全なコードを書くために非常に懸命に努力しました。ただし、ご覧のとおり、このセキュリティの欠陥をスキップすることは非常に簡単です。 Common Weakness Enumerationによると、欠陥はCWE-14に分類されます：バッファをクリアするコードのコンパイラ削除。

メモリを安全にクリアするには、 memset_s（）関数を使用します。 memset（）よりも安全であるだけでなく、コンパイラによって「無視」することもできません。

2位

出典： PVS-Studioがどのように3人半のプログラマーよりも注意を払ったのか

このトップの銀メダリストがお客様から送られてきました。彼は、アナライザーが誤った警告を発したことを確信していました。

ユージーンは手紙を受け取り、簡単にスキャンしてスヴィアトスラフに送った。 Svyatoslavは、クライアントから送信されたコードの一部を思慮深く見て、「アナライザーはあからさまに間違っているのではないか」と考えました。したがって、彼はアンドレイと相談に行きました。彼はまた、サイトをチェックして決定しました。実際、アナライザーは偽陽性を出します。

あなたは何をすることができます、あなたはそれを修正する必要があります。そして、Svyatoslavがタスクをバグトラッカーとして形式化するための合成例を作成し始めたときのみ、彼は何が起こっているのかを理解しました。

実際、コードにはエラーが存在していましたが、プログラマーがエラーを検出することはできませんでした。正直なところ、この記事の著者も成功しませんでした。

そしてこれは、分析装置が間違った場所に対して明確に警告を発したという事実にもかかわらずです！

そのようなcraftな間違いを見つけることができますか？警戒と注意力を確認してください。

PVS-Studio警告：

V560条件式の一部は常に偽です：（ch> = 0x0FF21）。 decodew.cpp 525
V560条件式の一部は常に真です：（ch <= 0x0FF3A）。 decodew.cpp 525
V560条件式の一部は常に偽です：（ch> = 0x0FF41）。 decodew.cpp 525
V560条件式の一部は常に真です：（ch <= 0x0FF5A）。 decodew.cpp 525

あなたが成功した場合-あなたは私の敬意を保持しません！

エラーは、論理否定演算子（！）が条件全体に適用されず、その最初の部分式にのみ適用されるという事実にあります。

 !((ch >= 0x0FF10) && (ch <= 0x0FF19))

この条件が満たされる場合、変数chの値は間隔[0x0FF10 ... 0x0FF19]にあります。したがって、さらに4つの比較は意味がありません。常に真または偽のいずれかになります。

このようなエラーを回避するには、いくつかのルールに従う価値があります。まず、コードをテーブルに合わせるのは非常に便利で明確です。第二に、括弧で式をオーバーロードしないでください。たとえば、このコードは次のように書き換えることができます。

 const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19) // 0..9 || (ch >= 0x0FF21 && ch <= 0x0FF3A) // A..Z || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z if (!isLetterOrDigit)

それから、第一に、ブラケットの数がはるかに少なくなり、第二に、目が犯した間違いを「キャッチ」する確率が高くなります。

そして今-チェリー：最初の場所に移動します！

一位

出典：ショック状態のシステム：伝説的なシステムショックのソースコードの興味深いエラー

したがって、今日のトップのファイナリストは、伝説的なシステムショックの間違いです！ 1994年にリリースされたこのゲームは、Dead Space、BioShock、Deus Exなどの象徴的なゲームの先駆者であり、刺激を受けました。

しかし、最初に、私は何かを認めなければなりません。これから説明する内容には、エラーは含まれていません。概して、それはコードの一部でさえありませんが、私はこれをあなたと共有しないことを拒みませんでした！

実際、ゲームのソースコードを分析する過程で、同僚のビクトリアは多くの興味深いコメントを見つけました。あちこちで、突然、遊び心と皮肉な発言、さらには詩がありました：

 // I'll give you fish, I'll give you candy, // I'll give you, everything I have in my hand // that kid from the wrong side came over my house again, // decapitated all my dolls // and if you bore me, you lose your soul to me // - "Gepetto", Belly, _Star_ // And here, ladies and gentlemen, // is a celebration of C and C++ and their untamed passion... // ================== TerrainData terrain_info; // Now the actual stuff... // ======================= // this is all outrageously horrible, as we dont know what // we really need to deal with here // And if you thought the hack for papers was bad, // wait until you see the one for datas... - X // Returns whether or not in the humble opinion of the // sound system, the sample should be politely obliterated // out of existence // it's a wonderful world, with a lot of strange men // who are standing around, and they all wearing towels

ロシア語を話す読者のために、おおよその無料翻訳を作成しました。

 //    ,    , //    ,       //      //      //     //     ,      // - "Gepetto", Belly, _Star_ //  ,   , //    C  C++     // ================== TerrainData terrain_info; //    ... // ======================= //    ,     , //         //    ,          //      ,      ... - X //       , //         //   ,     //   ,

これらのコメントは、90年代前半にゲームの開発者によって残されました...ところで、システムショックのチーフデザイナーであるダグチャーチもコードを書いていました。これらのコメントのいずれかが彼によって個人的に書かれたのかもしれません。タオルを着た男性について-これは彼の作品ではないことを願っています:)

おわりに

結論として、新しいエラーを探し、それらについての記事を書いてくれた同僚に感謝したいと思います。みんなありがとう！あなたがいなければ、この記事はそれほど面白くなかったでしょう。

また、私たちの業績についても少しお話したいと思います。なぜなら、1年間、私たちは単なるミスを探す以上のことに取り組んできたからです。また、アナライザーを開発および改善し、その結果、大幅な変更が行われました。

たとえば、いくつかの新しいコンパイラのサポートを追加し、診断ルールのリストを拡張しました。また、MISRA CおよびMISRA C ++標準の初期サポートも提供しました。最も重要で時間のかかるイノベーションは、新しい言語のサポートでした。はい、Javaコードを分析できるようになりました！そして、アイコンを更新しました :)

読者にも感謝したい。記事を読んで書いてくれてありがとう！あなたのフィードバックは非常に心地よく、私たちにとって重要です。

これにより、2018年の上位10個のC ++エラーが終了しました。一番好きだった場所とその理由は？2018年に興味深い例に出会いましたか？コメントでそれを教えてください！

次回まで！

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：George Gribkov。2018年に見つかったC ++プロジェクトの上位10個のバグ

2018年のC ++プロジェクトのトップ10エラー

10位

9位

8位

7位

6位

5位

4位

3位

2位

一位

おわりに

More articles: