Pull to refresh
281.78
PVS-Studio
Static Code Analysis for C, C++, C# and Java

How the Carla car simulator helped us level up the static analysis of Unreal Engine 4 projects

Reading time 17 min
Views 1.3K

One of the mechanisms of static analysis is method annotations of popular libraries. Annotations provide more information about functions during errors detecting. CARLA is an impressive open-source project in C++ that helped us implement this mechanism to our analyzer. Subsequently, the simulator became a test-target for the improved PVS-Studio static analyzer.


0888_Carla/image2.png


Introduction


CARLA is an open-source simulator for autonomous driving research. CARLA has been developed from the ground up to support development, training, and validation of autonomous driving systems. In addition to open-source code and protocols, CARLA provides open digital assets (urban layouts, buildings, vehicles) that were created for this purpose and can be used freely. The simulation platform supports flexible specification of sensor suites and environmental conditions.


The project is cross-platform and contains almost 78,000 lines of C++ code. In the project repository, we also found code written in Python, XML, YAML, DOS Batch, CMake and other languages.


Language files blank comment code
C++ 266 10334 5016 50325
C/C++ Header 447 10091 7176 27595
Python 7 4870 3651 19281
XML 9 30 4 7603
YAML 0 157 852 5759
DOS Batch 20 627 411 2462
Bourne Shell 18 778 332 1922
CMake 6 133 37 474
C# 5 47 30 276
CSS 1 40 22 206
JSON 2 0 0 137
make 2 10 0 60
Java 1 58 151 59
Javascript 1 0 0 9
SUM 905 27175 17682 116168

Static code analysis is the process of detecting errors and defects in a software's source code. Static analysis can be viewed as an automated code review process. One of the technologies used in static analysis is function annotations of popular libraries. The developer studies the documentation of such functions and notes facts useful for analysis. During the program check, the analyzer takes these facts from the annotations. This approach allows the analysis to be carried out with higher accuracy.


The result of checking projects — a report with warnings. In PVS-Studio, you can open the report in text editor or in the analyzer utility. It's possible to open reports in software development tools, such as Visual Studio or CLion, but it requires the use of appropriate plugins. Further the article will show you the top 10 errors found in the CARLA project. You can also test your skills and try detecting them yourself.


Building and analysis


To manage the build process in Unreal Engine, use their custom build system — Unreal Build Tool. Therefore, the analysis of projects written on the Unreal Engine is performed in a special way. There are two options for checking UE projects:


  1. analysis using Unreal Build Tool integration;
  2. analysis using compiler monitoring.

CARLA uses a modified Unreal Engine 4 kernel, which is also available on GitHub. However, both the original and modified kernel have private access. Building on Windows consists of two stages: building the engine and building the project itself. We will see how to analyze both.


Unreal Engine 4 Build


You can build Unreal Engine 4 in 8 steps.


  1. Register for an Epic Games account.
  2. Link your GitHub account to your Epic Games account.
  3. Accept an invitation to GitHub from Epic Games. After that, you will get an access to the Unreal Engine repository.
  4. Download the modified kernel repository.
  5. Run the Setup.bat and GenerateProjectFiles.bat configuration scripts.
  6. Open the UE4.sln solution generated in Visual Studio 2019.
  7. Select the Development Editor configuration and the Win64 platform.
  8. Build the project.

Unreal Engine 4 analysis


To check the engine, integrate static analysis into the Unreal Build Tool assembly system. To perform the analysis and get the check results, you need to perform the following steps.


  1. Install PVS-Studio if you haven't done so. Plugins for all versions of Visual Studio install automatically.
  2. In Visual Studio, open the Project Properties and go to the NMake tab.
  3. In the Build Command Line field, add -Staticanalyzer=PVSStudio at the very end. You can do the same for the Rebuild Command Line field.
  4. Build the project.
  5. In Visual Studio menu bar, select: Extensions -> PVS-Studio -> Open/Save -> Open Analysis Report.
  6. In the explorer window that opens, select the *\Engine\Saved\PVS-Studio\shadercompileworker.pvslog file, where '*' is the path to the Unreal Engine 4 folder.

