アンリアル エンジン開発プロセスの一環としての静的コード解析

この記事に掲載したプログラムのコードは、PVS-Studio の Ilya Ivanov とSergei Vasilyev が提供したものです。

アンリアル エンジンは、新規コードが追加されると共に、以前記述されたコードにも変更が加えられて進化を続けています。開発中のプロジェクトの結果として必ず生じることがあります。それはコードで新たなバグが生じてしまうことです。プログラマーはできる限り早くこうした問題を特定したいことでしょう。エラー数を減らす方法のひとつとして、PVS-Studio のような静的アナライザーの使用があります。さらに、静的アナライザーは進化しているだけでなく、新たなエラー パターンを探して絶えず学習しています。その一部についてこの記事で取り上げます。コードの品質に配慮していれば、この記事がお役に立つことでしょう。

静的コード解析とその理論

静的コード解析は、プログラムのソースコードで誤りや不備を検出するプロセスです。コード レビューのプロセスを自動化したものと考えられます。

コード レビューは、昔から行われており、誤りを検出するうえで有用な方法です。ソースコードを一緒に読んで、改善方法を提案します。こうしたプロセスを通して、誤りや後で問題になりうるコード部分を検出します。コード作成者は、プログラムの特定部分がどのように機能するかについて説明しないという暗黙のルールがあります。プログラムのテキストとコードのコメントを見るだけで、アルゴリズムは明確なものでなければなりません。そうでなければ、コードを修正します。

コード レビューは概してうまくいきます。自分のものよりも他のプログラマーが作成したコードの方が誤りを見つけやすいからです。コード レビューの方法論の詳細が、『Code Complete』 (Steve McConnell 著) という素晴らしい本に記載されています。

コード レビューの方法には以下の欠点があります。

1. コストが非常に高い。数名のプログラマーをメイン タスクから外して、新たに記述されたコード、および提案に従い書き直されたコードをレビューさせるからです。しかも、プログラマーは作業中に定期的に休憩を取らなければなりません。膨大なコードに目を通していると、集中力が切れてレビュー本来の目的が損なわれるおそれがあります。

2. 新規コードや修正コードに直接関連がない箇所の誤りを見つけるのは難しいことです。新しいコードの一部を見て、ヘッダー ファイル、stdlib.h がないから malloc 関数が正しく機能しないと考えるのは容易なことではありません。この状況については、『A nice 64-bit error in C』 という記事に詳しい説明があります。もう一つの例として、ヘッダー ファイルでの関数の型や変数の変更があります。理想としては、プログラマーはこうした変更後に関数や変数が使われる場所ですべてのコードをレビューすべきです。実際は、時間がかかり過ぎるため、レビューはプログラマーが変更を加えた箇所に限定されます。

一方で、コード レビューを定期的に行いたいという希望もあります。しかし、コストがかかり過ぎます。その妥協策となるのが、この静的解析です。静的解析ツールはプログラムのソース テキストをチェックし、特定のコード部分をレビューするように提案します。静的アナライザーは人間のように疲れませんし、ヘッダー ファイルでの変更の影響を受けるすべてのコードをチェックします。もちろん、プログラムは開発者チームが行う本格的なコード レビューに代わるものではありません。しかし、費用対効果を考えると、静的解析は多くの企業が採用している非常に有用な方法です。

エラー検出の他の方法と同様に、静的解析には長所と短所があります。プログラムをテストする理想的な方法はありません。様々な手法を組み合わせることで最良の結果を得ることができます。例えば、適切なコーディング スタイル、静的コード解析、動的コード解析、単体テスト、リグレッション テストなどの手法があります。

静的解析の大きなメリットは、コードで現れたらすぐに大量のエラーを検出できる機能です。つまり、修正にあまりコストがかかりません。エラー検出が早いほど、修正コストが低くなります。以下のように、『Code Complete』 (著：McConnell) という本によれば、コードのテスト段階でのエラー検出はコードの記述段階に比べて 10 倍のコストがかかるとのことです。

誤りを修正する平均的なコストは、どの段階 (stage) でバグが検出されるかに応じて変わります (以下の表のデータは、『Code Complete』 (著：S. McConnell) から引用したものです)。

静的解析ツールを使うと、大量のエラーを検出することができます。通常は、コードを最初に記述したときに利用し、これによりプロジェクト全体の開発コストが大幅に削減されます。

