Chromium：メモリリーク

Chromiumプロジェクトで見つかったエラーの例について、高品質のコードを作成するための推奨事項に関する一連の記事をご紹介します。 これは、メモリリーク専用の3番目の部分です。



Chromiumプロジェクトコードとそれに使用されるライブラリは非常に高品質であると思います。 はい、入門記事で約250のエラーを書きましたが、実際には-これは非常に少ない数です。 確率の法則により、巨大なプロジェクトでは多くの間違いがあります。



それにもかかわらず、メモリリークについて話をすると、それほど多くはありません。 Chromiumの開発者は、動的コードアナライザーを好むことを失望させていると思います。 もちろん、これらのツールには多くの利点があります。 たとえば、ダイナミックアナライザーがエラーを検出すると実際にエラーが発生するため、誤検知は発生しません。



一方、動的分析には弱点があります。 一部のコードが実行されない場合、エラーは検出されません。 そして、プログラマーは、コードの100％をテストでカバーすることは非常に難しいことを理解しています。実際には、これは単に不可能です。 その結果、多くのエラーがコード内に残り、それ自体を証明するための好ましい一連の状況を待ちます。



ここで、静的コードアナライザーが役立ちます。 はい、これはGoogleの開発者にとって、顧客になれば幸いになるというヒントです。 さらに、Chromiumプロジェクトの機能に合わせてPVS-Studioを適応および構成する追加作業の準備ができています。 また、発見したエラーを修正する準備ができています。 すでに同様の経験があります（ 例 ）。



しかし、メモリリークに戻ります。 後でわかるように、ほとんど制御されないコードで非表示になります。 これらは主にさまざまなエラーハンドラです。 静的アナライザーは、動的アナライザーとは異なり、割り当てられたメモリへの「ポインタの運命」を常に追跡できるわけではなく、多くのメモリリークを検出しません。 一方、静的アナライザーは、実行の可能性に関係なく、すべてのコードをチェックし、エラーを通知します。 したがって、静的アナライザーと動的アナライザーは互いに補完します。



PVS-Studioが発行したレポートの解析中に気付いたメモリリークを見てみましょう。 入門記事で書いたように、レポートを非常に流fluentに見たので、気づいていない他のエラーがあるかもしれません。 また、メモリリークはChromiumのようなプロジェクトにとって非常に不快であるため、それらについて話すのは興味深いことです。 CWEによると、これらのエラーはCWE-401として分類できます。

パート1：関数を終了する前にメモリを解放するのを忘れた

Chromiumコードのエラーを考慮してください。 まず、無効化されたメモリバッファを割り当てて返すBnNewヘルパー関数を示します。