0888_Carla/image4.png


As a result, instead of the project building or rebuilding, PVS-Studio performs the source code analysis. Now let's build the CARLA simulator itself.


CARLA build and analysis


The project does not generate a solution. This doesn't allow us to integrate into the Unreal Build Tool. So, let's check the project through compiler monitoring. There are two ways to do this:


  • use the command line utility — CLMonitoring.exe;
  • use the C and C++ Compiler Monitoring UI IDE.

0888_Carla/image6.png


Both utilities are already in the C:\Program Files (x86)\PVS-Studio folder after installing PVS-Studio. Let's use the second option — C and C++ Compiler Monitoring UI IDE. To start build process, follow the steps:


  1. Download the project repository from GitHub.
  2. Run Update.bat to download resources. Unpack them using 7zip.
  3. Set the UE4_ROOT environment variable with the path value to the Unreal Engine kernel folder.
  4. Run C and C++ Compiler Monitoring UI. In the main menu, select Tools -> Analyze your files (C and C++). In the window that opens, click Start Monitoring. After that, another compiler monitoring window will appear.
  5. Open x64 Native Tools Command Prompt for VS 2019 and go to the folder where CARLA is located.
  6. Run the make PythonAPI command to build the client.
  7. Run the make launch command to build the server.
  8. Click the Stop Monitoring button in the compiler monitoring window. Within seconds, the analysis based on the gathered information will start. The report is downloaded automatically.

To view the analyzer warnings easily, you can use Visual Studio. Open the folder with the CARLA repository and download the report. It may be useful to filter warnings issued on kernel files, autogenerated files and included library files. To do this, perform a few more actions:


  1. In C and C++ Compiler Monitoring UI, in the menu bar, select Save PVS-Studio Log As and specify the path to save the report.
  2. In Visual Studio, in the menu bar, select Extensions -> PVS-Studio -> Open/Save -> Open Analysis Report and specify the same path as in the previous step.
  3. In Visual Studio, in the menu bar, select Extensions -> PVS-Studio -> Options.
  4. In the window that opens, go to PVS-Studio -> Don't Check Files.
  5. Add the *.gen.* mask to the FileNameMasks group.
  6. Add the path to the Unreal Engine 4 folder to the PathMasks group.
  7. Add the *\Unreal\CARLAUE4\Plugins\CARLA\carladependencies\include\boost\* path to the PathMasks group, where '\' — the path to the CARLA repository folder.

Now let's study the analyzer warnings in Visual Studio. Let's start with warnings issued on CARLA simulator code and their own libraries.


0888_Carla/image7.png


We will view the errors found in the CARLA source files a little later. The point is, we needed to check this project for another task. Before testing the simulator, we slightly modified the PVS-Studio kernel so that it collects statistics of Unreal Engine 4 method calls. This data can now help us with annotating.


Method annotation


0888_Carla/image8.png


Annotation is performed in two stages:


  1. studying library methods;
  2. recording useful facts about these methods in a special format that analyzer understands.

At the next check of the project, information about the annotated methods that you encounter in the code will be obtained both from function signatures and annotations.


For example, an annotation may suggest that:


  • a function parameter cannot be a null pointer (for example, the first or second parameter of strncat);
  • a function parameter specifies the number of elements or the number of bytes (for example, the third parameter of strncat);
  • two different parameters cannot receive the same value (for example, the first and second parameters of strncat );
  • a parameter is a pointer by which the memory allocated by the function will be returned;
  • a return value of the function must be used (for example, the strcmp function);
  • a function has or does not have an internal state;
  • a function can return nullptr (for example, the malloc function);
  • a function returns a pointer or a reference to the data (for example, the std::string::c_str function);
  • a function returns the iterator to a potentially invalid position (for example, std::find);
  • a function frees some resource (for example, the std::basic_string::clear function);
  • a function behaves like memcpy (for example, the qMemCopy function);
  • and many more useful things.

Which annotation would be the most useful? It's a good question. Let's find out in the comments below.


Annotations not only help to detect new errors, but also allow you to exclude some false positives.