静的アナライザーは、最近のアプリケーションのコードベースの絶え間ない成長によって普及してきました。プログラムはどんどん大きく複雑になっています。一方でエラーの密度は、コードサイズに応じて非線形的に上昇します。

プロジェクトが大きくなるほど、コード 1000 行あたりのエラーは増えます。以下のチャートをご覧ください。

プロジェクトの規模と典型的なエラー密度を表しています。(出典："Program Quality and Programmer Productivity" (Jones, 1977), "Estimating Software Costs" (Jones, 1998))

わかりやすいように、データをグラフにしました。

プロジェクトの典型的なエラー密度を表しています。青 - 最大エラー密度、赤 - 平均密度、緑 - 最小エラー密度を表しています。

このグラフは、プロジェクトが大きくなるにつれて望ましいコード品質を保つために多くのツールを使わざるをえないことを示しています。例えば 8 年前と同じやり方で高品質なコードを作成するのは不可能になっています。これはチームにとって気分の良いものではありません。いつもどおりコードを記述しても、コードの状態が悪くなっていくのですから。

技術の進歩と変化に伴い、昔ながらの方法を使用し続けるのは難しいため、新しい方法やツールを試す必要があります。使用してみる価値がある方法のひとつとして静的解析があります。

静的解析の方法に馴染みがない場合は、この記事をお読みになってご興味を持っていただければと思います。詳細については以下のリンクをご覧ください。

1. Static Code Analysis  (著：John Carmack)

2. Static Code Analysis  (Wikipedia)

3. List of tools for static code analysis  (Wikipedia)

4. A Few Billion Lines of Code Later:Using Static Analysis to Find Bugs in the Real World  (共著：Al Bessey, Ken Block, Ben Chelf, Andy Chou, Bryan Fulton, Seth Hallem, Charles Henri-Gros, Asya Kamsky, Scott McPeak, Dawson Engler)

5. Videos about static code analysis  (著：Ekaterina Milovidova)

6. PVS-Studio の  ブログ.

そろそろ、理論から実践に移り、静的解析がアンリアル エンジン 4 (UE4) などのプロジェクトでどのように役立つかを見ていきましょう。

アンリアル エンジン

我々は再びアンリアル エンジンのコードに関わることができて喜ばしく思います。2 年前にも、作業をしました が、コードの編集と改善に関してさらに多くのことを行いました。2 年を経てプロジェクトのコードベースを見ると得るものがあり、興味深いものです。これにはいくつかの理由があります。

まず、アナライザーの誤検出を検討しました。この作業は、不要なメッセージの数を減らし、ツールの改善に役立ちました。誤検出はコード アナライザーのデベロッパーが絶えず対処するタスクです。さらに関心があれば、 "The way static analyzers fight against false positives, and why they do it" の記事をご覧いただくことをお勧めします。

UE4 のコードベースは過去 2 年間で大幅に変更されました。追加されたものもあれば、取り除かれたものもあります。場合によってはフォルダ全体がなくなっています。そのため、必ずしもコード全部に十分に注意が払われることはありませんでした。つまり、PVS-Studio には行うべき作業が沢山あるのです。

エピック ゲームズがコードに配慮して PVS-Studio のようなツールを使用していることは素晴らしいことだと思います。これを読んだ皆さんは笑うかもしれませんね。「自分たちの顧客だからエピック ゲームズを褒めるんでしょう」と。正直言って、エピック ゲームズのデベロッパーに好意的な意見を言う動機はあります。しかし、私は心からそう思って褒めているんです。静的解析ツールを使っているという事実が、プロジェクト開発サイクルの成熟度を示し、コードの信頼性と安全性に配慮していることを意味するからです。

なぜ、PVS-Studio を使用することがコードの大幅な改善につながるという確信を私が持っていると思いますか？ PVS-Studio は非常にパワフルな静的アナライザーであり、以下のようなプロジェクトでも簡単にエラーを検出できるからです。

• GCC (bugs)

• Clang (bugs, bugs, bugs)

• Valgrind (bugs)

• GDB (bugs)

• FreeBSD (bugs, bugs)

• など、他にも色々あります。

PVS-Studio を使用すると、コードの品質をさらに上のレベルに引き上げることができます。そうすることで、エピック ゲームズではプロジェクトに UE4 を採用するユーザーにも配慮しているのです。バグが見つかるたびに、誰かの頭痛の種が減るのです。

興味深いエラー

