Jump to content

  • Log In with Google      Sign In   
  • Create Account

Banner advertising on our site currently available from just $5!


1. Learn about the promo. 2. Sign up for GDNet+. 3. Set up your advert!


Like
23Likes
Dislike

A Long-Awaited Check of Unreal Engine 4

By Andrey Karpov | Published Apr 14 2014 08:57 PM in Game Programming
Peer Reviewed by (GuardianX, Servant of the Lord, Josh Vega)

c++ pvs-studio static code analyzer code review unreal engine 4 unreal engine Unreal Engine Unreal Engine 4

On March 19, 2014, Unreal Engine 4 was made publicly available. Subscription costs only $19 per month. The source codes have also been published at the github repository. Since that moment, we have received quite a number of e-mails, twitter messages, etc., people asking to check this game engine. So we are fulfilling our readers' request in this article; let's see what interesting bugs the PVS-Studio static code analyzer has found in the project's source code.

image1.png

Unreal Engine


The Unreal Engine is a game engine developed by Epic Games, first illustrated in the 1998 first-person shooter game Unreal. Although primarily developed for the first-person shooters, it has been successfully used in a variety of other genres, including stealth, MMORPGs, and other RPGs. With its code written in C++, Unreal Engine features a high degree of portability and is a tool used by many game developers today.

The official website: https://www.unrealengine.com/

The Wikipedia article: Unreal Engine.

Analysis methodology for an nmake-based project


There exist certain difficulties regarding analysis of the Unreal Engine project. To check it, we had to use a new feature recently introduced in PVS-Studio Standalone. Because of that, we had to postpone the publication of this article a bit so that it would follow the release of the new PVS-Studio version with this feature. I guess many would like to try it: it allows programmers to easily check projects that make use of complex or non-standard build systems.

PVS-Studio's original working principle is as follows:
  • You open a project in Visual Studio.
  • Click the "Start" button.
  • The Visual Studio-integrated plugin collects all the necessary information: which files need to be analyzed, which macros are to be expanded, where the header files location, and so on.
  • The plugin launches the analyzer module itself and outputs the analysis results.
What's special about Unreal Engine 4 is that it is an nmake-based project, therefore it can't be checked by the PVS-Studio plugin.

Let me explain this point. Unreal Engine is implemented as a Visual Studio project, but the build is done with nmake. It means that the plugin cannot know which files are compiled with which switches. Therefore, analysis is impossible. To be exact, it is possible, but it will be somewhat of an effort (see the documentation section, "Direct integration of the analyzer into build automation systems").

And here's PVS-Studio Standalone coming to help! It can work in two modes:

  1. You obtain preprocessed files in any way and let the tool check them.
  2. Its monitoring compiler calls and get all the necessary information.

It is the second mode that we are interested in now. This is how the check of Unreal Engine was done:

  1. We launched PVS-Studio Standalone.
  2. Clicked "Compiler Monitoring".
  3. Then we clicked "Start Monitoring" and made sure the compiler call monitoring mode was on.
  4. We opened the Unreal Engine project in Visual Studio and started the project build. The monitoring window indicated that the compiler calls were being tapped.
  5. When the build was finished, we clicked Stop Monitoring, and after that the PVS-Studio analyzer was launched.

The diagnostic messages were displayed in the PVS-Studio Standalone window.

Hint. It is more convenient to use Visual Studio instead of the PVS-Studio Standalone's editor to work with the analysis report. You only need to save the results into a log file and then open it in the Visual Studio environment (Menu->PVS-Studio->Open/Save->Open Analysis Report).

All that and many other things are described in detail in the article "PVS-Studio Now Supports Any Build System under Windows and Any Compiler. Easy and Right Out of the Box". Do read this article please before you start experimenting with PVS-Studio Standalone!

Analysis results


I found the Unreal Engine project's code very high-quality. For example, developers employ static code analysis during the development, which is hinted at by the following code fragments:

// Suppress static code analysis warning about a
// potential comparison of two constants
CA_SUPPRESS(6326);
....
// Suppress static code analysis warnings about a
// potentially ill-defined loop. BlendCount > 0 is valid.
CA_SUPPRESS(6294)
....
#if USING_CODE_ANALYSIS

These code fragments prove that they use a static code analyzer integrated into Visual Studio. To find out more about this tool, see the article Visual Studio 2013 Static Code Analysis in depth: What? When and How?

The project authors may also use some other analyzers, but I can't say for sure.

So their code is pretty good. Since they use static code analysis tools during the development, PVS-Studio has not found many suspicious fragments. However, just like any other large project, this one does have some bugs, and PVS-Studio can catch some of them. So let's find out what it has to show us.

Typos


static bool PositionIsInside(....)
{
  return
    Position.X >= Control.Center.X - BoxSize.X * 0.5f &&
    Position.X <= Control.Center.X + BoxSize.X * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f;
}

PVS-Studio's diagnostic message: V501 There are identical sub-expressions 'Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f' to the left and to the right of the '&&' operator. svirtualjoystick.cpp 97

Notice that the Position.Y variable is compared to the Control.Center.Y - BoxSize.Y * 0.5f expression twice. This is obviously a typo; the '-' operator should be replaced with '+' in the last line.

Here's one more similar mistake in a condition:

void FOculusRiftHMD::PreRenderView_RenderThread(
  FSceneView& View)
{
  ....
  if (View.StereoPass == eSSP_LEFT_EYE ||
      View.StereoPass == eSSP_LEFT_EYE)
  ....
}

PVS-Studio's diagnostic message: V501 There are identical sub-expressions 'View.StereoPass == eSSP_LEFT_EYE' to the left and to the right of the '||' operator. oculusrifthmd.cpp 1453

It seems that the work with Oculus Rift is not well tested yet.

Let's go on.

struct FMemoryAllocationStats_DEPRECATED
{
  ....
  SIZE_T  NotUsed5;
  SIZE_T  NotUsed6;
  SIZE_T  NotUsed7;
  SIZE_T  NotUsed8;
  ....
};

FMemoryAllocationStats_DEPRECATED()
{
  ....
  NotUsed5 = 0;
  NotUsed6 = 0;
  NotUsed6 = 0;  
  NotUsed8 = 0;  
  ....
}

PVS-Studio's diagnostic message: V519 The 'NotUsed6' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 86, 88. memorybase.h 88

Structure members are initialized here. A typo causes the NotUsed6 member to be initialized twice, while the NotUsed7 member remains uninitialized. However, the _DEPRECATED() suffix in the function name tells us this code is not of much interest anymore.

Here are two other fragments where one variable is assigned a value twice:
  • V519 The HighlightText variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 204, 206. srichtextblock.cpp 206
  • V519 The TrackError.MaxErrorInScaleDueToScale variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1715, 1716. animationutils.cpp 1716

Null pointers