What did we need the CARLA simulator for? To take and annotate all the Unreal Engine 4 functions is a very large-scale task. It requires a lot of time. Someday, maybe, we will power through it, but now we decided to start small and see the results. In order not to take 200 random engine functions, we decided to identify the most popular ones. We found a couple of large projects. They are rather outdated Unreal Tournament game and the currently supported CARLA simulator. The simulator in C++ suited us for the following reasons:


  • it's an open source project;
  • it has an up-to-date kernel (UE4 version 4.27);
  • it's a large-sized project (according to the authors, it takes about 4 hours to complete the build);
  • it offers an easy build and a detailed tutorial.

So, we selected the projects. We successfully completed the build and checked the projects. What's next? Now we need to collect statistics on functions calls of the game engine. How to do that — that is the question. Fortunately, we have the analyzer source code at hand. The analyzer builds a parse tree and allows us to find function calls with all the necessary information. So, it was enough to write something similar to a new diagnostic. The function suited us if two conditions were met:


  • a function is called from a file that belongs to the CARLA project;
  • a function declaration must be in a file that belongs to Unreal Engine 4.

If both conditions were met, information was recorded in a separate file. All we had to do was run the analysis with a modified kernel. After the analysis, we received a log of functions. Then we applied some simple formulas in Excel and converted the statistics to the following form:


0888_Carla/image10.png


We decided that for a start it is enough to annotate all the functions that we encountered more than 10 times. There were about 200 of them. Since developers don't really like to document code, we had to study the implementation of each Unreal Engine 4 function in the source code to annotate it. As an example, here is an annotation of the ConstructUFunction function:


C_"void ConstructUFunction(UFunction*& OutFunction, \
                           const FFunctionParams& Params)"
ADD(HAVE_STATE | RET_SKIP | F_ARG_ALLOC,
    "UE4CodeGen_Private",
    nullptr,
    "ConstructUFunction",
    ALLOC_ARG, SKIP);

The F_ARG_ALLOC flag means that the function allocates the resource and gives it back through one of its parameters. The ALLOC_ARG flag indicates that a pointer to the allocated resource is returned through the first parameter of the function, namely OutFunction. The SKIP flag says that the second argument of the function is not special and uninteresting for us.


After we annotated all the functions, we double-checked the CARLA simulator and the version of the engine the simulator uses. As expected, some of the false positives disappeared and several new warnings appeared.


New warning N1


V611 The memory was allocated using 'new' operator but was released using the 'free' function. Consider inspecting operation logics behind the 'Allocation' variable. Check lines: 1746, 1786. BulkData2.cpp 1746


void FBulkDataAllocation::SetMemoryMappedData(
  FBulkDataBase* Owner,
  IMappedFileHandle* MappedHandle,
  IMappedFileRegion* MappedRegion)
{
  ....
  FOwnedBulkDataPtr* Ptr
    = new FOwnedBulkDataPtr(MappedHandle, MappedRegion);      // <=

  Owner->SetRuntimeBulkDataFlags(BULKDATA_DataIsMemoryMapped);

  Allocation = Ptr;                                           // <=
}

void FBulkDataAllocation::Free(FBulkDataBase* Owner)
{
  if (!Owner->IsDataMemoryMapped())
  {
    FMemory::Free(Allocation);                                // <=
    Allocation = nullptr;
  }
  else { .... }
}

An object of the FOwnedBulkDataPtrtype is created using the new operator and released using the Free function. This last function calls std::free. This can lead to undefined behavior. The triggering appeared after we annotated the FMemory::Free function.


C_"static void Free(void* Original)"
  ADD(HAVE_STATE_DONT_MODIFY_VARS | RET_SKIP,
      nullptr,
      "FMemory",
      "Free",
      POINTER_TO_FREE);

New warning N2


V530 The return value of function 'CalcCacheValueSize' is required to be utilized. MemoryDerivedDataBackend.cpp 135