我々が見つけて修正したあらゆるエラーについてお話するつもりはありません。注目すべきものだけを取り上げます。ご興味があれば、Github の プル リクエスト で他のエラーをご覧いただくこともできます。ソースコードと指定したプルリクエストを利用するには、GitHub の Unreal Engine リポジトリにアクセス可能でなければなりません。まだアクセスできない場合は、このブログ にアクセス方法が記載されています。 

PVS-Studio アナライザーの開発では、新しい診断が作成されているだけではなく、既存の診断も改善されています。例えば、変数が取りうる値を判定するアルゴリズムは常に改善されています。そのため、アナライザーは以下のようなエラーを 1 年以上前から検出しています。

uint8* Data = (uint8*)PointerVal;

if (Data != nullptr || DataLen == 0)
{
   NUTDebug::LogHexDump(Data, DataLen);
}
else if (Data == nullptr)
{
   Ar.Logf(TEXT("Invalid Data parameter."));
}
else // if (DataLen == 0)
{
   Ar.Logf(TEXT("Invalid DataLen parameter."));
}

PVS-Studio の警告は以下の通りです。「V547 式 'Data == nullptr' が常に true です。 unittestmanager.cpp 1924」

条件 (Data != nullptr || DataLen == 0) が true でない場合、ポインタ Data は常に nullptr と等しいことを意味します。従って、さらに (Data == nullptr) をチェックする意味はありません。

以下は、このコードを正しく 変えたものです。

if (Data != nullptr && DataLen > 0)

この診断 V547 は 2010 年に記述されたものです。しかし、変数の値を評価するメカニズムは完璧ではなく、このエラーを見つけることはできませんでした。アナライザーは変数値、DataLen のチェックで混乱していました。様々な条件下でこの変数値が何に等しいかがわかりませんでした。おそらく、こうしたコードを解析する人にとっては問題ではないかもしれませんが、エラーを探すアルゴリズムを記述する場合には単純なことではありません。

さて、以上が PVS-Studio の内部メカニズム改善の実例ですが、これは、新たなエラーの検出に効果がありました。こうした内部の改善により、アナライザーが一層正確に機能するようになりました。

さらに、C++ 言語の新バージョンの新しい構成要素をサポートすることで外部に対する改善も加えました。とはいえ、C++11、 C++14 などをパースできるようになるだけでは不十分です。古い診断に磨きをかけることと、言語の新しい構成要素でバグを見つける新たな診断を実装することは同じくらい重要です。例として、誤った範囲ベースのループを探す診断 V714 について考えましょう。V714 の診断では、アンリアル エンジンのコードにある次のループを問題視しています。

for (TSharedPtr<SWidget> SlateWidget :SlateWidgets)
{
   SlateWidget = nullptr;
}

PVS-Studio の警告は以下の通りです。「V714 変数が for-each ループに参照渡しされていませんが、その値がループ内で変更されています。 vreditorradialfloatingui.cpp 170 」

プログラマーは、コンテナ SlateWidgets 内のすべての要素に値 nullptr を代入したいと考えました。しかし、SlateWidget が、ループが繰り返されるたびに作成される通常のローカル変数であるという点が間違っています。この変数に値を代入しても、コンテナ内の要素の値は変更されません。コードが 適切 に機能するように参照を用います。

for (TSharedPtr<SWidget> &SlateWidget :SlateWidgets)
{
   SlateWidget = nullptr;
}

もちろん、C++ 言語に関係ない診断も加えました。例えば、以前 UE4 に関する記事 を書いた 2015 年当時は、診断 V767 は存在していませんでした。この診断は、PVS-Studio バージョン 6.07 (2016 年 8 月 8 日) で登場しました。この診断のお蔭で、以下のようなエラーを検出しました。

for(int i = 0; i < SelectedObjects.Num(); ++i)
{
   UObject* Obj = SelectedObjects[0].Get();
   EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);

   if(EdObj)
   {
      break;
   }
}

PVS-Studio の警告は以下の通りです。「V767 ループ内で不変のインデックスによって 'SelectedObjects' 配列の要素にアクセスしているため、問題が疑われます。 skeletonnotifydetails.cpp 38」

ループには、UEditorSkeletonNotifyObj 型を持つ要素の検索を含む必要があります。タイプミスが原因で、要素の選択時に変数 i ではなく数値リテラル 0 が記述されました。

以下はコードを 正しく 変えたものです。

UObject* Obj = SelectedObjects[i].Get();

PVS-Studio 6.07 でも検出される診断 V763 についてみてみましょう。このバグは非常に興味深いものですが、RunTest 関数の長い本体を引用しなければなりません。