I pretty often come across null pointer dereferencing errors in error handlers. No wonder: these fragments are difficult and uninteresting to test. In Unreal Engine, you can find a null pointer dereferencing error in an error handler too:

bool UEngine::CommitMapChange( FWorldContext &Context )
{
  ....
  LevelStreamingObject = Context.World()->StreamingLevels[j];
  if (LevelStreamingObject != NULL)
  {
    ....
  }
  else
  {
    check(LevelStreamingObject);
    UE_LOG(LogStreaming, Log,
           TEXT("Unable to handle streaming object %s"),
           *LevelStreamingObject->GetName());
  }
  ....
}

PVS-Studio's diagnostic message: V522 Dereferencing of the null pointer 'LevelStreamingObject' might take place. unrealengine.cpp 10768

We want to print the object name when an error occurs. But the object doesn't exist.

Here's another fragment with null pointer dereferencing. It's all much more interesting here. Perhaps the error appeared because of an incorrect merge. Anyway, the comment proves that the code is incomplete:

void FStreamingPause::Init()
{
  ....
  if( GStreamingPauseBackground == NULL && GUseStreamingPause )
  {
    // @todo UE4 merge andrew
    // GStreamingPauseBackground = new FFrontBufferTexture(....);
    GStreamingPauseBackground->InitRHI();
  }
}

PVS-Studio's diagnostic message: V522 Dereferencing of the null pointer 'GStreamingPauseBackground' might take place. streamingpauserendering.cpp 197

A few more words about null pointers


Almost in every program I check, I get a pile of V595 warnings (examples). These warnings indicate the following trouble:

A pointer is dereferenced first and only then is checked for being null. That's not always an error, but this code is highly suspicious and needs to be checked anyway!

The V595 diagnostic helps us reveal slip-ups like this:

/**
 * Global engine pointer.
 * Can be 0 so don't use without checking.
 */
ENGINE_API UEngine* GEngine = NULL;

bool UEngine::LoadMap( FWorldContext& WorldContext,
  FURL URL, class UPendingNetGame* Pending, FString& Error )
{
  ....
  if (GEngine->GameViewport != NULL)
  {
    ClearDebugDisplayProperties();
  }

  if( GEngine )
  {
    GEngine->WorldDestroyed( WorldContext.World() );
  }
  ....
}

PVS-Studio's diagnostic message: V595 The 'GEngine' pointer was utilized before it was verified against nullptr. Check lines: 9714, 9719. unrealengine.cpp 9714

Notice the comment. The global variable GEngine may be equal to zero, so it must be checked before it can be used.

And there is such a check indeed in the function LoadMap():

if( GEngine )

Unfortunately, this check is executed only after the pointer has been already used:

if (GEngine->GameViewport != NULL)

There were quite a number of V595 warnings for the project (about 82). I guess many of them are false positives, so I won't litter the article with the samples and cite them in a separate list: ue-v595.txt.

Excess variable declaration


This error is pretty nice. It is about mistakenly declaring a new variable instead of using an already existing one.

void FStreamableManager::AsyncLoadCallback(....)
{
  ....
  FStreamable* Existing = StreamableItems.FindRef(TargetName);
  ....
  if (!Existing)
  {
    // hmm, maybe it was redirected by a consolidate
    TargetName = ResolveRedirects(TargetName);
    FStreamable* Existing = StreamableItems.FindRef(TargetName);
  }
  if (Existing && Existing->bAsyncLoadRequestOutstanding)
  ....
}

PVS-Studio's diagnostic message: V561 It's probably better to assign value to 'Existing' variable than to declare it anew. Previous declaration: streamablemanager.cpp, line 325. streamablemanager.cpp 332

I suspect the code must look like this:

// hmm, maybe it was redirected by a consolidate
TargetName = ResolveRedirects(TargetName);
Existing = StreamableItems.FindRef(TargetName);

Errors in function calls


bool FRecastQueryFilter::IsEqual(
  const INavigationQueryFilterInterface* Other) const
{
  // @NOTE: not type safe, should be changed when
  // another filter type is introduced
  return FMemory::Memcmp(this, Other, sizeof(this)) == 0;
}

PVS-Studio's diagnostic message: V579 The Memcmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. pimplrecastnavmesh.cpp 172

The comment warns us that it is dangerous to use Memcmp(). But actually it all is even worse than the programmer expects. The point is that the function compares only a part of the object.

The sizeof(this) operator returns the pointer size; that is, the function will compare the first 4 bytes in a 32-bit program and 8 bytes in a 64-bit program.

The correct code should look as follows:

return FMemory::Memcmp(this, Other, sizeof(*this)) == 0;

But that's not the only trouble with the Memcmp() function. Have a look at the following code fragment:

D3D11_STATE_CACHE_INLINE void GetBlendState(
  ID3D11BlendState** BlendState, float BlendFactor[4],
  uint32* SampleMask)
{
  ....
  FMemory::Memcmp(BlendFactor, CurrentBlendFactor,
                  sizeof(CurrentBlendFactor));
  ....
}

PVS-Studio's diagnostic message: V530 The return value of function 'Memcmp' is required to be utilized. d3d11statecacheprivate.h 547

The analyzer was surprised at finding the Memcmp() function's result not being used anywhere. And this is an error indeed. As far as I get it, the programmer wanted to copy the data, not compare them. If so, the Memcpy() function should be used:

FMemory::Memcpy(BlendFactor, CurrentBlendFactor,
                sizeof(CurrentBlendFactor));

A variable assigned to itself


enum ECubeFace;
ECubeFace CubeFace;

friend FArchive& operator<<(
  FArchive& Ar,FResolveParams& ResolveParams)
{
  ....
  if(Ar.IsLoading())
  {
    ResolveParams.CubeFace = (ECubeFace)ResolveParams.CubeFace;
  }
  ....
}

PVS-Studio's diagnostic message: V570 The 'ResolveParams.CubeFace' variable is assigned to itself. rhi.h 1279

The ResolveParams.CubeFace variable is of the ECubeFace type, and it is cast explicitly to the ECubeFace type, i.e. nothing happens. After that, the variable is assigned to itself. Something is wrong with this code.

The nicest of all the errors


image2.png
I do like the following error most of all:

bool VertInfluencedByActiveBone(
  FParticleEmitterInstance* Owner,
  USkeletalMeshComponent* InSkelMeshComponent,
  int32 InVertexIndex,
  int32* OutBoneIndex = NULL);

void UParticleModuleLocationSkelVertSurface::Spawn(....)
{
  ....
  int32 BoneIndex1, BoneIndex2, BoneIndex3;
  BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE;

  if(!VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[2]) &BoneIndex3)
  {
  ....
}

PVS-Studio's diagnostic message: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. particlemodules_location.cpp 2120