void FMemoryDerivedDataBackend::PutCachedData(
  const TCHAR* CacheKey,
  TArrayView<const uint8> InData,
  bool bPutEvenIfExists)
{
  ....
  FString Key(CacheKey);
  ....
  FCacheValue* Val = new FCacheValue(InData);
  int32 CacheValueSize = CalcCacheValueSize(Key, *Val);

  // check if we haven't exceeded the MaxCacheSize
  if (   MaxCacheSize > 0
      && (CurrentCacheSize + CacheValueSize) > MaxCacheSize)
  {
    ....
  }
  else
  {
    COOK_STAT(Timer.AddHit(InData.Num()));
    CacheItems.Add(Key, Val);
    CalcCacheValueSize(Key, *Val);                            // <=

    CurrentCacheSize += CacheValueSize;
  }
}

The return value of the CalcCacheValueSize method was not used. According to the analyzer, calling this method with no return value is senseless. Analyzer has information about the signatures of the CalcCacheValueSize method and its implementation, that's why it realized that the function has no state. Neither arguments, nor class properties, nor any other variables change. This became clear since annotated methods were used inside the CalcCacheValueSize function. A senseless function call may indicate a possible error in the program logic.


New warning N3


V630 The 'Malloc' function is used to allocate memory for an array of objects which are classes containing constructors. UnrealNames.cpp 639


class alignas(PLATFORM_CACHE_LINE_SIZE) FNamePoolShardBase : FNoncopyable
{
public:
  void Initialize(FNameEntryAllocator& InEntries)
  {
    LLM_SCOPE(ELLMTag::FName);
    Entries = &InEntries;

    Slots = (FNameSlot*)FMemory::Malloc(
      FNamePoolInitialSlotsPerShard * sizeof(FNameSlot), alignof(FNameSlot));
    memset(Slots, 0, FNamePoolInitialSlotsPerShard * sizeof(FNameSlot));
    CapacityMask = FNamePoolInitialSlotsPerShard - 1;
  }
....
}

The FNameSlot type objects are created without existing constructor call. The annotation of the Malloc function gives a hint. The annotation states that the Malloc function only allocates memory, and the size of the allocated memory block is specified in the first argument. This code fragment is suspicious and may lead to errors.


Thus, the Unreal Engine method annotations allows you to detect new errors. And now let's look at the check results of the CARLA simulator.


Check results


0888_Carla/image12.png


Warning N1


V522 Dereferencing of the null pointer 'CarlaActor' might take place. CarlaServer.cpp 1652


void FCarlaServer::FPimpl::BindActions()
{
  ....
  FCarlaActor* CarlaActor = Episode->FindCarlaActor(ActorId);
  if (CarlaActor)
  {
    return RespondError("get_light_boxes",
                        ECarlaServerResponse::ActorNotFound,
                        " Actor Id: " + FString::FromInt(ActorId));
  }
  if (CarlaActor->IsDormant())
  {
    return RespondError("get_light_boxes",
                        ECarlaServerResponse::FunctionNotAvailiableWhenDormant,
                        " Actor Id: " + FString::FromInt(ActorId));
  }
  else { .... }
  ....
}

One lost exclamation mark — and the function completely changes its behavior. Now, if CarlaActor is valid, an error is thrown. And if it is nullptr, the function leads to undefined behavior, which may be an abnormal program termination.


Warning N2


The analyzer issued a similar warning in another function.


V522 Dereferencing of the null pointer 'HISMCompPtr' might take place. ProceduralBuilding.cpp 32


UHierarchicalInstancedStaticMeshComponent* AProceduralBuilding::GetHISMComp(
    const UStaticMesh* SM)
{
  ....
  UHierarchicalInstancedStaticMeshComponent** HISMCompPtr =
    HISMComps.Find(SMName);

  if (HISMCompPtr) return *HISMCompPtr;

  UHierarchicalInstancedStaticMeshComponent* HISMComp = *HISMCompPtr;

  // If it doesn't exist, create the component
  HISMComp = NewObject<UHierarchicalInstancedStaticMeshComponent>(this,
    FName(*FString::Printf(TEXT("HISMComp_%d"), HISMComps.Num())));
  HISMComp->SetupAttachment(RootComponent);
  HISMComp->RegisterComponent();
  ....
}

