Clever Geek Handbook

帝国の逆襲

ダースベイダー-ユニコーン

最近、「Hackathon 2：Unreal Engine 4のタイムラプス分析」という記事が登場しました。Klocworkツールを使用すると、Unreal Engine 4プロジェクトで多くのエラーを見つける方法がわかります。 実際、PVS-Studioアナライザーがこのプロジェクトで検出したすべてのエラーをやがて修正しました。 すべてのエラーがまったく修正されたわけではなく、アナライザーによってのみ検出されたことは明らかです。 ただし、この記事では、PVS-Studioアナライザーが見逃しすぎた印象を与えています。 さて、今は私の番です。 また、Unreal Engine 4を再確認したところ、多くのエラーが見つかりました。 したがって、PVS-Studioは現在、Unreal Engine 4で新しいエラーを見つけることもできます。 引き分け。



歴史的背景



それは、1年半前に「 待望のUnreal Engine 4のチェック 」という記事を書いたときに始まりました。 その後、Epic Gamesとのコラボレーションがあり、その結果、PVS-Studioによって発行されたすべての警告が削除されました。 その結果、多くのエラーが修正され、アナライザーのすべての誤検知が排除されました。 私たちのチームは、PVS-StudioアラートのないプロジェクトをEpic Gamesに発行しました。 これがどのように起こったかは、記事「 PVS-Studioチームがアンリアルエンジンコードを改善した方法 」で見つけることができます。



最近、インターネットで「 Hackathon 2：Unreal Engine 4のタイムラプス分析 」という記事に出会いました。 記事は良くて正しいです。 Rogue Waveは、このようなイベントをホストし、Klocworkを強力な静的コードアナライザーにする素晴らしい会社です。 Michail Greshishchevも素晴らしいです。彼はUnreal Engineをチェックし、それについて記事を書く時間を見つけました。 すべて順調です。 しかし、静的アナライザーを初めて使用する部外者が間違った結論を導き出すのではないかと心配しています。 したがって、この記事にコメントする必要があります。



意図せずに、しかし初心者の読者には、この記事はKlocworkと比較して不利な観点から私たちを示しています。 PVS-Studioは、Klocworkよりも少ないエラーを見つけるようです。 実際、世界はもっと複雑です。 両方のアナライザーには、異なる診断セットがあります。 これらのセットは部分的に交差します。 ただし、各アナライザーには固有の診断セットがあります。 したがって、1つのアナライザーで大規模なプロジェクトをチェックすると、常に別のアナライザーで何かが見つかります。



別のニュアンス。 コード例の1つが示すように、サードパーティのライブラリ（ThirdPartyディレクトリ）はチェックせず、Michail Greshishchevは（少なくとも部分的に）チェックしました（HeadMountedDisplayCommon関数を参照）。 ThirdPartyでは、PVS-Studioアナライザーは多くの興味深い欠陥を簡単に見つけることもできます。 さらに、ThirdPartyソースのサイズはUE4自体の3倍です。



しかし、これはすべて正当化しようとする哀れな試みのように聞こえます:)。 したがって、スコアを平準化する以外に選択肢はありません。 この目的のために、Unreal Engine 4のソースは、最新バージョンのPVS-Studioアナライザーによって取得およびチェックされました。



そして今、私はあなたが大規模なペースの速いプロジェクトで常に簡単にエラーを見つけることができることを示します。



PVS-Studioを使用したプロジェクト検証結果



UE4のソースコードを最新バージョンのPVS-Studioで確認しました。 UE4のソースコードは、ThirdPartyカタログに関係なくチェックされました。 それらをチェックすると、記事は入手できませんが、参照は入手できます:)。



1792の第1レベルおよび第2レベルの汎用警告を受け取りました。 怖がらないでください。 この数字がどこから来たのかが明らかになります。



ほとんどの警告（93％）は、初期化されていないクラスメンバーを識別するように設計された新しい診断ルールV730に関連しています。 クラスの初期化されていないメンバーは必ずしも間違いではありませんが、それでもプログラム内でチェックする価値のある場所です。 一般に、1672 V730警告は多数あります。 他のプロジェクトでは、これらの警告が同量見られていません。 さらに、可能であれば、アナライザーは、初期化されていないメンバーが怖くないときを予測しようとします。 ところで、初期化されていないメンバーの検索は恩知らずの仕事であり、おそらく読者は「 初期化されていないクラスメンバーの検索 」という記事を読むことに興味があるでしょう。