It's not that easy to spot it. I'm sure you've just scanned through the code and haven't noticed anything strange. The analyzer warning, unfortunately, is also strange and suggests a false positive. But in fact, we are dealing with a real and very interesting bug.

Let's figure it all out. Notice that the last argument of the VertInfluencedByActiveBone() function is optional.

In this code fragment, the VertInfluencedByActiveBone() function is called 3 times. The first two times, it receives 4 arguments; with the last call, only 3 arguments. And here is where the error is lurking.

It is only from pure luck that the code compiles well, the error staying unnoticed. This is how it happens:

  1. The function is called with 3 arguments: VertInfluencedByActiveBone(Owner, SourceComponent, VertIndex[2]);
  2. The '!' operator is applied to the function result;
  3. The !VertInfluencedByActiveBone(...) expression evaluates to a bool value;
  4. The '&' (bitwise AND) operator is applied to it;
  5. All this is compiled successfully because there is a bool expression to the left of the '&' operator and an integer variable BoneIndex3 to the right.

The analyzer suspected something was wrong on discovering one of the '&' operator's arguments to have the bool type. And that was what it warned us about - not in vain.

To fix the error, we need to add a comma and put a closing parenthesis in the right place:

if(!VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
   !VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
   !VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[2], &BoneIndex3))

A break operator missing


static void VerifyUniformLayout(....)
{
  ....
  switch(Member.GetBaseType())
  {
    case UBMT_STRUCT:  BaseTypeName = TEXT("struct"); 
    case UBMT_BOOL:    BaseTypeName = TEXT("bool"); break;
    case UBMT_INT32:   BaseTypeName = TEXT("int"); break;
    case UBMT_UINT32:  BaseTypeName = TEXT("uint"); break;
    case UBMT_FLOAT32: BaseTypeName = TEXT("float"); break;
    default:           
      UE_LOG(LogShaders, Fatal,
        TEXT("Unrecognized uniform ......"));
  };
  ....
}

PVS-Studio's diagnostic message: V519 The 'BaseTypeName' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 862, 863. openglshaders.cpp 863

The break; operator is missing in the very beginning. I guess no comments and explanations are needed.

Microoptimizations


The PVS-Studio analyzer offers a small set of diagnostic rules that help carry out microoptimizations of the code. Though small, they may prove pretty useful at times. Let's take one assignment operator as an example:

FVariant& operator=( const TArray<uint8> InArray )
{
  Type = EVariantTypes::ByteArray;
  Value = InArray;
  return *this;
}

PVS-Studio's diagnostic message: V801 Decreased performance. It is better to redefine the first function argument as a reference. Consider replacing 'const .. InArray' with 'const .. &InArray'. variant.h 198

It's not a very good idea to pass an array by value. The InArray can and must be passed by a constant reference.

The analyzer generated quite a few warnings related to microoptimizations. I don't think many of them will be really useful, but here you are a list of these fragments just in case: ue-v801-V803.txt.

Suspicious sum


uint32 GetAllocatedSize() const
{
  return UniformVectorExpressions.GetAllocatedSize()
    + UniformScalarExpressions.GetAllocatedSize()
    + Uniform2DTextureExpressions.GetAllocatedSize()
    + UniformCubeTextureExpressions.GetAllocatedSize()
    + ParameterCollections.GetAllocatedSize()
    + UniformBufferStruct
        ?
        (sizeof(FUniformBufferStruct) +
         UniformBufferStruct->GetMembers().GetAllocatedSize())
        :
        0;
}

PVS-Studio's diagnostic message: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. materialshared.h 224

This code is pretty complicated. To make the explanation clearer, I have composed a simplified artificial sample:

return A() + B() + C() + uniform ? UniformSize() : 0;

A certain size is being calculated in this code. Depending on the value of the uniform variable, either UniformSize() or 0 should be added. But the code actually works in quite a different way. The priority of the addition operators '+' is higher than that of the '?:' operator.

So here's what we get:

return (A() + B() + C() + uniform) ? UniformSize() : 0;

A similar issue can be found in Unreal Engine's code. I suspect the program calculates something different than the programmer wanted it to.

Mess-up with enum


I didn't feel like describing this case at first as I would have to cite quite a large piece of code. But then I overcame my laziness, so please be patient too.

namespace EOnlineSharingReadCategory
{
  enum Type
  {
    None          = 0x00,
    Posts         = 0x01,
    Friends       = 0x02,
    Mailbox       = 0x04,
    OnlineStatus  = 0x08,
    ProfileInfo   = 0x10,  
    LocationInfo  = 0x20,
    Default       = ProfileInfo|LocationInfo,
  };
}

namespace EOnlineSharingPublishingCategory
{
  enum Type {
    None          = 0x00,
    Posts         = 0x01,
    Friends       = 0x02,
    AccountAdmin  = 0x04,
    Events        = 0x08,
    Default       = None,
  };