When the search for SMName in HISMComps is a success, the GetHISMComp method returns the found element. Otherwise, the HISMCompPtr contains null pointer and dereference occurs. This causes undefined behavior. Most likely, initialization in the HISMComp definition was unnecessary. Immediately after, HISMComp receives new value.


Warning N3


V547 Expression 'm_trail == 0' is always false. unpack.hpp 699


std::size_t m_trail; 
....
inline int context::execute(const char* data, std::size_t len,
 std::size_t& off)
{
  ....
  case MSGPACK_CS_EXT_8: {
                uint8_t tmp;
                load<uint8_t>(tmp, n);
                m_trail = tmp + 1;
                if(m_trail == 0) {
                    unpack_ext(m_user, n, m_trail, obj);
                    int ret = push_proc(obj, off);
                    if (ret != 0) return ret;
                }
                else {
                    m_cs = MSGPACK_ACS_EXT_VALUE;
                    fixed_trail_again = true;
                }
            } break;
  ....
}

The tmp variable has the uint8_t type, which means its value ranges from 0 to 255. The m_trail variable is in the range from 1 to 256 because of integer promotion of the tmp variable. Since the m_trail in the condition cannot equal 0, instructions in the condition body are never executed. Such code can be redundant or not corresponding to the author's intents. It needs checking.


The analyzer found several more similar code fragments:


  • V547 Expression 'm_trail == 0' is always false. unpack.hpp 741
  • V547 Expression 'm_trail == 0' is always false. unpack.hpp 785
  • V547 Expression 'm_trail == 0' is always false. parse.hpp 472
  • V547 Expression 'm_trail == 0' is always false. parse.hpp 514
  • V547 Expression 'm_trail == 0' is always false. parse.hpp 558

Warning N4


A very similar situation occurred in another function.


V547 Expression '(uint8) WheelLocation >= 0' is always true. Unsigned type value is always >= 0. CARLAWheeledVehicle.cpp 510


float ACarlaWheeledVehicle::GetWheelSteerAngle(
  EVehicleWheelLocation WheelLocation) {

  check((uint8)WheelLocation >= 0)
  check((uint8)WheelLocation < 4)
  ....
}

Some check function takes the bool type value as its argument. The function throws an exception if the false value is passed. In the first check, the expression always has the true value, since the uint8 type has a range from 0 to 255. Probably, there is a typo in the check contents. The exact same check is in 524 line.


Warning N5


V547 Expression 'rounds > 1' is always true. CarlaExporter.cpp 137


void FCarlaExporterModule::PluginButtonClicked()
{
  ....
  int rounds;
  rounds = 5;
  ....
  for (int round = 0; round < rounds; ++round)
  {
    for (UObject* SelectedObject : BP_Actors)
    {
      ....
      // check to export in this round or not
      if (rounds > 1)                                          // <=
      {
        if (areaType == AreaType::BLOCK && round != 0)
          continue;
        else if (areaType == AreaType::ROAD && round != 1)
          continue;
        else if (areaType == AreaType::GRASS && round != 2)
          continue;
        else if (areaType == AreaType::SIDEWALK && round != 3)
          continue;
        else if (areaType == AreaType::CROSSWALK && round != 4)
          continue;
      }
      ....
    }
  }
}

It's clearly a typo. Instead of round a developer wrote rounds. It's easy to make a mistake in one letter, especially at the end of the tough workday. We are all human and we get tired. But a static code analyzer is a program, and it always works with the same vigilance. So, it's good to have such a tool at hand. Let me dilute the continuous code with a picture with simulator graphics.


0888_Carla/image14.png


Warning N6


V612 An unconditional 'return' within a loop. EndPoint.h 84


static inline auto make_address(const std::string &address) {
  ....
  boost::asio::ip::tcp::resolver::iterator iter = resolver.resolve(query);
  boost::asio::ip::tcp::resolver::iterator end;
  while (iter != end)
  {
    boost::asio::ip::tcp::endpoint endpoint = *iter++;
    return endpoint.address();
  }
  return boost::asio::ip::make_address(address);
}