bool FCreateBPTemplateProjectAutomationTests::RunTest(
const FString& Parameters)
{
      TSharedPtr<SNewProjectWizard> NewProjectWizard;
      NewProjectWizard = SNew(SNewProjectWizard);

      TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates =
      NewProjectWizard->FindTemplateProjects();
   int32 OutMatchedProjectsDesk = 0;
   int32 OutCreatedProjectsDesk = 0;
   GameProjectAutomationUtils::CreateProjectSet(Templates,
      EHardwareClass::Desktop,
      EGraphicsPreset::Maximum,
      EContentSourceCategory::BlueprintFeature,
      false,
      OutMatchedProjectsDesk,
      OutCreatedProjectsDesk);

   int32 OutMatchedProjectsMob = 0;
   int32 OutCreatedProjectsMob = 0;
   GameProjectAutomationUtils::CreateProjectSet(Templates,
      EHardwareClass::Mobile,
      EGraphicsPreset::Maximum,
      EContentSourceCategory::BlueprintFeature,
      false,
      OutMatchedProjectsMob,
      OutCreatedProjectsMob);

   return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
( OutMatchedProjectsMob == OutCreatedProjectsMob );
}

以下の部分が最も重要です。

• プログラマーは変数、OutMatchedProjectsDesk と OutCreatedProjectsDesk を、CreateProjectSet 関数の最初の呼び出しによって初期化しようとしています。

• CreateProjectSet 関数の 2 つめの呼び出しを使って、変数、OutMatchedProjectsMob と OutCreatedProjectsMob を初期化しようとします。

次に、こうした変数の値が以下の条件を充たすかのチェックがあります。

return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
( OutMatchedProjectsMob == OutCreatedProjectsMob );

レビューした関数の本体内でエラーを探さないでください。そこにはありません。このコードをお見せしたのは、関数 CreateProjectSet が、最後の 2 つの実引数として渡される変数に値を代入するように意図されているものであることを示すためです。

エラーは、関数 CreateProjectSet に隠れています。

static void CreateProjectSet(.... int32 OutCreatedProjects,
int32 OutMatchedProjects)
{
   ....
   OutCreatedProjects = 0;
   OutMatchedProjects = 0;
   ....
   OutMatchedProjects++;
   ....
   OutCreatedProjects++;
   ....
}

PVS-Studio は、以下の 2 つの警告を出します。

• 「V763 パラメータ、 'OutCreatedProjects' は、使われる前に常に値が関数本体において再書き込みされています。 gameprojectautomationtests.cpp 88」

• 「V763 パラメータ、'OutMatchedProjects' は、使われる前に常に値が関数本体において再書き込みされています。 gameprojectautomationtests.cpp 89」

アナライザーは、引数、OutCreatedProjects と OutMatchedProjects が使われておらず、ただちに 0 で上書きされていると警告していますが、そのとおりです。

エラーは単純なものです。プログラマーがパラメータを参照渡しすることを忘れていました。以下はコードを 正しく 変えたものです。

static void CreateProjectSet(.... int32 &OutCreatedProjects, int32 &OutMatchedProjects)

これまでは、検出に注意を払う必要があるエラーを挙げてみましたが、はるかに単純で平凡なエラーもあります。例えば、以下のような break 文の欠如があります。

{
   case EWidgetBlendMode::Opaque:
   ActualBackgroundColor.A = 1.0f;
   case EWidgetBlendMode::Masked:
   ActualBackgroundColor.A = 0.0f;
}

いくつかの変数を比較してその等しさを判定する際に犯してしまう誤りもあります。

checkf(GPixelFormats[PixelFormat].BlockSizeX
   == GPixelFormats[PixelFormat].BlockSizeY
   == GPixelFormats[PixelFormat].BlockSizeZ
   == 1,
TEXT("Tried to use compressed format?"));

C++ に慣れておらず、この比較がなぜ誤っているかわからない場合は、V709 の診断の記述を見ることをお勧めします。

こうしたエラーは、PVS-Studio が検出するエラーの中でも最も数が多いものです。しかし、そんなに単純に見えるのに、どうして気づかないのでしょうか？ 記事で取り上げてみると、よくある誤りなのに。しかし、実際のアプリケーションのコード内で見つけるのは非常に困難です。コード レビュー中でさえ、以下のコード ブロックを見て、エラーに気付かないかもしれません。