  inline const TCHAR* ToString
    (EOnlineSharingReadCategory::Type CategoryType)
  {
    switch (CategoryType)
    {
    case None:
    {
      return TEXT("Category undefined");
    }
    case Posts:
    {
      return TEXT("Posts");
    }
    case Friends:
    {
      return TEXT("Friends");
    }
    case AccountAdmin:
    {
      return TEXT("Account Admin");
    }
    ....
  }
}

The analyzer generates a few V556 warnings at once on this code. The reason is that the switch operator has a variable of the EOnlineSharingReadCategory::Type type as its argument. At the same time, case operators work with values of a different type, EOnlineSharingPublishingCategory::Type.

A logical error


const TCHAR* UStructProperty::ImportText_Internal(....) const
{
  ....
  if (*Buffer == TCHAR('\"'))
  {
    while (*Buffer && *Buffer != TCHAR('\"') &&
           *Buffer != TCHAR('\n') && *Buffer != TCHAR('\r'))
    {
      Buffer++;
    }

    if (*Buffer != TCHAR('\"'))
  ....
}

PVS-Studio's diagnostic message: V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 310, 312. propertystruct.cpp 310

The programmer intended to skip all text in double quotes. The algorithm was meant to be like this:
  • Once the program comes across a double quote, a loop is started.
  • The loop keeps skipping characters until stumbling across the next double quote.
The error is about the pointer failing to be referenced to the next character after the first double quote is found. As a result, the second double quote is found right away, too, and the loop doesn't start.

Here is simpler code to clarify the point:

if (*p == '\"')
{
  while (*p && *p != '\"')
      p++;
}

To fix the error, you need to change the code in the following way:

if (*p == '\"')
{
  p++;
  while (*p && *p != '\"')
      p++;
}

Suspicious shift


class FMallocBinned : public FMalloc
{
  ....
  /* Used to mask off the bits that have been used to
     lookup the indirect table */
  uint64 PoolMask;
  ....
  FMallocBinned(uint32 InPageSize, uint64 AddressLimit)
  {
    ....
    PoolMask = ( ( 1 << ( HashKeyShift - PoolBitShift ) ) - 1 );
    ....
  }
}

PVS-Studio's diagnostic message: V629 Consider inspecting the '1 < (HashKeyShift - PoolBitShift)' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mallocbinned.h 800

Whether or not this code contains an error depends on whether the value 1 needs to be shifted by more than 31 bits. Since the result is saved into a 64-bit variable PoolMask, it seems highly probable.

If I am right, the library contains an error in the memory allocation subsystem.

The number 1 is of the int type, which means that you cannot shift it by 35 bits, for example. Theoretically, it leads to undefined behavior (find out more). In practice, an overflow will occur and an incorrect value will be computed.

The fixed code looks as follows:

PoolMask = ( ( 1ull << ( HashKeyShift - PoolBitShift ) ) - 1 );

Obsolete checks


void FOculusRiftHMD::Startup()
{
  ....
  pSensorFusion = new SensorFusion();
  if (!pSensorFusion)
  {
    UE_LOG(LogHMD, Warning,
      TEXT("Error creating Oculus sensor fusion."));
    return;
  }
  ....
}

PVS-Studio's diagnostic message: V668 There is no sense in testing the 'pSensorFusion' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. oculusrifthmd.cpp 1594

For a long time now the new operator has been throwing an exception in case of a memory allocation error. The if (!pSensorFusion) check is not needed.

I usually find quite a lot of such fragments in large projects, but Unreal Engine's code contains surprisingly few of them: ue-V668.txt.

Copy-Paste


The code fragments below have most likely appeared through the Copy-Paste method. Regardless of the condition, one and the same code branch is executed:

FString FPaths::CreateTempFilename(....)
{
  ....  
  const int32 PathLen = FCString::Strlen( Path );
  if( PathLen > 0 && Path[ PathLen - 1 ] != TEXT('/') )
  {
    UniqueFilename =
      FString::Printf( TEXT("%s/%s%s%s"), Path, Prefix,
                       *FGuid::NewGuid().ToString(), Extension );
  }
  else
  {
    UniqueFilename =
      FString::Printf( TEXT("%s/%s%s%s"), Path, Prefix,
                       *FGuid::NewGuid().ToString(), Extension );
  }
  ....
}

PVS-Studio's diagnostic message: V523 The 'then' statement is equivalent to the 'else' statement. paths.cpp 703

One more example:

template< typename DefinitionType >            
FORCENOINLINE void Set(....)
{
  ....
  if ( DefinitionPtr == NULL )
  {
    WidgetStyleValues.Add( PropertyName,
      MakeShareable( new DefinitionType( InStyleDefintion ) ) );
  }
  else
  {
    WidgetStyleValues.Add( PropertyName,
      MakeShareable( new DefinitionType( InStyleDefintion ) ) );
  }
}

PVS-Studio's diagnostic message: V523 The 'then' statement is equivalent to the 'else' statement. slatestyle.h 289

Miscellaneous


What's left is just diverse subtle issues which are not very interesting to discuss. So let me just cite a few code fragments and corresponding diagnostic messages.

void FNativeClassHeaderGenerator::ExportProperties(....)
{
  ....
  int32 NumByteProperties = 0;
  ....
  if (bIsByteProperty)
  {
    NumByteProperties;
  }
  ....
}

PVS-Studio's diagnostic message: V607 Ownerless expression 'NumByteProperties'. codegenerator.cpp 633

static void GetModuleVersion( .... )
{
  ....
  char* VersionInfo = new char[InfoSize];
  ....
  delete VersionInfo;
  ....
}

PVS-Studio's diagnostic message: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] VersionInfo;'. windowsplatformexceptionhandling.cpp 107

const FSlateBrush* FSlateGameResources::GetBrush(
  const FName PropertyName, ....)
{
  ....
  ensureMsgf(BrushAsset, TEXT("Could not find resource '%s'"),
             PropertyName);
  ....
}

PVS-Studio's diagnostic message: V510 The 'EnsureNotFalseFormatted' function is not expected to receive class-type variable as sixth actual argument. slategameresources.cpp 49

Conclusions


Using the static analyzer integrated into Visual Studio does make sense but it is not enough. The authors should consider using specialized tools in addition to it, for example our analyzer PVS-Studio. If you compare PVS-Studio to VS2013's analyzer, the former detects 6 times more bugs. Here you have the proof:

  1. Comparison of static code analyzers: CppCat, Cppcheck, PVS-Studio and Visual Studio;
  2. Comparison methodology.

I invite all those who want their code to be high-quality to try our code analyzer.

P.S. I should also mention that the errors described in this article (except for microoptimizations) could theoretically have been found by the lightweight analyzer CppCat as well. A one-year license for CppCat costs $250; annual renewal costs $200. But it wouldn't do in this particular case because it is lightweight and lacks the necessary functionality to monitoring compiler launches, which is a crucial requirement when checking Unreal Engine. However, the CppCat analyzer may well satisfy the authors of small projects.



License


GDOL (Gamedev.net Open License)




Comments

In the first found bug, I think it's actually two typo's (actually just a copy+paste bug).

static bool PositionIsInside(....)
{
  return
    Position.X >= Control.Center.X - BoxSize.X * 0.5f &&
    Position.X <= Control.Center.X + BoxSize.X * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f;
}

The last line, as you mentioned, should use a + not a - operator. But also, it should use a <= not a >= operator. Changing one without fixing the other would make your PVS-Studio error message go away but would still result in incorrect results.

Perhaps the best thing here would be to add some test cases to test the actual results of the function (in addition to static code analysis).

 

Really, kudos that your static code analyzer caught that "The nicest of all the errors". I glanced over it multiple times, and only noticed the "missing" closing parenthesis. The fact that that typo happened to compile is very unfortunate - I would never catch that because it visually looks perfect and compiles fine.

 

I'm also impressed that you can detect the "Mess-up with enum" bug and some of the others.

Sorry for being a nitpick in case the answer is an obvious yes, but since it's not expressly mentioned in the article: do you have permission from Epic to reproduce the UE4 code snippets in public?

Sorry for being a nitpick in case the answer is an obvious yes, but since it's not expressly mentioned in the article: do you have permission from Epic to reproduce the UE4 code snippets in public?

Of Course it's open source!

 

Sorry for being a nitpick in case the answer is an obvious yes, but since it's not expressly mentioned in the article: do you have permission from Epic to reproduce the UE4 code snippets in public?

Of Course it's open source!

 

No it is not.

 

From Epic games website:

 

"

  • Can I share the Unreal Engine source code or tools with others?

    You can share the source code or tools, along with any modifications you’ve made, with anyone who is an Unreal Engine licensee who is authorized to access the same version of the engine as yours, e.g. the 4.x.x version number of your installed build."

However I did like your article :)

It's not open source, but I doubt it could be a problem.

All of the code posted is meaningless without context (as it was heavily stripped away) and doesn't really expose any of the engine's inner workings.

Not to mention that the same article is posted here.

This article is basically just a big (and very convincing!) ad for PVS-Studio, but it's a great read nonetheless and makes it clear how useful static code analysis is!

It's not open source, but I doubt it could be a problem.

All of the code posted is meaningless without context (as it was heavily stripped away) and doesn't really expose any of the engine's inner workings.

Oh that I agree with. I just wanted to point out that the Unreal 4 Source Code is not Open Source :)