The while loop, the condition, the iterator increment — all that shows that the instructions in the block must be executed more than once. However, due to return, only one iteration is performed. Surely there must be another logic here, otherwise the loop can be eliminated.


Warning N7


V794 The assignment operator should be protected from the case of 'this == &other'. cpp11_zone.hpp 92


struct finalizer_array
{
  void call() {
    finalizer* fin = m_tail;
    for(; fin != m_array; --fin) (*(fin-1))();
  }
  ~finalizer_array() {
     call();
     ::free(m_array);
  }
  finalizer_array& operator=(finalizer_array&& other) noexcept
  {
    this->~finalizer_array();                                // <=
    new (this) finalizer_array(std::move(other));
    return *this;
  }
  finalizer_array(finalizer_array&& other) noexcept
    : m_tail(other.m_tail), m_end(other.m_end), m_array(other.m_array)
  {
    other.m_tail = MSGPACK_NULLPTR;
    other.m_end = MSGPACK_NULLPTR;
    other.m_array = MSGPACK_NULLPTR;
  }
  ....
  finalizer* m_tail;
  finalizer* m_end;
  finalizer* m_array;
}

The analyzer detected an overloaded assignment operator, where this == &other lacks a check. Calling a destructor via this pointer results in the loss of other data. Subsequently, the assignment operator returns a copy of the cleaned object. The analyzer issued several more warnings that could be potential errors:


  • V794 The assignment operator should be protected from the case of 'this == &other'. cpp11_zone.hpp 154
  • V794 The assignment operator should be protected from the case of 'this == &other'. unpack.hpp 1093
  • V794 The assignment operator should be protected from the case of 'this == &other'. create_object_visitor.hpp 44
  • V794 The assignment operator should be protected from the case of 'this == &other'. parse.hpp 821
  • V794 The assignment operator should be protected from the case of 'this == &other'. sbuffer.hpp 55

Warning N8


V1030 The 'signals' variable is used after it was moved. MapBuilder.cpp 926


void MapBuilder::CreateController(....,
  const std::set<road::SignId>&& signals) 
{
   ....
    // Add the signals owned by the controller
    controller_pair.first->second->_signals = std::move(signals);

    // Add ContId to the signal owned by this Controller
    auto& signals_map = _map_data._signals;
    for(auto signal: signals) {                         // <=
      auto it = signals_map.find(signal);
      if(it != signals_map.end()) {
        it->second->_controllers.insert(signal);
      }
    }
}

The signals container will become empty after moving, and the range-based for loop will not execute. One of the right approaches would be to use controller_pair.first->second->_signals:


for (auto signal: controller_pair.first->second->_signals)

However, it would be correct, except for one thing. The signals container has a const specifier, which means it cannot be moved. Instead, it is copied, and therefore the program logically works correctly. A developer who wanted to optimize the code was able to confuse both himself and the analyzer. Kudos to him for this code. For the V1030 diagnostic fine-tuning, we will take this situation into account. Maybe we will write a new diagnostic.


Warning N9


V1061 Extending the 'std' namespace may result in undefined behavior. Waypoint.cpp 11


Let's look at two code snippets from the Waypoint.h and Waypoint.cpp files:


// Waypoint.h
namespace std {

  template <>
  struct hash<carla::road::element::Waypoint> {

    using argument_type = carla::road::element::Waypoint;

    using result_type = uint64_t;

    result_type operator()(const argument_type& waypoint) const;

  };

} // namespace std

// Waypoint.cpp
namespace std {

  using WaypointHash = hash<carla::road::element::Waypoint>;  // <=

  WaypointHash::result_type WaypointHash::operator()(
    const argument_type &waypoint) const
  {
    WaypointHash::result_type seed = 0u;
    boost::hash_combine(seed, waypoint.road_id);
    boost::hash_combine(seed, waypoint.section_id);
    boost::hash_combine(seed, waypoint.lane_id);
    boost::hash_combine(seed,
                        static_cast<float>(std::floor(waypoint.s * 200.0)));
    return seed;
  }

} // namespace std