しかし、UE4プロジェクトに戻ります。 この記事では、警告V730については触れません。 それらの数が多すぎるため、この変数またはその初期化されていない変数がプログラムのエラーにつながる可能性があるかどうかを判断するには、UE4プロジェクトが少なすぎます。 ただし、これらの1672の警告には、重大な重大エラーがかなりあると確信しています。 Epic Gamesの開発者はそれらを分析すべきだと思います。 これらが連続的な誤検知であると考える場合、この診断は単にオフにすることができます。



したがって、1792-1672 =120。合計で、PVS-Studioは、Unreal Engineのチェック時に120の汎用警告（レベル1および2）を発行しました。 これらの警告の多くは、実際のエラーを特定しています。 最も興味深いコードとそれに対応する警告を検討してください。



PVS-Studioで見つかった興味深いエラー



この記事では、注意と編集に値するコードのすべてのセクションを紹介するわけではないことを再度強調します。 まず、レポートを流fluentに見て、何か面白いものを見逃していたかもしれません。 第二に、軽微なエラーや説明が難しいエラーを書きませんでした（多くのコードフラグメントが必要です）。



エラーN1 

FORCEINLINE bool operator==(const FShapedGlyphEntryKey& Other) const { return FontFace == Other.FontFace && GlyphIndex == Other.GlyphIndex && FontSize == Other.FontSize && FontScale == Other.FontScale && GlyphIndex == Other.GlyphIndex; }


PVS-Studio警告：V501「&&」演算子の左側と右側には、同一の副次式「GlyphIndex == Other.GlyphIndex」があります。 fontcache.h 139



「GlyphIndex == Other.GlyphIndex」チェックは2回繰り返されます。 アクションの最後の行の効果 。 どうやら、最後の比較はKeyHash == Other.KeyHashになります。



エラーN2



最後の別のライン効果。 正しい。 

 bool Compare(const FPooledRenderTargetDesc& rhs, bool bExact) const { .... return Extent == rhs.Extent && Depth == rhs.Depth && bIsArray == rhs.bIsArray && ArraySize == rhs.ArraySize && NumMips == rhs.NumMips && NumSamples == rhs.NumSamples && Format == rhs.Format && LhsFlags == RhsFlags && TargetableFlags == rhs.TargetableFlags && bForceSeparateTargetAndShaderResource == rhs.bForceSeparateTargetAndShaderResource && ClearValue == rhs.ClearValue && AutoWritable == AutoWritable; }


警告PVS-Studio：V501「==」演算子の左右に同じサブ式があります：AutoWritable == AutoWritable rendererinterface.h 180



最後に、「rhs」を追加するのを忘れました。その結果、「AutoWritable」変数はそれ自体と比較されます。



エラーN3 

 void AEQSTestingPawn::PostLoad() { .... UWorld* World = GetWorld(); if (World && World->IsGameWorld() && bTickDuringGame == bTickDuringGame) { PrimaryActorTick.bCanEverTick = false; } }


PVS-Studio警告：V501 '=='演算子の左右に同じ副次式があります：bTickDuringGame == bTickDuringGame eqstestingpawn.cpp 157



エラーN4 

 int32 SRetainerWidget::OnPaint(....) const { .... if ( RenderTargetResource->GetWidth() != 0 && RenderTargetResource->GetWidth() != 0 ) .... }


PVS-Studioの警告：V501「&&」演算子の左右には、同一の副次式「RenderTargetResource-> GetWidth（）！= 0」があります。 sretainerwidget.cpp 291



エラーN5、N6



2つの同一のエラーが近傍にあります。 ZeroMemoryマクロはmemset（）関数の呼び出しに過ぎず、割り当てられたメモリの一部のみをゼロにします。 

 class FD3D12BufferedGPUTiming { .... FD3D12CLSyncPoint* StartTimestampListHandles; FD3D12CLSyncPoint* EndTimestampListHandles; .... }; void FD3D12BufferedGPUTiming::InitDynamicRHI() { .... StartTimestampListHandles = new FD3D12CLSyncPoint[BufferSize]; ZeroMemory(StartTimestampListHandles, sizeof(StartTimestampListHandles)); EndTimestampListHandles = new FD3D12CLSyncPoint[BufferSize]; ZeroMemory(EndTimestampListHandles, sizeof(EndTimestampListHandles)); .... }


PVS-Studioの警告：

エラーは、sizeof（）演算子が配列ではなくポインターのサイズを計算することです。 正しいオプションの1つは、これを書くことです。 

 ZeroMemory(StartTimestampListHandles, sizeof(FD3D12CLSyncPoint) * BufferSize); ZeroMemory(EndTimestampListHandles, sizeof(FD3D12CLSyncPoint) * BufferSize);