I think pointing out these errors is in somewhat poor taste.  Add in that it was done to further their own goals makes it worse.

This article is basically just a big (and very convincing!) ad for PVS-Studio, but it's a great read nonetheless and makes it clear how useful static code analysis is!

I agree. I like the detailed explanations for each bug, that way you can learn to avoid such mistakes.

I think pointing out these errors is in somewhat poor taste.  Add in that it was done to further their own goals makes it worse.


How so? Real world examples are the best kind of examples. Epic even get some of their bugs fixed for free, everyone wins. Still, it highlights as others have said to have SA as part of the development arsenal.

I am a big fan of static code analysis and routinely use such tools in my indie projects. This is not the first PVS-Studio advertisement for gamedev communities, and in general the takeaway message "there are bugs even in UE4 codebase that PVS-Studio static analysis can catch" is nice and convinces that the tool is good, but not quite enough to lead to a buying decision.

 

For people interested in C/C++ static analysis tools, there are several that are free:

  • VS2012&VS2013 /analyze option (that was mentioned in this article). Feels like the weakest static analyzer out there, but understandable, as it is the youngest analyzer project in development.
  • cppcheck - in my experience finds much more issues than /ANALYZE, but also some false positives.
  • Clang static analyser (scan-build) - probably the strongest static analyzer out there.

Given that these tools are free, and PVS-Studio is a commercial one, I would like to see for an article like this to compare how PVS-Studio performs against these free tools on real-world codebases like UE4? If I am already running routinely with the three above tools, is there any worth in paying for PVS-Studio?

 

As a second question, there are also the following excellent free runtime analyzer tools:

  • Visual Leak Detector: memory leak tracker for Visual Studio.
  • Valgrind: runtime udb analysis tool for linux.
  • Clang Sanitizers: AddressSanitizer, ThreadSanitizer, MemorySanitizer, LeakSanitizer - an excellent suite of runtime checkers.

Is PVS-Studio capable of surpassing the combined strength of these free static+dynamic analyzers?

 

And finally, I find odd is that the price of the tool is "please write us to ask for a quote", and is passed out in one-year annual licenses. This does not look like a pricing model specifically targeted towards indie developers like the GameDev.net community is, but more like a strategy towards larger business-to-business clients. Even after reading an excellent article like this at GameDev.net, the pricing model really puts me off, especially since I am not sure how well it would augment the above existing free tools in my toolbox. Nevertheless, this was an interesting read, thanks for the writeup!

It is open source, it's just not free.

 

In detail: How we compared code analyzers.

 
 
We have no comparison with Clang. But we find errors in Clang: Clang check (August 2011), second check (August 2012).
 
 
2. Dynamic analysis.
 
I just recently wrote an article on this topic: Static and Dynamic Code Analysis.
 
 
3. For indie developers we recommend CppCat tool: An Alternative to PVS-Studio at $250.

Thanks for the answers Karpov, very useful!

I'm disappointed this article is not actually about unreal engine (as other have already pointed out it's really just marketing material for PVS-Studio)

 

The marketing material is interesting and reads nicely, but it would IMO have been more trustworthy, with a name/ description/ introduction that identified this article as a use case for PVS-Studio on large projects. (I guess what I'm trying to say is I would have preferred the article not to pass itself off for something it isn't)

