Мы используем куки, чтобы пользоваться сайтом
было удобно.
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
Одним из механизмов статического анализа является аннотирование методов популярных библиотек. Аннотации позволяют обладать большей информацией при диагностировании ошибок в коде. Впечатляющий свободный проект на С++ CARLA помог нам внедрить этот механизм. Впоследствии симулятор стал целью для проверки улучшенным статанализатором PVS-Studio.
CARLA – это симулятор с открытым исходным кодом для исследования автономного вождения. Проект был разработан с нуля для поддержки разработки, обучения и проверки автономных систем вождения. Помимо открытого исходного кода и протоколов, CARLA предоставляет открытые цифровые активы (городские планировки, здания, транспортные средства), которые были созданы для этой цели и могут использоваться свободно. Платформа моделирования поддерживает гибкую спецификацию комплектов датчиков и условий окружающей среды.
Проект является кроссплатформенным и содержит почти 78 000 строк кода на С++. В репозитории проекта также был найден код, написанный на Python, XML, YAML, DOS Batch, CMake и других языках.
Статический анализ кода – это процесс выявления ошибок и недочётов в исходном коде программ. Статический анализ можно рассматривать как автоматизированный процесс обзора кода. Одной из технологий, используемой в статическом анализе, является аннотирование функций популярных библиотек. Разработчик изучает документации таких функций и отмечает полезные для анализа факты. Непосредственно во время проверки программы анализатор подтягивает эти факты из аннотаций, что позволяет проводить анализ с более высокой точностью.
Результатом проверки проектов является отчёт с предупреждениями. В PVS-Studio он может быть открыт в обычном редакторе, в утилите анализатора или в инструментах для разработки ПО, например в Visual Studio или CLion, с помощью соответствующих плагинов. Далее в статье вы найдёте топ-10 ошибок в проекте CARLA, а также сможете сами испытать свои силы в их поиске.
Для сборки в Unreal Engine используется собственная сборочная система – Unreal Build Tool. Поэтому анализ проектов, написанных на движке Unreal Engine, выполняется особенным образом. Есть два варианта проверки UE-проектов:
CARLA использует модифицированное ядро Unreal Engine 4, которое также есть на GitHub. Однако доступ к репозиторию приватный, как и к самому Unreal Engine 4. Сборка симулятора на Windows состоит из двух этапов: сборка движка и сборка самого проекта. Мы рассмотрим, как проанализировать и то, и другое.
Сборку Unreal Engine 4 можно выполнить в 8 шагов.
Для проверки движка интегрируем статический анализ в сборочную систему Unreal Build Tool. Чтобы выполнить анализ и ознакомиться с результатами проверки, потребуется выполнить следующие действия.
В результате вместо сборки или пересборки проекта будет выполняться анализ исходного кода средствами PVS-Studio. Теперь приступим к сборке самого симулятора CARLA.
Проект не генерирует solution, что не позволит нам интегрироваться в Unreal Build Tool. Поэтому мы воспользуемся вариантом проверки с отслеживанием вызовов компилятора. Существует два способа это сделать:
Обе утилиты уже есть в папке C:\Program Files (x86)\PVS-Studio после установки PVS-Studio. Мы воспользуемся вторым вариантом. Для сборки необходимо выполнить следующие действия.
Для удобного просмотра предупреждений анализатора можно открыть папку с репозиторием CARLA с помощью Visual Studio и загрузить отчёт. Затем будет полезно отфильтровать предупреждения на файлах ядра, автогенерированных файлах и подключаемых библиотеках. Для этого нужно выполнить ещё несколько действий:
Теперь можно изучать предупреждения анализатора в Visual Studio. Предупреждения будут только на коде симулятора CARLA и их собственных библиотеках.
Разборы ошибок, найденных в исходниках CARLA, отложим ненадолго. Дело в том, что анализ этого проекта нам был нужен ещё для одной задачи. Перед проверкой симулятора мы немного модифицировали ядро PVS-Studio так, чтобы оно собирало статистику вызовов методов Unreal Engine 4. Эти данные теперь помогут нам в аннотировании.
Аннотирование выполняется в два этапа:
При очередной проверке проекта информация о встреченных в коде проаннотированных методах будет получена не только из сигнатур функций, но и из аннотаций.
Например, аннотация может подсказать, что:
Какая аннотация была бы самой полезной? Хороший вопрос. Можем попробовать это выяснить в комментариях под статьей.
Аннотации не только помогают выявить новые ошибки, но и позволяют исключать некоторые ложные срабатывания.
Для чего же тут понадобился симулятор CARLA? Дело в том, что взять и проаннотировать все функции Unreal Engine 4 задача очень масштабная, требующая уйму времени. Когда-нибудь, быть может, мы её осилим, но сейчас мы решили начать с малого и посмотреть, что получится. Чтобы не брать первые попавшиеся 200 функций движка, было решено выявить наиболее популярные. Для этого мы нашли пару крупных проектов. Это довольно устаревшая игра Unreal Tournament и поддерживаемый на данный момент симулятор CARLA. Симулятор на С++ подошел нам по следующим причинам:
Итак, проекты выбраны. Они успешно собираются и анализируются. Что дальше? А дальше нам нужно собрать статистику по вызовам функций игрового движка. Как это сделать – вопрос интересный. К нашему счастью, под рукой – исходный код анализатора, который строит дерево разбора и позволяет выявлять вызовы функций со всей необходимой информацией. Поэтому достаточно было написать что-то похожее на новую диагностику. Функция нам подходила, если выполнялись два условия:
Если оба условия выполнялись, то информация о вызове записывалась в отдельный файл. Оставалось только запустить анализ с модифицированным ядром. После анализа мы получили лог функций. С помощью нескольких простых формул в Exсel, статистика приобрела следующий вид:
Мы посчитали, что для начала достаточно проаннотировать все функции, встретившиеся больше 10 раз. Их оказалось около 200. Так как программисты не очень любят документировать код, для аннотирования пришлось изучать реализацию каждой функции Unreal Engine 4 в исходном коде. В качестве примера приведу аннотацию функции ConstructUFunction:
C_"void ConstructUFunction(UFunction*& OutFunction, \
const FFunctionParams& Params)"
ADD(HAVE_STATE | RET_SKIP | F_ARG_ALLOC,
"UE4CodeGen_Private",
nullptr,
"ConstructUFunction",
ALLOC_ARG, SKIP);Флаг F_ARG_ALLOC означает, что функция выделяет ресурс и отдает его через один из своих параметров. Флаг ALLOC_ARG указывает, что через первый параметр функции, а именно OutFunction, возвращается указатель на выделенный ресурс. Флаг SKIP говорит, что второй аргумент функции для нас не является особенным и неинтересен.
После того как все аннотации были написаны, мы ещё раз проверили симулятор CARLA и версию движка, которую он использует. Как и ожидалось, часть ложных срабатываний исчезла и появилось несколько новых предупреждений.
Новое предупреждение 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 { .... }
}Объект типа FOwnedBulkDataPtr создаётся с помощью оператора new и удаляется с помощью функции Free, которая вызывает std::free. Это может привести к неопределённому поведению. Срабатывание появилось после аннотирования функции FMemory::Free.
C_"static void Free(void* Original)"
ADD(HAVE_STATE_DONT_MODIFY_VARS | RET_SKIP,
nullptr,
"FMemory",
"Free",
POINTER_TO_FREE);Новое предупреждение 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;
}
}Возвращаемое значение метода CalcCacheValueSize не было использовано. Анализатор посчитал вызов этого метода без использования возвращаемого значения бесполезным. Просмотрев сигнатуры метода CalcCacheValueSize и заглянув в реализацию, анализатор понял, что функция не имеет состояния. Не меняются ни аргументы, ни свойства класса, ни какие-либо другие переменные. Это стало понятно благодаря тому, что внутри функции CalcCacheValueSize использовались проаннотированные методы. Бессмысленный вызов функции свидетельствует о возможной ошибке в логике программы.
Новое предупреждение 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;
}
....
}Объекты типа FNameSlot создаются в обход существующего конструктора. Это подсказывает аннотация функции Malloc. В аннотации указано, что функция Malloc только выделяет память, при этом размер блока выделяемой памяти указан в первом аргументе. Данный код подозрителен и может привести к ошибкам.
Таким образом, аннотирование методов Unreal Engine позволяет выявлять новые ошибки. А теперь давайте приступим к изучению результатов проверки симулятора CARLA.
Предупреждение 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 { .... }
....
}Один потерянный восклицательный знак – и функция полностью меняет поведение. Теперь в случае валидности CarlaActor выбрасывается ошибка, а в случае nullptr функция приводит к неопределённому поведению, которым может быть аварийное завершение программы.
Предупреждение N2
Похожее предупреждение анализатор выдал в другой функции.
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();
....
}Когда поиск SMName в HISMComps завершается успехом метод GetHISMComp возвращает найденный элемент. В противном случае преждевременный выход из метода не осуществляется, но происходит разыменовывание нулевого указателя HISMCompPtr, что становится причиной неопределённого поведения. Скорее всего, инициализация в определении HISMComp была лишней. Сразу следом HISMComp принимает новое значение.
Предупреждение 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;
....
}Переменная tmp имеет тип uint8_t, а значит диапазон её значений от 0 до 255. Переменная m_trail принимает значение на 1 больше, поэтому её диапазон возможных значений от 1 до 256. Так как m_trail в условии не может быть равной 0, инструкции в теле условия никогда не будут выполнены. Такой код может быть как избыточным, так и не соответствующим задумкам автора. Следует его проверить.
Анализатор нашёл ещё несколько похожих фрагментов кода:
Предупреждение N4
Очень похожая ситуация встретилась и в другой функции.
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)
....
}Некая функция проверки check принимает своим аргументом значение типа bool и вызывает исключение, если было передано значение false. В первой проверке выражение всегда будет иметь значение true, так как тип uint8 имеет диапазон от 0 до 255. Возможно, в содержимом check допущена опечатка. Точно такая же проверка есть в строке 524.
Предупреждение 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;
}
....
}
}
}А вот тут явная опечатка. Вместо round используется rounds. Допустить ошибку в одной букве легко, особенно в конце тяжёлого рабочего дня. Все мы люди и устаём. А вот статический анализатор кода – это программа, и она всегда работает с одинаковой бдительностью. Поэтому хорошо иметь такой инструмент под рукой. Разбавлю код картинкой с графикой симулятора.
Предупреждение 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);
}Цикл while, условие, инкрементирование итератора – всё это говорит, что инструкции в блоке должны выполняться больше одного раза. Однако из-за return будет выполнена лишь одна итерация. Наверняка, тут должна быть другая логика, иначе цикл можно устранить.
Предупреждение 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;
}Анализатор обнаружил перегрузку оператора присваивания, в котором не осуществляется проверка this == &other. Вызов деструктора через указатель this приводит к потере данных other. Впоследствии оператор присваивания возвращает копию очищенного объекта. Анализатор выявил ещё несколько таких потенциальных ошибок:
Предупреждение 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);
}
}
}Контейнер signals после перемещения станет пустым, и цикл range-based for не отработает. Правильным было бы использовать controller_pair.first->second->_signals:
for (auto signal: controller_pair.first->second->_signals)Однако так было бы правильно, если бы не одно "но". Контейнер signals имеет спецификатор const, а значит, не может быть перемещен. Вместо этого он будет скопирован, и поэтому программа логически работает правильно. Программист, который хотел оптимизировать код, смог запутать и себя, и анализатор. Спасибо ему за этот код, теперь мы учтём подобную ситуацию при доработке диагностики V1030, а может, даже напишем новую.
Предупреждение N9
V1061 Extending the 'std' namespace may result in undefined behavior. Waypoint.cpp 11
Рассмотрим два фрагмента кода из файлов Waypoint.h и Waypoint.cpp:
// 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В заголовочном файле пространство имен std расширяется явной специализацией шаблона класса hash для работы с типом carla::road::element::Waypoint. В файле Waypoint.cpp в std добавляют псевдоним структуры WaypointHash и реализацию оператора вызова функции operator().
Вообще стандарт С++ запрещает расширять пространство имен std, так как его содержимое определяется исключительно комитетом стандартизации и меняется от версии языка С++. Модификация данных в этом пространстве имён может привести к неопределенному поведению. Однако добавление явной или частичной специализации шаблона, как в файле Waypoint.h, является исключением. Реализация оператора вызова функции в файле Waypoint.cpp тоже допустима, а вот декларации псевдонима в пространстве имен std не должно быть, о чём сообщает диагностика V1061.
На самом деле, вовсе не обязательно так расширять пространство имён std. Достаточно добавить специализацию шаблона std::hash для пользовательского типа за пределами std (да, так можно):
// 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 {....}Предупреждение N10
Одну интересную ошибку я оставил напоследок. Предлагаю найти её самостоятельно. Она, в отличие от остальных, из самого игрового движка Unreal Engine 4.
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 {....}
....
}Даю две подсказки:
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
Ошибка в вызове функции strncmp:
strncmp(var->name, "gl_Layer", 3)Третьим аргументом функции передается число сравниваемых символов, а во втором – строковый литерал. В базе данных анализатора имеется аннотация стандартной функции strncmp, которая подсказывает, что число символов, вероятно, должно совпадать с длиной строкового литерала. К тому же для предыдущих вызовов функции strncmp число символов действительно совпадало с длиной строкового литерала. Однако в приведённом фрагменте кода функция сравнивает только часть строки. Проверка
strncmp(var->name, "gl_Layer", 3) == 0бессмысленна, так как bBuiltinVariable уже содержит результат такой же проверки:
strncmp(var->name, "gl_", 3) == 0Скорее всего, вызов функции должен был выглядеть так:
strncmp(var->name, "gl_Layer", 8)Симулятор CARLA не только интересный и полезный проект на Unreal Engine 4, но и довольно качественный. Использование статического анализа ведёт к уменьшению времени на разработку и отладку приложений, а аннотирование функций помогает выполнять более точный анализ. Спасибо авторам этого чудесного проекта за возможность исследования кода на предмет частоты использования функций UE4.
Ещё больше про статический анализ в видеоигровой индустрии и топ-10 программных ошибок можно почитать здесь.
Как и другие инструменты С++ программистов, статические анализаторы кода не стоят на одном месте и постоянно развиваются. Про это и многое другое можно почитать в свежей статье.
0
0
0
0