エラーN7 

 void FDeferredShadingSceneRenderer::RenderLight(....) { .... if (bClearCoatNeeded) { SetShaderTemplLighting<false, false, false, true>( RHICmdList, View, *VertexShader, LightSceneInfo); } else { SetShaderTemplLighting<false, false, false, true>( RHICmdList, View, *VertexShader, LightSceneInfo); } .... }


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



条件に関係なく、同じアクションが実行されます。



エラーN8 

 bool FBuildDataCompactifier::Compactify(....) const { .... uint64 CurrentFileSize; .... CurrentFileSize = IFileManager::Get().FileSize(*File); if (CurrentFileSize >= 0) { .... } else { GLog->Logf(TEXT("Warning. ......"), *File); } .... }


PVS-Studioの警告：V547式 'CurrentFileSize> = 0'は常にtrueです。 符号なしの型の値は常に> = 0です。buildpatchcompactifier.cpp 135



「if（CurrentFileSize> = 0）」をチェックしても意味がありません。 変数 'CurrentFileSize'には符号なしの型があります。つまり、値は常に0以上です。



エラーN9 

 template<typename TParamRef> void UnsetParameters(....) { .... int32 NumOutUAVs = 0; FUnorderedAccessViewRHIParamRef OutUAVs[3]; OutUAVs[NumOutUAVs++] = ObjectBuffers......; OutUAVs[NumOutUAVs++] = ObjectBuffers.Bounds.UAV; OutUAVs[NumOutUAVs++] = ObjectBuffers.Data.UAV; if (CulledObjectBoxBounds.IsBound()) { OutUAVs[NumOutUAVs++] = ObjectBuffers.BoxBounds.UAV; } .... }


V557配列のオーバーランが可能です。 「NumOutUAVs ++」インデックスは、配列の境界を超えています。 distancefieldlightingshared.h 388



条件（CulledObjectBoxBounds.IsBound（））が満たされると、配列は境界を超えます。 配列「OutUAV」は3つの要素のみで構成されていることに注意してください。



エラーN10 

 class FSlateDrawElement { .... FORCEINLINE void SetPosition(const FVector2D& InPosition) { Position = Position; } .... FVector2D Position; .... };


PVS-Studio警告：V570「位置」変数はそれ自体に割り当てられます。 drawelements.h 435



このエラーは説明する必要さえありません。 タイプミス。 {Position = InPosition;でなければなりません。 }。



エラーN11 

 bool FOculusRiftHMD::DoesSupportPositionalTracking() const { const FGameFrame* frame = GetFrame(); const FSettings* OculusSettings = frame->GetSettings(); return (frame && OculusSettings->Flags.bHmdPosTracking && (OculusSettings->SupportedTrackingCaps & ovrTrackingCap_Position) != 0); }


PVS-Studio警告：V595 nullptrに対して検証される前に、「フレーム」ポインターが使用されました。 チェック行：301、302。oculusrifthmd.cpp 301



変数「frame」が最初に使用され、次にnullptrの等価性がチェックされます。



このエラーは、Klocworkの記事で説明されているエラーと非常によく似ています ： 

 bool FHeadMountedDisplay::IsInLowPersistenceMode() const { const auto frame = GetCurrentFrame(); const auto FrameSettings = frame->Settings; return frame && FrameSettings->Flags.bLowPersistenceMode; }


ご覧のとおり、両方のアナライザーがこのタイプの欠陥を検出できます。



Klocworkの記事で提供されているコードは、プロジェクトを確認しなかったThirdPartyディレクトリを参照していることに注意してください。



エラーN12-N21 

 FName UKismetNodeHelperLibrary::GetEnumeratorName( const UEnum* Enum, uint8 EnumeratorValue) { int32 EnumeratorIndex = Enum->GetIndexByValue(EnumeratorValue); return (NULL != Enum) ? Enum->GetEnum(EnumeratorIndex) : NAME_None; }


PVS-Studio警告：V595 nullptrに対して検証される前に 'Enum'ポインターが使用されました。 行を確認してください：146、147。kismetnodehelperlibrary.cpp 146



繰り返しますが、最初はポインターが逆参照され、その後のみチェックされます。 このようなエラーをさらに考慮することは退屈です。 注目すべき場所のリスト：

