ブレークおよびフォールスルーステートメント

Chromiumプロジェクトで見つかったエラーの例について、高品質のコードを作成するための推奨事項に関する一連の記事をご紹介します。 これは2番目の部分で、switchステートメント、または忘れられたbreakステートメントの問題に当てられます。



何年もの間、プログラムのエラーを研究してきましたが、今では自信を持ってCで言うことができ、C ++ではその後、 switchステートメントが間違って作成されています。 breakを記述しないことで制御を渡すことができるため、洗練されたアルゴリズムを記述できることを理解しています。 しかし、それでも、膨大な数のエラーにより、間違ったアプローチが選択されたと確信しました。 今では遅すぎることは明らかです。 正しい解決策は、たとえばbreakthroughのようにワードブレークまたは反対のキーワードを記述することです。 どんなに労力、時間、お金が節約されても。 もちろん、この欠陥はNull References：The Billion Dollar Mistakeと比較することはできませんが、それでも大きな失敗です。



さて、哲学的で、それで十分です。 C ++言語はそのままです。 ただし、これはリラックスしてコードの品質と信頼性を向上させるために何もしないという意味ではありません。 「休憩ミス」問題は大きな問題であり、過小評価すべきではありません。 高品質のChromiumプロジェクトでも、このタイプのエラーは隠されています。



PVS-Studioによって発行されたレポートの解析中に私が気付いたことを見てみましょう。 入門記事で書いたように、レポートを非常に流fluentに見たので、気づいていない他のエラーがあるかもしれません。 それでも、見つかったエラーは、これらすべてが個別のランダムなエラーではなく、安定したエラーパターンであることを示すのに十分です。 読者はこのパターンを真剣に受け止め、それを防ぐための対策を講じる必要があります。



最初のエラーは、Chromiumプロジェクトコードから直接取得されます。