uint32_t* BnNew() { uint32_t* result = new uint32_t[kBigIntSize]; memset(result, 0, kBigIntSize * sizeof(uint32_t)); return result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

次に、メモリリークを引き起こす可能性のあるコードを見てみましょう。

 std::string AndroidRSAPublicKey(crypto::RSAPrivateKey* key) { .... uint32_t* n = BnNew(); .... RSAPublicKey pkey; pkey.len = kRSANumWords; pkey.exponent = 65537; // Fixed public exponent pkey.n0inv = 0 - ModInverse(n0, 0x100000000LL); if (pkey.n0inv == 0) return kDummyRSAPublicKey; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

条件（pkey.n0inv == 0）が満たされると、関数はバッファーを解放せずに終了します。バッファーへのポインターは変数nに格納されます。



アナライザーは、次の警告を発行してこの欠陥を示します。V773 CWE-401「n」ポインターを解放せずに機能が終了しました。 メモリリークが発生する可能性があります。 android_rsa.cc 248



ところで、これは特にChromium自体に関連するメモリリークが終了する場所です。 しかし、使用されるライブラリにはそれらの多くがあります。 また、ユーザーは、メモリがChromiumライブラリまたはChromium自体のどちらに流れるかを気にしません。 したがって、ライブラリのエラーも重要です。



次のエラーは、WebKitエンジンに関連しています。 補助機能から再度開始する必要があります。

 static CSSValueList* CreateSpaceSeparated() { return new CSSValueList(kSpaceSeparator); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

エラーを含むコード：

 const CSSValue* CSSTransformValue::ToCSSValue(....) const { CSSValueList* transform_css_value = CSSValueList::CreateSpaceSeparated(); for (size_t i = 0; i < transform_components_.size(); i++) { const CSSValue* component = transform_components_[i]->ToCSSValue(secure_context_mode); if (!component) return nullptr; // <= transform_css_value->Append(*component); } return transform_css_value; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コンポーネントポインターがnullであることが判明した場合、関数は終了し、メモリリークが発生します。



PVS-Studioアナライザーは警告を生成します：V773 CWE-401 'transform_css_value'ポインターを解放せずに関数が終了しました。 メモリリークが発生する可能性があります。 csstransformvalue.cpp 73



WebKitに関連する他のエラーを見てみましょう。

 Request* Request::CreateRequestWithRequestOrString(....) { .... BodyStreamBuffer* temporary_body = ....; .... temporary_body = new BodyStreamBuffer(script_state, std::move(init.GetBody())); .... if (exception_state.HadException()) return nullptr; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

HadException（）関数がtrueを返す場合、関数はスケジュールよりも早く作業を完了します。 この場合、 temporary_body変数に格納されているポインターに対してdeleteオペレーターを呼び出すことはありません。



PVS-Studio警告：V773 CWE-401 'temporary_body'ポインターを解放せずに関数が終了しました。 メモリリークが発生する可能性があります。 request.cpp 381





たくさん？ たくさん。 同時に、私が力を持っていたメッセージだけがここに書き出されました。 私はすぐに退屈し、そのような警告を非常に表面的に見ました。 ほとんどの場合、レポートをより慎重に分析すると、WebKitでより多くのエラーが検出されます。



これはどういう意味ですか？ これは、WebKitプロジェクトにメモリリークの問題があることを意味し、このプロジェクトを祝福します。



それでは、ICUプロジェクトに移り、そこで見つかったエラーを見てみましょう。

 UVector* RuleBasedTimeZone::copyRules(UVector* source) { if (source == NULL) { return NULL; } UErrorCode ec = U_ZERO_ERROR; int32_t size = source->size(); UVector *rules = new UVector(size, ec); if (U_FAILURE(ec)) { return NULL; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

タイプUVectorのオブジェクトの初期化中に特定のエラーが発生した場合、これはec変数に設定されたステータスに影響します。 たとえば、必要な数の要素を格納するためのメモリバッファを割り当てることができない場合、コンストラクタはステータスU_MEMORY_ALLOCATION_ERRORを返します。 ただし、要素を格納するためのメモリを割り当てることができるかどうかに関係なく、 UVector型のオブジェクトが作成され、このオブジェクトへのポインターがルール変数に配置されます。



コンストラクターがステータスU_MEMORY_ALLOCATION_ERRORを返した場合、関数は終了します。 同時に、 UVector型のオブジェクトは削除されず、メモリリークが発生します。



PVS-Studio警告：V773 CWE-401「ルール」ポインターを解放せずに機能が終了しました。 メモリリークが発生する可能性があります。 rbtz.cpp 668





他に何に気付きましたか？







おそらく、これらはすべてのエラーとはほど遠いですが、PVS-Studioの機能を実証してこの記事を書くには十分です。

パート1：推奨事項

上記のすべてのケースを統合するものは何ですか？ そのエラーは、手動のメモリ管理により可能になりました！



友人たち、すでにC ++ 17を使用しています。 new演算子の呼び出しを停止し、結果を通常のポインターに入れてから、解放するのを忘れます。 恥ずかしい。



割り当てられたリソースの通常のポインターとそれに続く手動制御はもう必要ありません！ 常にスマートポインターを使用しましょう。



最新のC ++標準は、 unique_ptr 、 shared_ptr、 weak_ptrなどのスマートポインターを提供します。 ほとんどの場合、 unique_ptrは1つで十分です。



たとえば、次の誤ったコードに戻りましょう。

 const CSSValue* CSSTransformValue::ToCSSValue(....) const { CSSValueList* transform_css_value = CSSValueList::CreateSpaceSeparated(); for (size_t i = 0; i < transform_components_.size(); i++) { const CSSValue* component = transform_components_[i]->ToCSSValue(secure_context_mode); if (!component) return nullptr; transform_css_value->Append(*component); } return transform_css_value; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

unique_ptrを使用して書き換えましょう。 これを行うには、まず、ポインター自体のタイプを変更する必要があります。 第二に、最後に、 リリースされた関数を呼び出して、管理対象オブジェクトへのポインターを返し、それを制御する必要がなくなります。



正しいコードは次のとおりです。

 const CSSValue* CSSTransformValue::ToCSSValue(....) const { unique_ptr<CSSValueList> transform_css_value( CSSValueList::CreateSpaceSeparated()); for (size_t i = 0; i < transform_components_.size(); i++) { const CSSValue* component = transform_components_[i]->ToCSSValue(secure_context_mode); if (!component) return nullptr; transform_css_value->Append(*component); } return transform_css_value.release(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この記事では、スマートポインターの使用方法を教えるつもりはありません。 このトピックは、書籍の多くの優れた記事とセクションに当てられています。 変更によってコードがより複雑で面倒にならないことを示したかっただけです。 しかし、今では間違いを犯すのがはるかに困難になります。



new / deleteまたはmalloc / freeに対処して間違いをしないのはあなただとは思わないでください。 Chromium開発者はそのような間違いを犯します。 他の開発者はそうします 。 あなたはそのような間違いを犯します。 そして、あなたのチームが特別であるという不必要な夢を持ってはいけません:)。 この機会に、マネージャーにこのメモを今すぐ読んでもらいます 。



スマートポインターを使用します。

パート2：realloc

私の経験では、プログラマーはrealloc関数を誤用することがあります。 この関数の使用に関連する古典的なエラーパターンは次のようになります。

 p = realloc(p, n); if (!p) return ERROR;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

関数の次のプロパティに注意してください。十分なメモリがない場合、古いメモリブロックは解放されず、nullポインタが返されます。



NULLはバッファへのポインタを格納した変数pに書き込まれるため、このバッファを解放する機能は失われます。 メモリリークがあります。



正しいオプションは、たとえば次のようにコードを書き換えることです。

 void *old_p = p; p = realloc(p, n); if (!p) { free(old_p); return ERROR; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Chromiumプロジェクトで使用されるライブラリにこのようなエラーがないわけではありません。



たとえば、FLACコーデックのコードを考えてみましょう。

 FLAC__bool FLAC__format_entropy_codi.....ce_contents_ensure_size( FLAC__EntropyCodingMethod_PartitionedRiceContents *object, unsigned max_partition_order) { .... if(object->capacity_by_order < max_partition_order) { if(0 == (object->parameters = realloc(object->parameters, ....))) return false; if(0 == (object->raw_bits = realloc(object->raw_bits, ....))) return false; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

この関数は、2つのバッファーのサイズを増やします。

• オブジェクト->パラメーター
• オブジェクト-> raw_bits

メモリ割り当てエラーが発生した場合、関数はスケジュールより早く終了し、 falseを返します 。 この場合、ポインタの以前の値が失われ、メモリリークが発生します。



PVS-Studioアナライザーは、ここで2つの関連する警告を生成します。

• V701 CWE-401 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインター 'object-> parameters'が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 format.c 576
• V701 CWE-401 realloc（）リークの可能性：realloc（）がメモリの割り当てに失敗すると、元のポインタ 'object-> raw_bits'が失われます。 realloc（）を一時ポインターに割り当てることを検討してください。 format.c 578

幸い、Chromiumにはこのタイプのエラーはほとんどありません。 少なくとも、他のプロジェクトで通常見られるものよりもはるかに少ないです。

パート2：推奨事項

バッファサイズを頻繁に変更する必要があるときに効率的なコードを記述できるため、 realloc関数を常に拒否できるとは限りません。



したがって、私はそれを完全に放棄することを急いで勧めません。 時にはそれは不当になります。 この関数には注意して、上記のエラーパターンを忘れないようにしてください。



ただし、多くの場合、C ++では、この関数を完全に省き、 std :: vectorやstd :: stringなどのコンテナを使用できます 。 コンテナの効率は近年大幅に向上しています。 たとえば、PVS-Studioコアでは、自作の文字列クラスとstd :: stringの間にパフォーマンスの違いがなくなったことがわかり、驚きました。 しかし、何年も前、自作の文字列クラスにより、アナライザのパフォーマンスが約10％向上しました。 このような効果はそれ以上見られず、独自のクラスを削除することが可能になりました。 現在、 std ::文字列クラスは10年前とまったく異なります 。 最新のコンパイラー最適化機能と、たとえば移動コンストラクターなどの言語の革新のおかげで、効率が大幅に向上しました。



一般的に、急いで袖をまくり、 malloc 、 realloc 、 free関数を使用してメモリを手動で管理しないでください。 ほぼ確実に、 std :: vectorは、タスクにとってそれほど効果的ではありません。 同時に、 std :: vectorを使用する方がはるかに簡単です。 間違いを犯すことも難しくなります。 プロファイラーがこれが実際にプログラムのボトルネックの1つであることを示した場合にのみ、低レベル関数に戻ることは理にかなっています。



ご清聴ありがとうございました。 PVS-Studioアナライザーをダウンロードしてお試しください。







この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 Chromium：メモリリーク 。