エラーN22 

 class FD3D12Device { .... virtual void InitD3DDevice(); virtual void CleanupD3DDevice(); .... //    .... };


V599「FD3D12Device」クラスには仮想関数が含まれていますが、仮想デストラクタは存在しません。 d3d12device.cpp 448



FD3D12Deviceクラスには仮想メソッドがあります。 つまり、このクラスには相続人がいると想定されます。 ただし、このクラスには仮想デストラクタはありません。 これは非常に危険であり、遅かれ早かれエラーにつながる可能性が高いです。



エラーN23-N26 

 int SpawnTarget(WCHAR* CmdLine) { .... if(!CreateProcess(....)) { DWORD ErrorCode = GetLastError(); WCHAR* Buffer = new WCHAR[wcslen(CmdLine) + 50]; wsprintf(Buffer, L"Couldn't start:\n%s\nCreateProcess() returned %x.", CmdLine, ErrorCode); MessageBoxW(NULL, Buffer, NULL, MB_OK); delete Buffer; return 9005; } .... }


PVS-Studio警告：V611メモリは「new T []」演算子を使用して割り当てられましたが、「delete」演算子を使用して解放されました。 このコードを調べることを検討してください。 「delete [] Buffer;」を使用する方がおそらく良いでしょう。 bootstrappackagedgame.cpp 110



間違った方法で、割り当てられたメモリが解放されます。 それは書かれるべきです： 

 delete [] Buffer;


このようなエラーがさらにいくつかあります。

エラーN27 

 void FSlateTexture2DRHIRef::InitDynamicRHI() { .... checkf(GPixelFormats[PixelFormat].BlockSizeX == GPixelFormats[PixelFormat].BlockSizeY == GPixelFormats[PixelFormat].BlockSizeZ == 1, TEXT("Tried to use compressed format?")); .... }


PVS-Studio警告：V709疑わしい比較が見つかりました： 'a == b == c'。 「a == b == c」は「a == b && b == c」と等しくないことに注意してください。 slatetextures.cpp 67



検証は、プログラマが意図したとおりにはまったく機能しません。 どうやらあなたは書くべきです： 

 GPixelFormats[PixelFormat].BlockSizeX == 1 && GPixelFormats[PixelFormat].BlockSizeY == 1 && GPixelFormats[PixelFormat].BlockSizeZ == 1


エラーN28 

 void UWidgetComponent::UpdateRenderTarget() { .... FLinearColor ActualBackgroundColor = BackgroundColor; switch ( BlendMode ) { case EWidgetBlendMode::Opaque: ActualBackgroundColor.A = 1.0f; case EWidgetBlendMode::Masked: ActualBackgroundColor.A = 0.0f; } .... }


V519「ActualBackgroundColor.A」変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。 行をチェック：938、940。widgetcomponent.cpp 940



ここでは、欠落している「break」ステートメントが間接的に検出されます。 変数「ActualBackgroundColor.A」には、異なる値を連続して2回割り当てることができます。 これがPVS-Studioアナライザーを心配するものです。



エラーN29 

 void FProfilerManager::TrackDefaultStats() { // Find StatId for the game thread. for( auto It = GetProfilerInstancesIterator(); It; ++It ) { FProfilerSessionRef ProfilerSession = It.Value(); if( ProfilerSession->GetMetaData()->IsReady() ) { ....; } break; } }


PVS-Studio警告：V612ループ内の無条件の「ブレーク」。 profilermanager.cpp 717



非常に疑わしいコード。 「ブレイク」ステートメントは場違いのようです。 よくわかりませんが、おそらく次のように書く予定でした。 

 for( auto It = GetProfilerInstancesIterator(); It; ++It ) { FProfilerSessionRef ProfilerSession = It.Value(); if( ProfilerSession->GetMetaData()->IsReady() ) { ....; break; } }


まとめ



PVS-Studioによって発行された120の警告のうち少なくとも29は、実際のエラーを示しました（24％）。 別の50％は臭いがするコードです。 残りは誤検知です。 プロジェクトの確認とこの記事の執筆に費やした合計時間は約10時間です。



PVS-StudioおよびKlocworkアナライザーの結果からどのような結論を導き出すことができますか：
  1. 大規模で急速に発展しているプロジェクトでは、常にもう少しエラーを見つけることができます:)
  2. 交差点はありますが、PVS-StudioとKlocworkの診断セットは異なります。
  3. Klocworkは、サードパーティライブラリ（ThirdParty）を含むUnreal Engine 4をテストした可能性があります。 これらのライブラリは、当時も現在もチェックしていません。
  4. 両方のアナライザーは優れており、それらの使用はプログラム開発プロセスに多くの利益をもたらすことができます。


ご清聴ありがとうございました。





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



記事を読んで質問がありますか？
多くの場合、記事には同じ質問が寄せられます。 ここで回答を集めました： PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。



More articles:


All Articles