{
   case EWidgetBlendMode::Opaque:
   ActualBackgroundColor.A = 1.0f;
   case EWidgetBlendMode::Masked:
   ActualBackgroundColor.A = 0.0f;
}

コードは非常に単純に見えるので、プログラマーは完全に正しいと思い込んで注意深く読もうともしません。

次の疑問について考えましょう。エラーの数は減らせるのでしょうか？

推奨事項

この記事に記載したエラーは、PVS-Studio を使用して見つかったものです。ですから、著者が静的解析ツールの使用を推奨すると読者の皆さんはお考えになるでしょう。そのとおりです。PVS-Studio 静的アナライザーを開発プロセスに統合することをお勧めします。コードを記述してすぐにいくつかのバグが見つかることはありえます。ここでは、コードの品質に関する記事で通常は言及しない重要なポイントを説明しようと思います。

プログラマー チームがミスをする、しかも時にはとても単純なミスをすることを認めるまで高品質なプロジェクトにすることは不可能です。これは自明のことだと思うかもしれませんが、とても重要です。このことがプログラマー全般のこととして考えるのではなく、プログラマーが自分のこととして考えられるようになるまで、ツールや方法論は役立ちません。概してプログラマーは、高品質なコードを記述するために新たなツールや方法が必要であると認めるのはプライドが許さないようです。

すべてのプログラマーは、プログラムにエラーがつきものであることを認識しています。それにも関わらず、ルール、推奨事項、ツールは自分には不要だと考えるのです。バグなしのコードを記述する偉大なプロフェッショナル デベロッパーだと思っているからです。これは、レベルを過大評価しており、問題になります。"The Problem With 'Above Average Programmers'" の記事は、この影響について適切に解説しています。以下は、この記事からの引用です。

「ご自分のプログラミング スキルはどれくらいですか (平均以下、平均、平均以上) ？」 

多様なグループに対する心理学的研究 によると全プログラマーの約 90% が「平均以上」と回答するとのことです。 もちろん、そんなことはないはずですが。100 名のグループでは、50 名が平均以上で 50 名が平均以下になるのですから。この効果は、 優越性の錯覚 (illusory superiority) として知られています。この表現は多くの分野で使われていますが、皆さんは耳にしたことはないかもしれません。しかし、本当に「平均より上」と答える人は多いのです。

これはプログラマーが新しい技術や方法を学ばないという問題につながります。チームと個人の作業に対する姿勢を再考することをお勧めします。「私 / 私たちは素晴らしいコードを書く」という考えは、逆効果をもたらします。ミスをするのは人間として当たり前のことで、プログラマーにも該当します。ミスはつきものと考えることで、高品質なソフトウェアにするという大きな一歩を踏み出すことになります。プロジェクト マネージャの方々には、この 記事をご覧いただくことをお勧めします。

もう一つ誤った考え方を提起しておきたいと思います。静的アナライザーと動的アナライザーは、主に単純なバグやタイプミスを検出するものです。高度な論理エラーは見つからないでしょう。まだ AI が用意できていないからです。とはいえ、単純なエラーによって大きな被害を生じ、修正するために多くの時間、コスト、労力が必要になります。このトピックに関する詳細は、"If the coding bug is banal, it doesn't mean it's not crucial" をご覧ください。

さらに、もうひとつあります。特効薬的なものを求めないでください。代わりに以下のような様々な要素を組み合わせて使用しましょう。

• 「自分たちのチームは平均以上」という考えは忘れてください。

• コーディング規約を作り、チーム内のメンバー全員で共有してください。

• コード レビューを行います (少なくとも、非常に重要な部分、および経験の少ないメンバーが記述したコードを対象に)。

• 静的コード解析

• 動的コード解析

• リグレッション テスト、スモーク テスト

• 単体テスト、TDD を使用する

• および他にもあります。

上記のすべての方法を一度に使うことを提案しているわけではありません。様々なプロジェクトで、有用なアプローチもあれば、そうでないものもあることでしょう。要するに、ひとつのものを単独で機能させるのではなく、複数の方法を合理的に組み合わせて使用するとよいでしょう。こうした取り組みをすることで、コードの品質と信頼性が高まります。

結論

UE4 のデベロッパーはコードの品質に配慮しており、PVS-Studio チームではそうした努力を支援すべく最善の努力をしています。

PVS-Studio チームは、皆さんのプロジェクトのコードの作業にも対応する用意ができています。ツールのライセンス供与とサポート以外にも、コードの監査、コードの移行など を行っています。

プログラムのバグができる限り少なくなるといいですね。