From coach outlet,coach factory,coach factory outlet,coach outlet store,coach outlet store online,coach factory online,coach factory outlet online,coach outlet online satellite photos speculate louboutin,louboutin outlet,louboutin outlet italia that the test device is about 120-150 meters long, about 80 abercrombie and fitch,abercrombie,abercrombie and fitch UK meters long electromagnetic rail, tory burch outlet,tory burch,tory burch handbags,tory burch shoes,tory burch sale,toryburch,tory burch sandals,toryburch.com,tory burch flip flops able to build such a large scale polo ralph lauren outlet online,ralph lauren,polo ralph,polo ralph lauren,ralph lauren outlet,polo shirts,ralph lauren outlet online,polo ralph lauren outlet,ralphlauren.com,polo outlet,ralph lauren polo experimental vans shoes,vans outlet,vans store,star wars vans shoes,cheap vans,vans shoes outlet,white vans,black vans,red vans,vans star wars,vans sneakers,vans shoes outlet store,vans sale,cheap vans shoes facilities "proves that China has been fully validated and mastered a large linear induction motors, advanced energy storage devices vans,vans scarpe,vans italia forced burberry outlet online,burberry,burberry outlet,burberry handbags,burberry factory outlet,burberry sale and air force,nike air force,air force 1,air force one,nike air force 1,nike air force one,air force one nike high-performance pulse generator key prada outlet,prada,prada handbags,prada sunglasses,prada shoes,prada bags technologies such as electromagnetic catapults. "Compared to the steam catapult, the advantages of electromagnetic catapult michael kors,michael kors canada,michael kors outlet,michael kors outlet canada is comprehensive, first accelerating force michael kors handbags,michael kors outlet,michael kors outlet online,michael kors,kors outlet,michael kors outlet online sale,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors uniform and controllable. "Nimitz" when C-13-1-type steam catapults louis vuitton outlet,louis vuitton outlet online,louis vuitton,louisvuitton.com,authentic louis vuitton,louis vuitton factory outlet,cheap louis vuitton on the emission oakley sunglasses,cheap oakley sunglasses,oakley sunglasses cheap,oakley vault,oakleys,oakley.com,sunglasses outlet,cheap oakley,oakley outlet,cheap sunglasses,oakley prescription glasses,fake oakleys,oakley sunglasses outlet,oakley glasses,oakley store,fake oakley,oakley sale,cheap oakleys,discount oakley sunglasses level can birkin bag,hermes belt,hermes handbags,hermes birkin,hermes bags,birkin bags reach the maximum overload 6g, while the average acceleration of the kate spade handbags,kate spade,kate spade outlet,katespade whole trip nike air max,air max,air max pas cher,air max one,air max 90,air max france is just a little more than 2g. F / A-18 fighter pilots often ridicule C-13-1 in the rear section catapult air max,nike air max themselves often do not have the louis vuitton,borse louis vuitton,louis vuitton sito ufficiale,louis vuitton outlet aircraft engine longchamp,longchamp bags,longchamp uk acceleration "to force." With the speed and cylinder volume increases, the vast majority of the expansion of chi flat iron,chi hair the superheated steam for steam energy to accelerate and promote themselves, and gas expansion increased hollister,hollister uk,abercrombie,abercrombie fitch,abercrombie and fitch the proportion of steam required increase in volume after the establishment party relationships. Steam catapult length and cylinder volume almost reached its limit, until the end of the ejection stroke, steam basically only accelerated piston aircraft little help.Electromagnetic catapult launch thrust segment is not the kind of sudden explosive impact of steam, air max,nike air max,air max 90,nike air max 90,air max 1 the peak air jordan,jordan pas cher,air jordan pas cher,nike air jordan,air jordan france overload can nfl jerseys,jerseys,baseball jerseys,cheap jerseys,nba jerseys,hockey jerseys,basketball jerseys,jerseys from china,cheap nfl jerseys be reduced from 6g to 3g, which polo ralph lauren outlet,ralph lauren,polo ralph,polo ralph lauren,ralph lauren outlet,polo shirts,ralph lauren outlet online,polo ralph lauren outlet online,ralphlauren.com,polo outlet,ralph lauren polo will not only help toms outlet to extend ray ban,rayban,occhiali ray ban the life of the herve leger,herve leger dresses aircraft, the pilot's body to feel is a good improvement. In addition, since the electromagnetic sac longchamp,longchamp,longchamps,longchamp pas cher,sac longchamp pas cher,longchamp pliage,longchamp soldes,sac longchamps,longchamp france catapult launch acceleration and length does not matter, in addition to being the aerodynamic drag and friction affect, louboutin,christian louboutin,red bottom shoes,louboutin shoes,red bottoms,louboutin outlet,christian louboutin shoes,christian louboutin outlet,red bottom shoes for women,louboutins the catapult into the iphone 6 cases,iphone 6 case,iphone 5c cases,iphone 5s cases,iphone cases,iphone case,iphone 5 cases,iphone 4s cases,iphone 4 cases,ipad cases,ipad mini cases,ipad air cases,galaxy s5 cases,galaxy s4 cases,phone cases early part of the last paragraph of the basic acceleration will not fluctuate too much, which is a gradual decline over the steam catapult more and more efficiency. According to calculations, the average acceleration of the same, electromagnetic catapults than nike free,nike free run,free running,free run,nike running steam catapults make aircraft more than 8-15% load. hogan,hogan outlet,scarpe hogan,hogan sito ufficiale,hogan interactive When the ejection of different weights, different models takeoff speed, electromagnetic catapult catapult also flexible and accurate converse pas cher,converse control of ralph lauren,ralph lauren uk,ralph lauren outlet,polo ralph lauren outlet energy through the computer.Electromagnetic catapult ejection hollister,hollister canada,abercrombie and fitch,abercrombie,abercrombie and,abercrombie kids,af efficiency is easton bats also higher than the steam michael kors outlet online,michael kors,kors outlet,michael kors outlet,michael kors handbags,michael kors outlet online sale,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors catapult, to enhance the nike huarache,nike huaraches,nike air huarache sense that major combat aircraft, such as christian louboutin shoes,louboutin shoes,louboutin outlet,louboutin,christian louboutin,red bottom shoes,red bottoms,christian louboutin outlet,red bottom shoes for women,louboutins the use vanessa bruno,sac vanessa bruno,vanessa bruno sac,vanessa bruno pas cher,cabas vanessa bruno,sac vanessa bruno en solde,sac vanessa bruno pas cher of electromagnetic catapult aircraft carrier-based Sunrise Ford momentum true religion from "Nimitz" coach outlet,coach factory outlet,coach outlet store,coach factory,coach outlet store online,coach factory online,coach factory outlet online,coach outlet online class of 120 jimmy choo,jimmy choo shoes,jimmy choo outlet,jimmy choo handbags sorties to 160 sorties: new balance shoes,new balance,balance shoes,new balance outlet,new balance store,new balance store locator,new balance shoes for women,joe's new balance outlet,newbalance,newbalance.com,new balance walking shoes Sunrise peak momentum from the air max,nike air max,air max 2015,nike air max 2015,air max 90,airmax,air max 95,nike air max 90 original 220-240 vehicles to 270 vehicles. At the same time, the hollister,abercrombie absence of pandora jewelry,pandora charms,pandora bracelet,pandora bracelets,pandora rings,pandora jewelry store locator,pandora charm,pandora charms clearance,pandora store,pandora jewelry outlet store,pandora jewelry sale online a large cylinder, the volume of the electromagnetic catapult is relatively michael kors outlet,michael kors outlet online,michael kors,kors outlet,michael kors handbags,michael kors outlet online sale,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors small, supra shoes very beneficial to the swarovski uk "Liaoning" class platform installation.In recent years, China's electronic technology valentino shoes,valentino,valentinos weaponry rapid progress. By 2020 there converse,scarpe converse,converse italia,converse sito ufficiale,converse all star six years, Ma Weiming nike trainers,nike trainers uk academician and his beats by dre,dr dre,beats headphones,dre beats,beats by dr,beats by dr dre,beats audio,dr dre beats,dre headphones,beats by dre headphones,beats by dr. dre,cheap beats research team there kate spade outlet,kate spade,katespade,kate spade handbags is still time, the oakley,occhiali oakley,oakley italia Chinese navy as a steam catapult can cross this threshold, so the first one sac louis vuitton,louis vuitton,louis vuitton pas cher,sac louis vuitton pas cher,louis vuitton france domestic carrier mounted montre pas cher directly on 2-3 sets of electromagnetic catapults, "Liaoning "saturation volume level karen millen dresses on the 2nd carrier longchamp,longchamp handbags,longchamp outlet,longchamp bags,long champ aircraft ship will likely rise to around 65, the amount of actual service burberry outlet online,burberry outlet,burberry,burberry handbags,burberry factory outlet,burberry sale carrier aircraft louis vuitton,louis vuitton bags,louis vuitton handbags,louis vuitton uk can reach the size of 55, plus three times now mulberry,mulberry handbags,mulberry outlet,mulberry bags,mulberry uk and can be loaded oakley sunglasses,oakley vault,oakley sunglasses cheap,oakleys,oakley.com,sunglasses outlet,cheap oakley,cheap oakley sunglasses,oakley outlet,cheap sunglasses,oakley prescription glasses,fake oakleys,oakley sunglasses outlet,oakley glasses,oakley store,fake oakley,oakley sale,cheap oakleys,discount oakley sunglasses with petrol bombs attendance combat new michael kors outlet store,michael kors outlet,michael kors outlet online,michael kors,kors outlet,michael kors handbags,michael kors outlet online sale,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors carrier machine. Ship "new wine in old bottles" operational capability hollister,abercrombie,hollister sito ufficiale,abercrombie italia,abercrombie and fitch,abercrombie outlet of the aircraft carrier is expected to rolex watches,replica watches,omega watches,rolex watches for sale,replica watches uk,fake rolex reach "Forrester" class and the "Kitty Hawk" grade level.Nuclear fantasy and realityIf the "Liaoning" on the 2nd class destroyers finally tiffany and co jewelry,tiffany and co outlet,tiffany and co,tiffany's,tiffanys,tiffany co,tiffany jewelry adopted electromagnetic catapult, you bottega veneta,bottega,bottega veneta outlet must do the appropriate upgrade rolex watches,rolex,watches for men,watches for women,omega watches,replica watches,rolex watches for sale,rolex replica,rolex watch,cartier watches,rolex submariner,fake rolex,rolex replica watches,replica rolex its power systems, electromagnetic catapults need strong power protection, vans,vans pas cher,vans soldes especially louboutin outlet,louboutin,christian louboutin,red bottom shoes,louboutin shoes,red bottoms,christian louboutin shoes,christian louboutin outlet,red bottom shoes for women,louboutins in high-voltage power supply capacity of the instantaneous ejection time, which is probably "Liaoning" No existing power system can not provide."Admiral Kuznetsov" was thomas sabo uk adopted four TB-12 true religion steam turbine and eight supercharged boiler. TB-12's predecessor is TB-8, and TB-8 is just the louboutin,chaussure louboutin,louboutin pas cher,chaussures louboutin,chaussure louboutin pas cher,louboutin france domestic 051 destroyers host Domestic Model 453 type air max,nike air max,air max pas cher,air max one,air max 90,air max france steam turbine. TB-8 coach purses,coach handbags,coach bags power is 36,000 horsepower, 051 destroyers then adopted two sets of TB-8 steam turbine speed was 38 knots, power TB-12 increased to 45,000 horsepower. In addition, China converse shoes,converse,converse.com,converse sneakers,converse outlet introduced the "modern" GTZA-674 type steam turbine installed on class instyler ionic styler,instyler destroyer is improved longchamp handbags,longchamp,longchamp outlet,longchamp bags,long champ TB-12's.TB-8 from the mcm handbags,mcm bags,mcm backpack,mcm outlet TB-12 and michael kors outlet online sale,michael kors,kors outlet,michael kors outlet,michael kors handbags,michael kors outlet online,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors then GTZA-674, China in the series steam turbine has accumulated a wealth of technology, use and maintenance experience. If the steam timberland outlet turbine on "Liaoning" was used for the localization model, I believe, at least up to the level of GTZA-674, michael kors,sac michael kors,michael kors pas cher,sac michael kors pas cher,michael kors france with louis vuitton,louis vuitton canada,louis vuitton outlet,louis vuitton outlet online a coach outlet store online,coach outlet store,coach outlet total output power of north face,the north face,north face uk,north face jackets,north face outlet,northface four steam turbines for 200,000 horsepower. The power tiffany and co jewelry,tiffany and co outlet,tiffany and co,tiffany's,tiffanys,tiffany co,tiffany jewelry system maintains no catapult "Liaoning" No high speed sailing is no problem, but if it is using a new electromagnetic catapult aircraft on some stretched up. "Ford" was only four electromagnetic catapult while charging it takes about north face jackets,north face,the north face,northface,north face outlet,north face jackets clearance,the north face outlet 200,000 horsepower, if the "Liaoning" on the 2nd class destroyers to install ray ban sunglasses outlet,ray ban sunglasses,ray ban,rayban,ray bans,ray ban outlet,ray-ban,raybans,ray ban wayfarer,ray-ban sunglasses,raybans.com,rayban sunglasses,cheap ray ban three links of london uk sets of the same power sac guess,guess pas cher,guess,guess collection,sac a main guess of the electromagnetic catapult, the required electrical energy that is over 16,000 horsepower; hollister taking into steam turbine power generation process in about 50% of energy nike air max,air max,air max 2015,nike air max 2015,air max 90,airmax,air max 95,nike air max 90 consumption, only lululemon outlet,lululemon,yoga pants,lulu lemon,lulu.com,lululemon.com three catapult babyliss pro,babyliss maximum prada handbags,prada,prada sunglasses,prada shoes,prada outlet,prada bags required lancel power accounted for about 16% of the total power of the ship, which is bound to gucci outlet,gucci handbags,gucci belts,gucci shoes,gucci,gucci belt,gucci sunglasses,gucci bags,cheap gucci seriously ghd,ghd hair,ghd hair straighteners,ghd straighteners affect the use of aircraft speed and lacoste,polo lacoste,lacoste pas cher,lacoste.fr,lacoste soldes,survetement lacoste other electrical equipment.More insanity,insanity workout,insanity workout calendar,insanity calendar,insanity workout schedule vigilance is needed, longchamp outlet,longchamp,longchamp handbags,longchamp bags,long champ whether it is "Admiral Kuznetsov" was still in India, "Vic Rama Tia" No, they are installed roshe run,nike roshe,roshe runs,nike roshe run,nike roshes TB-12 ray ban sunglasses,ray ban sunglasses outlet,ray ban,rayban,ray bans,ray ban outlet,ray-ban,raybans,ray ban wayfarer,ray-ban sunglasses,raybans.com,rayban sunglasses,cheap ray ban steam turbine and auxiliary boilers occurred many times in the course of giuseppe zanotti,giuseppe zanotti outlet major accident. Russia louis vuitton,louis vuitton outlet online,louis vuitton outlet,louisvuitton.com,authentic louis vuitton,louis vuitton factory outlet,cheap louis vuitton has north face outlet,north face,the north face,northface,north face jackets,north face jackets clearance,the north face outlet a rich pandora jewelry,pandora charms,pandora bracelet,pandora bracelets,pandora rings,pandora jewelry store locator,pandora charm,pandora charms clearance,pandora store,pandora jewelry outlet store,pandora jewelry sale online ship with a steam turbine development and experience like this, the reliability of the hermes,sac hermes,hermes pas cher,sac hermes pas cher TB-12 based on the development of China-made large true religion ship with the first soccer shoes,nike mercurial generation of louboutin,christian louboutin,louboutin shoes,louboutins,louboutin uk,christian louboutin uk,red bottom shoes,red bottoms,louboutin outlet,christian louboutin shoes,christian louboutin outlet steam turbines also needs further examination.Needless to say, the best use of the wedding dresses,wedding dress,dresses for wedding,bride dresses,dresses for weddings,wedding dresses uk,cheap wedding dresses,vintage wedding dresses,monsoon wedding dresses,lace wedding dresses,wedding dresses for older brides,wedding dresses 2014 electromagnetic catapult powered aircraft carrier lululemon,lululemon canada,lululemon outlet canada,lululemon outlet online program is certainly similar to "Ford" class nike roshe run,roshe run,nike roshe,rosh run,roshe run pas cher,nike roshe france nuclear-powered + all-electric propulsion tn pas cher,nike tn,nike tn pas cher,nike tn requin,tn requin,tn requin pas cher system. Nuclear coach factory outlet,coach factory,coach factory outlet online,coach factory online energy juicy couture can provide catapult inexhaustible; wedding dresses,prom dresses,bridesmaid dresses,evening dresses,beach wedding dresses,cheap wedding dresses,homecoming dresses,prom dresses,wedding dresses and the all-electric propulsion system eliminates ray ban pas cher,ray ban,rayban,lunette ray ban pas cher the mechanical transmission mechanism, the reactor will supply energy into louis vuitton outlet stores,louis vuitton outlet online,louis vuitton,louis vuitton outlet,louisvuitton.com,authentic louis vuitton,louis vuitton factory outlet,cheap louis vuitton electrical energy directly to the catapult, propeller motors, timberland,timberland pas cher,chaussure timberland pas cher,timberland france as well as other power line carrier terminal, without the huge transmission layout is very conducive to aircraft interior design, handling and juicy couture outlet maintenance is also very convenient. Of course, louis vuitton outlet online,louis vuitton,louis vuitton outlet,louisvuitton.com,authentic louis vuitton,louis vuitton factory outlet,cheap louis vuitton the all-electric propulsion system for the energy chanel handbags,chanel bags,chanel sunglasses,chanel purses,chanel outlet conversion efficiency is generally better than mechanical transmission, with burberry,sac burberry,burberry pas cher,sac burberry pas cher,burberry france endless nike blazer,blazer nike,nike blazer pas cher energy asics,asics gel,asics running,asics running shoes,asics shoes,asics gel nimbus,asics gel kayano,asics gt reactors ralph lauren,polo ralph lauren,ralph lauren outlet,ralph lauren italia,ralph lauren sito ufficiale is undoubtedly the most suitable partner.Real possibility of view, "Liao in" class on the 2nd ship a "nuclear" nike roshe,roshe run,nike roshe run,roshe runs power system louis vuitton,sac louis vuitton,louis vuitton pas cher,sac louis vuitton pas cher,louis vuitton france is unlikely. Although Russia discuss the "Admiral Kuznetsov" No. overhaul conversion program, the dress had been discussed nuclear power, eventually dropped. I reebok outlet,reebok,reebok skyscape,reebok shoes will not speak on the basic platform "Liaoning" was doing true religion so beating ferragamo shoes,ferragamo,salvatore ferragamo,ferragamo belts,ferragamo belt,ferragamo outlet the availability of the technical feasibility of internal transformation, China I am afraid there is no ready-made high-performance aircraft for use nike shoes,nike outlet,nike factory,nike store,nike factory outlet,nike outlet store,cheap nike shoes,nike sneakers pressurized water reactor. The new reactors on 094 strategic nuclear submarines mac cosmetics,m a c cosmetics,mac makeup,maccosmetics.com reportedly good performance, celine handbags,celine bag,celine bags there has been no reliable sources have new balance pas cher,new balance,new balance femme confirmed that the boat has been the longchamp,sac longchamp,longchamps,longchamp pas cher,sac longchamp pas cher,longchamp pliage,longchamp soldes,sac longchamps,longchamp france formation of combat, the maturity of its power systems doubtful. We should also not forget that the abercrombie and fitch,abercrombie and,abercrombie,abercrombie kids,abercrombie fitch,abercrombie.com risk ray ban sunglasses uk of north face,the north face,north face pas cher,north face soldes,north face france boats reactors ship with p90x,p90x3,p90x workout,p90x workout schedule,p90x workout sheets,p90x 3,p90x2 a great change, oakley sunglasses cheap,cheap oakley sunglasses,oakley sunglasses,oakley vault,oakleys,oakley.com,sunglasses outlet,cheap oakley,oakley outlet,cheap sunglasses,oakley prescription glasses,fake oakleys,oakley sunglasses outlet,oakley glasses,oakley store,fake oakley,oakley sale,cheap oakleys,discount oakley sunglasses the air max US nuclear aircraft carriers and nuclear submarines had never shared michael kors,michael kors uk,michael kors handbags,michael kors bags reactor; France has the world's top nuclear technology in michael kors outlet,michael kors,kors outlet,michael kors handbags,michael kors outlet online,michael kors outlet online sale,michael kors handbags clearance,michael kors purses,michaelkors.com,michael kors bags,michael kors shoes,michaelkors,cheap michael kors the construction of "Charles de swarovski jewelry Gaulle" when granted directly jordan shoes,jordans,air jordan,jordan retro,jordan 11,jordan xx9,jordan 6,new jordans,air jordans,cheap jordans,retro jordans,jordan retro 11,jordan 5,air jordan 11,jordans for sale,jordan 4,jordan 1,jordan future,jordan 3,jordan 12,michael jordan shoes,air jordan shoes,air jordan retro to the intended Reactor "Triumph" class gucci,borse gucci,gucci sito ufficiale,gucci outlet strategic nuclear submarine used louis vuitton handbags,vuitton handbags,louis vuitton bags,louis vuitton purses in the hollister,abercrombie,abercrombie fitch,hollister france,hollister pas cher,abercrombie and fitch,hollister pas cher "Charles de Gaulle". nike air max,air max,air max 90,nike air max 90,air max 1 K15 natural circulation integral nike free,free run,nike free run,nike free pas cher,nike free run pas cher,nike free france PWR technology really advanced in the "Triumph" class did a great job, but mounted on the "Charles de Gaulle" pandora charms,pandora uk,pandora bracelet,pandora rings,pandora,pandora sale,pandora bracelets,pandora jewellery,pandora ring,pandora charm,pandora earrings,pandora jewelry,pandora necklace,pandora charms uk is the "acclimatization", resulting oakley pas cher,oakley,oakley soldes,lunette oakley pas cher,oakley france in "Charles de Gaulle" period is prolonged, the installation of mont blanc,montblanc,mont blanc pens pressure cabin dead weight after barely put to use, ralph lauren,polo ralph lauren,ralph lauren pas cher,polo ralph lauren pas cher,ralph lauren france but still in the sea trials and follow-up service during soccer jerseys,soccer jersey,usa soccer jersey,football jerseys times of nike free,free running,nike free run,nike free 5.0,free running 2,nike running shoes,nike free trainer,nike free trainer 5.0,free runs,free run 5.0 exposure problems.


Note: Please offer only positive, constructive comments - we are looking to promote a positive atmosphere where collaboration is valued above all else.




PARTNERS