In the header file, the developer extends the std namespace by declaring the explicit template specialization of the hash class in order to work with the carla::road::element::Waypoint type. In the file Waypoint.cpp, the developer adds the WaypointHash alias and the definition of the operator() function to the std namespace.


The C++ standard forbids extending the std namespace. The contents of the 'std' namespace are defined solely by the C++ Standards Committee and changed depending on the C++ language version. Modifying namespace's content may result in undefined behavior. However, adding an explicit or partial template specialization, as in the Waypoint.h file, is an exception. The V1061 diagnostic says that the definition of the operator() function in the Waypoint.cpp file is permitted, but the alias declaration in the std namespace is prohibited.


Actually, it is not necessary to extend the std namespace this way. It is enough to add the std::hash template specialization for a user type outside of std (yes, it is possible):


// Waypoint.h
// Not inside namespace "std"
template <>
struct std::hash<carla::road::element::Waypoint> {....};

// Waypoint.cpp
// Not inside namespace "std"
using WaypointHash = std::hash<CARLA::road::element::Waypoint>;

WaypointHash::result_type WaypointHash::operator()(
  const WaypointHash::argument_type& waypoint) const {....}

Warning N10


0888_Carla/image15.png


I left one interesting error for last. I encourage you to find it yourself. Unlike the others, this error is from the engine of Unreal Engine 4 game itself.


 virtual void visit(ir_variable *var)
  {
    ....
    const bool bBuiltinVariable = (var->name && 
                                   strncmp(var->name, "gl_", 3) == 0);

    if (bBuiltinVariable && ShaderTarget == vertex_shader && 
                            strncmp(var->name, "gl_InstanceID", 13) == 0)
    {
      bUsesInstanceID = true;
    }

    if (bBuiltinVariable &&
      var->centroid == 0 && (var->interpolation == 0 || 
                             strncmp(var->name, "gl_Layer", 3) == 0) &&
      var->invariant == 0 && var->origin_upper_left == 0 &&
      var->pixel_center_integer == 0)
    {
      // Don't emit builtin GL variable declarations.
      needs_semicolon = false;
    }
    else if (scope_depth == 0 && var->mode == ir_var_temporary)
    {
      global_instructions.push_tail(new(mem_ctx) global_ir(var));
      needs_semicolon = false;
    }
    else {....}
    ....
}

Here are two hints for you:


  1. the warning is issued with help of the method annotation.
  2. the warning is issued by the V666 diagnostic.

0888_Carla/image16.png


V666 Consider inspecting third argument of the function 'strncmp'. It is possible that the value does not correspond with the length of a string which was passed with the second argument. GlslBackend.cpp 943


Error in the strncmp function call:


strncmp(var->name, "gl_Layer", 3)

As the third argument of the function the number of characters to compare is passed, and as the second one — a string literal. The analyzer database has an annotation of the standard strncmp function, which says that the number of characters should probably match the the string literal length. In addition, for earlier calls of the strncmp function, the number of characters did coincide with the length of the string literal. However, in the code snippet above, the function compares only part of the string. The check of


strncmp(var->name, "gl_Layer", 3) == 0

is senseless, since bBuiltinVariable already contains the result of the same check:


strncmp(var->name, "gl_", 3) == 0

Most likely, the function call should've looked like this:


strncmp(var->name, "gl_Layer", 8)

Conclusion


The CARLA simulator is not only an entertaining and useful Unreal Engine 4 project, but it's also a high-quality product. The use of static analysis decreases the time spent on application development and debugging, and function annotations help perform more accurate analysis. We thank authors of this wonderful project for the opportunity to study the source code.


You can read more about static analysis in video game development and view the top 10 software bugs here.


0888_Carla/image17.png


Like other C++ software tools, static code analyzers never stay still for long and are continuously evolving. You may find interesting our latest article on C++ tools evolution. Check it out!

Tags:
Hubs:
+1
Comments 0
Comments Leave a comment

Articles

Information

Website
pvs-studio.com
Registered
Founded
2008
Employees
31–50 employees