int GetFieldTypeGroupMetric(....) { .... switch (AutofillType(field_type).group()) { .... case ADDRESS_HOME_LINE3: group = GROUP_ADDRESS_LINE_3; break; case ADDRESS_HOME_STREET_ADDRESS: group = GROUP_STREET_ADDRESS; case ADDRESS_HOME_CITY: group = GROUP_ADDRESS_CITY; break; case ADDRESS_HOME_STATE: group = GROUP_ADDRESS_STATE; break; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

特定の番地フィールドまたは市区町村フィールドに自動的に入力する必要があるかどうかに関係なく、 GROUP_ADDRESS_CITY定数が選択されます。 すなわち どこかで、通りの名前の代わりに、都市名が自動的に置き換えられます。



理由は、 breakステートメントが欠落しているためです。 その結果、割り当て後：

 group = GROUP_STREET_ADDRESS;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

グループ変数にはすぐに新しい値が割り当てられます。

 group = GROUP_ADDRESS_CITY;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーは、この二重の割り当てに気付き、警告を発行します。V519 「グループ」変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認してください：145、147。autofill_metrics.cc 147



2番目のエラーはChromiumコードにも適用され、同様に見えます。

 void GLES2Util::GetColorFormatComponentSizes(...., int* a) { .... // Sized formats. switch (internal_format) { case GL_ALPHA8_EXT: *a = 8; case GL_ALPHA16F_EXT: *a = 16; case GL_ALPHA32F_EXT: *a = 32; case GL_RGB8_OES: case GL_SRGB8: case GL_RGB8_SNORM: case GL_RGB8UI: case GL_RGB8I: *r = 8; *g = 8; *b = 8; break; case GL_RGB565: .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ここでは、2つまたは3つのbreakステートメントを忘れています。 このコードがどのように機能するか正確にはわからないので、エラーの修正方法についてコメントすることは控えます。 PVS-Studioアナライザーは、このコードに対して2つの警告を生成します。

• V519 CWE-563「* a」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：1385、1387。gles2_cmd_utils.cc 1387
• V519 CWE-563「* a」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認：1387、1389。gles2_cmd_utils.cc 1389

Chromiumコードからの3番目のエラー。

 gfx::ColorSpace VideoColorSpace::ToGfxColorSpace() const { .... switch (primaries) { .... case PrimaryID::SMPTEST431_2: primary_id = gfx::ColorSpace::PrimaryID::SMPTEST431_2; break; case PrimaryID::SMPTEST432_1: primary_id = gfx::ColorSpace::PrimaryID::SMPTEST432_1; case PrimaryID::EBU_3213_E: primary_id = gfx::ColorSpace::PrimaryID::INVALID; break; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

前とまったく同じ写真。 PVS-Studio警告：V519 CWE-563 'primary_id'変数には値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認：106、109。video_color_space.cc 109



Chromiumコードの4番目のエラー。 今回は、V519ではなく、V796の警告が役立ちます。 V519診断は、再割り当てに気づいたときに、欠落しているブレークを間接的に検出します。 Diagnostics V796は 、欠落しているbreakステートメントを見つけるように特別に設計されています。

 void RecordContextLost(ContextType type, CommandBufferContextLostReason reason) { switch (type) { .... case MEDIA_CONTEXT: UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.Media", reason, CONTEXT_LOST_REASON_MAX_ENUM); break; case MUS_CLIENT_CONTEXT: UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.MusClient", reason, CONTEXT_LOST_REASON_MAX_ENUM); break; case UI_COMPOSITOR_CONTEXT: UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.UICompositor", reason, CONTEXT_LOST_REASON_MAX_ENUM); case CONTEXT_TYPE_UNKNOWN: UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.Unknown", reason, CONTEXT_LOST_REASON_MAX_ENUM); break; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

「UI_COMPOSITOR_CONTEXT」ブランチの実行後、制御は「CONTEXT_TYPE_UNKNOWN」ブランチに転送されます。 どうやら、これはどこか間違った処理につながる...しかし、私はそれが何に影響するかわかりません。 しかし、ここでの休憩は、明らかに偶然ではなく、意図的にスキップされています。



PVS-Studio警告：V796 CWE-484 switchステートメントに「break」ステートメントが欠落している可能性があります。 command_buffer_metrics.cc 125



Chromiumコードの5番目のエラー。これにより、マウスの中ボタンが影響を受けます。

 void SystemInputInjectorMus::InjectMouseButton( ui::EventFlags button, bool down) { .... int modifier = ui::MODIFIER_NONE; switch (button) { case ui::EF_LEFT_MOUSE_BUTTON: modifier = ui::MODIFIER_LEFT_MOUSE_BUTTON; break; case ui::EF_RIGHT_MOUSE_BUTTON: modifier = ui::MODIFIER_RIGHT_MOUSE_BUTTON; break; case ui::EF_MIDDLE_MOUSE_BUTTON: modifier = ui::MODIFIER_MIDDLE_MOUSE_BUTTON; default: LOG(WARNING) << "Invalid flag: " << button << " for the button parameter"; return; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

中央のマウスボタンのクリックが誤って処理されました。 正しいアクションの後：

 modifier = ui::MODIFIER_MIDDLE_MOUSE_BUTTON;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーフラグハンドラーへの移行があり、関数はスケジュールより早く作業を完了します。



PVS-Studio警告：V796 CWE-484 switchステートメントに「break」ステートメントが欠落している可能性があります。 system_input_injector_mus.cc 78



ここでは、読者は「すべてが明確で十分です！」と言う権利があります。 ただし、使用されているライブラリでこれらのエラーをさらにいくつか見つけたので、それらを見てみましょう。 この種のエラーはどこにでもあることを納得できるように示したいと思います。



6番目のエラーは、Chromiumで使用されるAngleライブラリのコードにあります。

 void State::getIntegerv(const Context *context, GLenum pname, GLint *params) { .... switch (pname) { .... case GL_DEBUG_GROUP_STACK_DEPTH: *params = static_cast<GLint>(mDebug.getGroupStackDepth()); break; case GL_MULTISAMPLE_EXT: *params = static_cast<GLint>(mMultiSampling); break; case GL_SAMPLE_ALPHA_TO_ONE_EXT: *params = static_cast<GLint>(mSampleAlphaToOne); // <= case GL_COVERAGE_MODULATION_CHROMIUM: *params = static_cast<GLint>(mCoverageModulation); break; case GL_ATOMIC_COUNTER_BUFFER_BINDING: .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V519 CWE-563「* params」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：2044、2046。state.cpp 2046



7番目のエラーは、Chromiumで使用されるSwiftShaderライブラリのコードに存在します。

 GL_APICALL void GL_APIENTRY glInvalidateSubFramebuffer(....) { .... switch(target) { case GL_DRAW_FRAMEBUFFER: case GL_FRAMEBUFFER: framebuffer = context->getDrawFramebuffer(); case GL_READ_FRAMEBUFFER: framebuffer = context->getReadFramebuffer(); break; default: return error(GL_INVALID_ENUM); } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V519 CWE-563「framebuffer」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認：3879、3881。libglesv3.cpp 3881



7は美しい数字です。 その上で停止します。 他のエラーがあるかもしれませんが、私はそれらをChromiumとライブラリの作者に任せます。 V519の警告を注意深く見るのは退屈でした。 Diagnostics V519は、不正確なコード記述またはマクロに関連する多くの愚かな誤検知を提供します。 そして、このような大規模プロジェクト用のアナライザーのセットアップは、すでに支払いが必要な仕事です（はい、それはGoogleにとって微妙なヒントでした）。



それで、例を見て終わりました。そして、議論されているエラーパターンから自分自身を守る方法について話をする時です。



提言



私が最初に書いたように、私の意見では、そのようなエラーの理由は、言語構文の実装の失敗です。 そして、何かを変えるには遅すぎます。 ただし、コンパイラーとアナライザーはこの問題を徐々に解決します。 breakステートメントが忘れられていることを伝える警告が長い間ありました。 そして、さらに制御を移す必要がある場合は、次のような特別な魔法の助けを借りて、コンパイラーとアナライザーにこのことを通知します。

• [[gnu :: fallthrough]];
• [[clang :: fallthrough]];
• __attribute __（（フォールスルー））;
• BOOST_FALLTHROUGH;
• などなど。

残念ながら、これはすべて何らかの理由で普遍的ではありませんでした。 幸いなことに、私はすべてのC ++プログラマに朗報があります。 C ++ 17標準は、プログラマーがさらに制御を移すことを具体的に計画していることをコンパイラーに伝えるための標準的な方法を最終的に承認しました。 これは[[fallthrough]]属性です。 もちろん、アナライザーもこのヒントを使用します。 ところで、この標準の新機能に関する記事「 C ++ 17 」を参照することをお勧めします。



属性[[fallthrough]]についてもう少し。



この属性は、 caseブロック内のbreakステートメントが意図的に欠落している（つまり、制御が次のcaseブロックに転送される）ため、コンパイラーまたは静的コードアナライザーに対応する警告を発行しないことを示します。



使用例：

 switch (i) { case 10: f1(); break; case 20: f2(); [[fallthrough]]; //    case 30: f3(); break; case 40: f4(); break; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

すでにC ++ 17に切り替えている場合は、 [[fallthrough]]を使用しない理由はありません。 コンパイラーで警告をオンにして、 休憩の失敗について通知します。 そして、breakステートメントが本当に必要でない場合は、 [[fallthrough]]を書きます。 また、会社で使用されているコーディング標準でこれらすべてを記述することをお勧めします。



ClangコンパイラとGCCコンパイラは、フラグを指定すると、 ブレークの失敗について警告し始めます。

 -Wimplicit-fallthrough
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

[[fallthrough]]を追加すると、警告は消えます。



MSVCはより困難です。 Visual C ++ 2017 RTM以降、/ W4フラグが指定されている場合、警告C4468を発行するようです。 詳細： コンパイラのバージョンごとのコンパイラの警告 （C4468を参照）。 しかし、私は最新バージョンのVisual Studioを持っていますが、すべての最新の更新は頑固にサイレントです。 しかし、私は長い間実験をせず、おそらく何か間違ったことをしました。 いずれにせよ、現在ではない場合、近い将来、このメカニズムはVisual C ++で動作します。



ご清聴ありがとうございました。 すべてのコードレスコードをお祈りします。 はい。PVS-Studioで作業プロジェクトを確認することを忘れないでください。





この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 ブレークアンドフォールスルー 。