Симулятор — это приложение, способное имитировать запуск программы, предназначенной для одной платформы, на другой.
Примером эмулятора является GPCS4, предназначенный для запуска игр PS4 на ПК.
Недавно состоялся первый релиз GPCS4, и мы решили проверить этот проект. Давайте посмотрим, какие ошибки PVS-Studio удалось найти в исходном коде этого эмулятора.
о проекте
GPCS4 — это эмулятор PlayStation 4, написанный на C и C++.Изначально целью автора проекта было исследование архитектуры PS4. Однако проект быстро развивался, и уже в начале 2020 года разработчикам GPCS4 удалось запустить игру на своем эмуляторе.
Мы обречены .
Это был первый успешный стабильный запуск игры для PS4 на ПК.
Однако сам геймплей еще был далек от идеала: игра тормозила, картинка дергалась.
Тем не менее, главный разработчик проекта полон энтузиазма и продолжает совершенствовать эмулятор.
В конце апреля 2022 года первый выпуск GPCS4 .
Я скачал и протестировал версию проекта 0.1.0. Кстати, на момент публикации статьи уже вышел релиз 0.2.0 — проект очень быстро развивается.
Перейдем к ошибкам и недостаткам, которые анализатору PVS-Studio удалось найти в первом релизе проекта GPCS4.
Потерянный перерыв
В796 [CWE-484] Возможно, в операторе переключения отсутствует оператор «break».AudioOut.cpp 137
В этом фрагменте кода для случая SCE_AUDIO_OUT_PARAM_FORMAT_FLOAT_MONO потерянный перерыв .static AudioProperties getAudioProperties(uint32_t param) { uint32_t format = param & 0x000000ff; AudioProperties props = {}; switch (format) { // .case SCE_AUDIO_OUT_PARAM_FORMAT_S16_8CH_STD: { props.nChannels = 8; props.bytesPerSample = 2; props.audioFormat = RTAUDIO_FORMAT_SINT16; break; } case SCE_AUDIO_OUT_PARAM_FORMAT_FLOAT_MONO: { props.nChannels = 1; props.bytesPerSample = 4; props.audioFormat = RTAUDIO_FORMAT_FLOAT32; // <= } case SCE_AUDIO_OUT_PARAM_FORMAT_FLOAT_STEREO: { props.nChannels = 2; props.bytesPerSample = 4; props.audioFormat = RTAUDIO_FORMAT_FLOAT32; break; } } return props; }
Из-за этого количество каналов будет установлено неправильно.
Проверка указателя после использования
В595 Указатель m_moduleData использовался до его проверки на соответствие nullptr. Проверьте строки: 49, 53. ELFMapper.cpp 49 struct NativeModule { /*.
*/ };
class ELFMapper
{
// .
NativeModule *m_moduleData;
};
bool ELFMapper::validateHeader()
{
bool retVal = false;
auto &fileMemory = m_moduleData->m_fileMemory;
do
{
if (m_moduleData == nullptr)
{
LOG_ERR("file has not been loaded");
break;
}
// .
} while (false);
return retVal;
}
В приведенном выше фрагменте индекс m_moduleData сначала разыменовывается, а затем в цикле делать пока по сравнению с нульптр .
Внимательные читатели могут возразить: «Может быть, на вход функции всегда приходит валидный указатель? И затем в цикле делать пока этот указатель изменен и может стать нулевым? Тогда ошибки нет».
В данном случае это не так.
Во-первых, из-за условия пока (ложь) Будет выполнена ровно одна итерация цикла.
Во-вторых, указатель m_moduleData не изменен .
Другое возражение может заключаться в том, что переход по ссылке безопасен.
Ведь эта ссылка будет использоваться только в том случае, если указатель действителен.
Но нет, вот так код содержит неопределенное поведение .
Это ошибка.
Скорее всего, вам нужно проверить указатель перед его первым разыменованием: bool ELFMapper::validateHeader()
{
bool retVal = false;
do
{
if (m_moduleData == nullptr)
{
LOG_ERR("file has not been loaded");
break;
}
auto &fileMemory = m_moduleData->m_fileMemory;
// .
} while (false);
return retVal;
}
Переназначение
В519 [CWE-563] Переменной '*memoryType' присваиваются значения дважды подряд. Возможно, это ошибка.
Проверьте строки: 54, 55. sce_kernel_memory.cpp 55 int PS4API sceKernelGetDirectMemoryType(sce_off_t start, int *memoryType,
sce_off_t *regionStartOut, sce_off_t *regionEndOut)
{
LOG_SCE_DUMMY_IMPL();
*memoryType = SCE_KERNEL_WB_GARLIC;
*memoryType = SCE_KERNEL_WC_GARLIC;
return SCE_OK;
}
Как можно догадаться по названию LOG_SCE_DUMMY_IMPL , реализация метода sceKernelGetDirectMemoryType все равно изменится.
И еще, два назначения на один и тот же адрес тип памяти выглядеть странно.
Возможно, это произошло в результате неудачного слияния кода.
Переполнение буфера
В512 [CWE-119] Вызов функции memset приведет к переполнению буфера param-> reserved. sce_gnm_draw.cpp 420 В531 [CWE-131] Странно, что оператор sizeof() умножается на sizeof().
sce_gnm_draw.cpp 420 struct GnmCmdPSShader
{
uint32_t opcode;
gcn::PsStageRegisters psRegs;
uint32_t reserved[27];
};
int PS4API sceGnmSetPsShader350(uint32_t* cmdBuffer, uint32_t numDwords,
const gcn::PsStageRegisters *psRegs)
{
// .
memset(param->reserved, 0, sizeof(param->reserved) * sizeof(uint32_t));
return SCE_OK;
}
Иногда случается, что на одну и ту же строку кода указывают сразу несколько диагностик PVS-Studio. Вот что произошло в этом примере.
В этом фрагменте кода в функцию Мемсет Неверное значение передается в качестве третьего аргумента.
Выражение sizeof (параметр-> зарезервировано) уже вернет размер массива параметр-> зарезервировано .
Умножение на размер(uint32_t) увеличит это значение в 4 раза, и значение будет неправильным.
Таким образом, в результате вызова Мемсет массив выйдет за границу параметр-> зарезервировано .
Нам нужно убрать ненужное умножение: int PS4API sceGnmSetPsShader350( /*.
*/ ) { // .
memset(param->reserved, 0, sizeof(param->reserved));
return SCE_OK;
}
Всего анализатор обнаружил 20 таких переполнений; Приведу еще один пример: В512 [CWE-119] Вызов функции memset приведет к переполнению буфера initParam-> reserved. sce_gnm_dispatch.cpp 16 uint32_t PS4API sceGnmDispatchInitDefaultHardwareState(uint32_t* cmdBuffer,
uint32_t numDwords)
{
// .
memset(initParam->reserved, 0,
sizeof(initParam->reserved) * sizeof(uint32_t));
return initCmdSize;
}
Этот фрагмент кода выходит за границу массива initParam-> зарезервировано .
Учимся считать до семи или другое переполнение буфера
В557 [CWE-787] Возможно переполнение массива.Индекс «dynamicStateCount++» указывает за пределы массива.
VltGraphics.cpp 157 VkPipeline VltGraphicsPipeline::createPipeline(/* .
*/) const { // .
std::array<VkDynamicState, 6> dynamicStates; uint32_t dynamicStateCount = 0; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_VIEWPORT; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_SCISSOR; if (state.useDynamicDepthBias()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BIAS; if (state.useDynamicDepthBounds()) { dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BOUNDS; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BOUNDS_TEST_ENABLE; } if (state.useDynamicBlendConstants()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_BLEND_CONSTANTS; if (state.useDynamicStencilRef()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_STENCIL_REFERENCE; // .
}
Анализатор предупреждает, что может произойти переполнение массива динамические состояния .
В этом фрагменте кода есть 4 проверки:
- если (state.useDynamicDepthBias())
- если (state.useDynamicDepthBounds())
- если (state.useDynamicBlendConstants())
- если (state.useDynamicStencilRef())
Например, проверка если (state.useDynamicDepthBias()) : bool useDynamicDepthBias() const
{
return rs.depthBiasEnable();
}
VkBool32 depthBiasEnable() const
{
return VkBool32(m_depthBiasEnable);
}
Оказывается, все эти 4 проверки могут быть истинными одновременно.
Затем будут выполнены 7 строк формы 'dynamicStates[dynamicStateCount++] = .
' .
На седьмой такой строке будет вызов динамические состояния[6] .
Это переполнение массива.
Чтобы это исправить, нужно выделить память под 7 элементов: VkPipeline VltGraphicsPipeline::createPipeline(/* .
*/) const { // .
std::array<VkDynamicState, 7> dynamicStates; // <= uint32_t dynamicStateCount = 0; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_VIEWPORT; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_SCISSOR; if (state.useDynamicDepthBias()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BIAS; if (state.useDynamicDepthBounds()) { dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BOUNDS; dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_DEPTH_BOUNDS_TEST_ENABLE; } if (state.useDynamicBlendConstants()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_BLEND_CONSTANTS; if (state.useDynamicStencilRef()) dynamicStates[dynamicStateCount++] = VK_DYNAMIC_STATE_STENCIL_REFERENCE; // .
}
Неправильное использование флага
В547 [CWE-570] Выражение «nOldFlag & VMPF_NOACCESS» всегда является ложным.
PlatMemory.cpp 22 #define PAGE_NOACCESS 0x01
#define PAGE_READONLY 0x02
#define PAGE_READWRITE 0x04
#define PAGE_EXECUTE 0x10
#define PAGE_EXECUTE_READ 0x20
#define PAGE_EXECUTE_READWRITE 0x40
enum VM_PROTECT_FLAG
{
VMPF_NOACCESS = 0x00000000,
VMPF_CPU_READ = 0x00000001,
VMPF_CPU_WRITE = 0x00000002,
VMPF_CPU_EXEC = 0x00000004,
VMPF_CPU_RW = VMPF_CPU_READ | VMPF_CPU_WRITE,
VMPF_CPU_RWX = VMPF_CPU_READ | VMPF_CPU_WRITE | VMPF_CPU_EXEC,
};
inline uint32_t GetProtectFlag(VM_PROTECT_FLAG nOldFlag)
{
uint32_t nNewFlag = 0;
do
{
if (nOldFlag & VMPF_NOACCESS)
{
nNewFlag = PAGE_NOACCESS;
break;
}
if (nOldFlag & VMPF_CPU_READ)
{
nNewFlag = PAGE_READONLY;
}
if (nOldFlag & VMPF_CPU_WRITE)
{
nNewFlag = PAGE_READWRITE;
}
if (nOldFlag & VMPF_CPU_EXEC)
{
nNewFlag = PAGE_EXECUTE_READWRITE;
}
} while (false);
return nNewFlag;
}
Функция GetProtectFlag Преобразует флаг разрешений файла из одного формата в другой.
Однако он делает это неправильно.
Программист не учел, что значение VMPF_NOACCESS равен нулю.
Из-за этого состояния если (nOldFlag и VMPF_NOACCESS) всегда ложно, и функция никогда не вернет значение СТРАНИЦА_NOACCESS .
Кроме того, функция GetProtectFlag неправильно конвертирует не только флаг VMPF_NOACCESS , но и другие.
Например, флаг VMPF_CPU_EXEC будет преобразован во флаг PAGE_EXECUTE_READWRITE .
Когда я думал о том, как это исправить, моей первой мыслью было написать что-то вроде этого: inline uint32_t GetProtectFlag(VM_PROTECT_FLAG nOldFlag)
{
uint32_t nNewFlag = PAGE_NOACCESS;
if (nOldFlag & VMPF_CPU_READ)
{
nNewFlag |= PAGE_READ;
}
if (nOldFlag & VMPF_CPU_WRITE)
{
nNewFlag |= PAGE_WRITE;
}
if (nOldFlag & VMPF_CPU_EXEC)
{
nNewFlag |= PAGE_EXECUTE;
}
return nNewFlag;
}
Однако в данном случае такой подход не работает. Это факт СТРАНИЦА_NOACCESS , PAGE_READONLY а остальное это флаги винды со своей спецификой.
Например, среди них нет флага PAGE_WRITE .
Считается, что если у вас есть права на запись, то, по крайней мере, у вас есть и права на чтение.
По тем же причинам нет флага PAGE_EXECUTE_WRITE .
Кроме того, побитовое ИЛИ двух флагов Windows не приводит к получению флага, соответствующего сумме прав: PAGE_READONLY | PAGE_EXECUTE != PAGE_EXECUTE_READ .
Поэтому вам нужно перебрать все возможные комбинации флагов: inline uint32_t GetProtectFlag(VM_PROTECT_FLAG nOldFlag)
{
switch (nOldFlag)
{
case VMPF_NOACCESS:
return PAGE_NOACCESS;
case VMPF_CPU_READ:
return PAGE_READONLY;
case VMPF_CPU_WRITE: // same as ReadWrite
case VMPF_CPU_RW:
return PAGE_READWRITE;
case VMPF_CPU_EXEC:
return PAGE_EXECUTE;
case VMPF_CPU_READ | VMPF_CPU_EXEC:
return PAGE_EXECUTE_READ:
case VMPF_CPU_WRITE | VMPF_CPU_EXEC: // same as ExecuteReadWrite
case VMPF_CPU_RWX:
return PAGE_EXECUTE_READWRITE;
default:
LOG("unknown PS4 flag");
return PAGE_NOACCESS;
}
}
Дополнительная проверка
В547 [CWE-571] Выражение «retAddress» всегда истинно.Память.
cpp 373 void* MemoryAllocator::allocateInternal(void* addrIn, size_t len,
size_t alignment, int prot)
{
// .
while (searchAddr < SCE_KERNEL_APP_MAP_AREA_END_ADDR) { // .
void* retAddress = VMAllocate(reinterpret_cast<void*>(regionAddress), len, plat::VMAT_RESERVE_COMMIT, uprot); if (!retAddress) { searchAddr = reinterpret_cast<size_t>(mi.pRegionStart) + mi.nRegionSize; continue; } // .
if (retAddress) { // unlikely plat::VMFree(retAddress); } // .
} // .
}
В этом фрагменте кода указатель retAddress проверяется дважды.
Сначала проводится проверка если (!retAddress) .
Если указатель равен нулю, выполнение перейдет к следующей итерации цикла.
пока .
В противном случае указатель retAddress ненулевой.
Поэтому вторая проверка если (ретАдрес) всегда верно и может быть удалено.
Еще одно всегда верное условие
В547 [CWE-571] Выражение 'pipeConfig == kPipeConfigP16' всегда истинно.
GnmDepthRenderTarget.h 170 uint8_t getZReadTileSwizzleMask(void) const
{
// From IDA
auto pipeConfig = getPipeConfig();
auto zfmt = getZFormat();
auto tileMode = getTileMode();
if (pipeConfig != kPipeConfigP16 || // <=
zfmt == kZFormatInvalid ||
!GpuAddress::isMacroTiled(tileMode))
{
return 0;
}
auto dataFormat = DataFormat::build(zfmt);
auto totalBitsPerElement = dataFormat.getTotalBitsPerElement();
uint32_t numFragments = 1 << getNumFragments();
uint32_t shift = 0;
NumBanks numBanks = {};
if (pipeConfig == kPipeConfigP16) // <=
{
GpuAddress::getAltNumBanks(&numBanks, tileMode,
totalBitsPerElement, numFragments);
shift = 4;
}
else
{
GpuAddress::getNumBanks(&numBanks, tileMode,
totalBitsPerElement, numFragments);
shift = 3;
}
return (this->m_regs[2] & (((1 << (numBanks + 1)) - 1) << shift)) >> 4;
}
В этом фрагменте кода анализатор всегда находил истинное условие если (pipeConfig == kPipeConfigP16) .
Давайте разберемся, почему это так.
Если вызов функции getPipeConfig вернул значение, не равное kPipeConfigP16 , то первое условие будет истинным и выполнение программы не дойдет до проверки если (pipeConfig == kPipeConfigP16) .
Получается, что вторая проверка этой переменной либо не выполняется, либо всегда верна.
Но не спешите удалять его.
Возможно, первое условие было добавлено временно и в будущем будет удалено.
Ошибка копирования-вставки
В517 [CWE-570] Было обнаружено использование шаблона «if (A) {.} else if (A) {.
}».
Существует вероятность наличия логической ошибки.
Проверить строки: 469, 475. GnmGpuAddress.cpp 469 int32_t sce::GpuAddress::adjustTileMode(/* .
*/) { switch(microTileMode) { case Gnm::kMicroTileModeThin: if (newArrayMode == Gnm::kArrayMode3dTiledThick) *outTileMode = Gnm::kTileModeThick_3dThick; else if (newArrayMode == Gnm::kArrayMode2dTiledThick) *outTileMode = Gnm::kTileModeThick_2dThick; else if (newArrayMode == Gnm::kArrayMode1dTiledThick) *outTileMode = Gnm::kTileModeThick_1dThick; else if (newArrayMode == Gnm::kArrayMode3dTiledThin) *outTileMode = Gnm::kTileModeThin_3dThin; // .
else if (newArrayMode == Gnm::kArrayMode3dTiledThinPrt) *outTileMode = Gnm::kTileModeThin_3dThinPrt; // .
else if (newArrayMode == Gnm::kArrayMode2dTiledThin) // <= *outTileMode = Gnm::kTileModeThin_2dThin; // .
else if (newArrayMode == Gnm::kArrayMode2dTiledThinPrt) *outTileMode = Gnm::kTileModeThin_2dThinPrt; // .
else if (newArrayMode == Gnm::kArrayModeTiledThinPrt) *outTileMode = Gnm::kTileModeThin_ThinPrt; // .
else if (newArrayMode == Gnm::kArrayMode2dTiledThin) // <= *outTileMode = Gnm::kTileModeThin_2dThin; else if (newArrayMode == Gnm::kArrayMode1dTiledThin) *outTileMode = Gnm::kTileModeThin_1dThin; else break; return kStatusSuccess; // .
}
}
Были также некоторые ошибки копирования.
В этом фрагменте кода одна и та же проверка пишется дважды newArrayMode == Gnm::kArrayMode2dTiledThin .
Трудно сказать точно, как это исправить.
Скорее всего, вторая проверка должна быть немного другой.
А может это лишнее и его можно удалить.
Почему лучше избегать сложных выражений?
В732 [CWE-480] Унарный оператор минус не изменяет значение типа bool. Рассмотрите возможность использования '!' оператор.
GnmRenderTarget.h 237 typedef enum RenderTargetChannelType
{
kRenderTargetChannelTypeUNorm = 0x00000000,
kRenderTargetChannelTypeSNorm = 0x00000001,
kRenderTargetChannelTypeUInt = 0x00000004,
kRenderTargetChannelTypeSInt = 0x00000005,
kRenderTargetChannelTypeSrgb = 0x00000006,
kRenderTargetChannelTypeFloat = 0x00000007,
} RenderTargetChannelType;
void setDataFormat(DataFormat format)
{
// .
int v3; RenderTargetChannelType type; // [rsp+4h] [rbp-3Ch] __int64 v9; // [rsp+10h] [rbp-30h] bool typeConvertable = format.getRenderTargetChannelType(&type); v2 = type | kRenderTargetChannelTypeSNorm; v3 = (uint8_t) - (type < 7) & (uint8_t)(0x43u >> type) & 1; // <= // .
}
Похоже, что программист ожидал следующего поведения при вычислении выражения:
- пусть переменная тип меньше 7 ;
- тогда выражение тип < 7 равно истинный ;
- затем, чтобы истинный применяется унарный минус, получается -1 ;
- значение -1 приводит к беззнаковый символ , оказывается 0b1111'1111 .
- пусть переменная тип меньше 7 ;
- тогда выражение тип < 7 равно истинный ;
- затем, чтобы истинный применяется унарный минус, получается 1 ;
- значение 1 приводит к беззнаковый символ , оказывается 0b0000'0001 .
По этой счастливой случайности код работает полностью так, как ожидает программист. Однако стоит исправить этот код. Попробуем понять, какое значение присвоено переменной v3 в зависимости от стоимости тип .
Первый случай: переменная тип больше или равно 7.
- Тогда выражение тип < 7 равно ЛОЖЬ .
- К ЛОЖЬ применяется унарный минус, получается ЛОЖЬ .
- False приводится к беззнаковому символу, что приводит к 0b0000'0000 .
- Побитовое И с 0 всегда дает 0, поэтому результат равен 0.
- Как мы выяснили ранее, выражение (uint8_t) - (тип < 7) равно 1.
- В этом случае имеет смысл оценить выражение 0x43u > > введите .
- Для удобства запишем двоичное представление числа 0x43 = 0b0100'0011.
- Нас интересует только младший бит, поскольку результат выражения 0x43u > > введите будет применено побитовое «И», начиная с 1.
- Если тип равно 0, 1 или 6, то младший бит будет 1 и результат всего выражения будет 1. Во всех остальных случаях результат будет 0.
(тип == 0) || (тип == 1) || (тип == 6) .
Я предлагаю следующий код: typedef enum RenderTargetChannelType
{
kRenderTargetChannelTypeUNorm = 0x00000000,
kRenderTargetChannelTypeSNorm = 0x00000001,
kRenderTargetChannelTypeUInt = 0x00000004,
kRenderTargetChannelTypeSInt = 0x00000005,
kRenderTargetChannelTypeSrgb = 0x00000006,
kRenderTargetChannelTypeFloat = 0x00000007,
} RenderTargetChannelType;
void setDataFormat(DataFormat format)
{
// .
int v3; RenderTargetChannelType type; // [rsp+4h] [rbp-3Ch] __int64 v9; // [rsp+10h] [rbp-30h] bool typeConvertable = format.getRenderTargetChannelType(&type); v2 = type | kRenderTargetChannelTypeSNorm; v3 = (type == kRenderTargetChannelTypeUNorm) || (type == kRenderTargetChannelTypeSNorm) || (type == kRenderTargetChannelTypeSrgb); // .
}
Здесь я также заменил числа 0, 1 и 6 соответствующими именованными значениями перечисления и записал подвыражения в табличной форме.
Крайний регистр в операторе перемещения
В794 Оператор присваивания должен быть защищен от случая «this == &other».ВлтШадер.
cpp 39 VltShaderConstData& VltShaderConstData::operator=(VltShaderConstData&& other)
{
delete[] m_data;
this->m_size = other.m_size;
this->m_data = other.m_data;
other.m_size = 0;
other.m_data = nullptr;
return *this;
}
Если при звонке такому оператору окажется, что 'это == &другое' , то все поля текущего объекта будут очищены и данные будут потеряны.
Такое поведение неверно, нужно добавить галочку.
Исправленный код: VltShaderConstData& VltShaderConstData::operator=(VltShaderConstData&& other)
{
if (this == std::addressof(other))
{
return *this;
}
delete[] m_data;
this->m_size = other.m_size;
this->m_data = other.m_data;
other.m_size = 0;
other.m_data = nullptr;
return *this;
}
Переназначение как причина рефакторинга
В1048 [CWE-1164] Переменной 'retVal' было присвоено то же значение.Модуль.
cpp 129 bool NativeModule::getSymbolInfo( /* .
*/) const
{
bool retVal = false;
do
{
uint32_t modId = 0, libId = 0;
if (modName == nullptr || libName == nullptr || nid == nullptr)
{
break;
}
if (!isEncodedSymbol(encSymbol))
{
*modName = "";
*libName = "";
*nid = 0;
retVal = true;
break;
}
retVal = decodeSymbol(encSymbol, &modId, &libId, nid);
if (!retVal)
{
LOG_ERR("fail to decode encoded symbol");
break;
}
retVal = getModNameFromId(modId, mods, modName);
if (!retVal)
{
LOG_ERR("fail to get module name for symbol: %s in %s",
encSymbol.c_str(), fileName.c_str());
break;
}
retVal = getLibNameFromId(libId, libs, libName);
if (!retVal)
{
LOG_ERR("fail to get library name");
break;
}
retVal = true; // <=
} while (false);
return retVal;
}
Этот фрагмент кода включает переназначение значений.
истинный в переменную retVal .
Давайте разберемся, почему это происходит. Для этого рассмотрим все возможные модификации переменной retVal перед назначением, указанным анализатором.
- Переменная retVal инициализировано значением ЛОЖЬ .
- Если вызов функции isEncodedSymbol вернулся ЛОЖЬ , то переменная retVal присвоено значение истинный и цикл прерывается делать пока .
- Переменная retVal присваивается результат вызова функции декодироватьсимвол .
После этого если ретвал == ложь , то цикл делать пока прерывается.
- То же самое происходит с двумя вызовами функций getModNameFromId .
Если какой-либо из вызовов возвращается ЛОЖЬ , то цикл делать пока прерывается.
Поэтому переменная retVal уже равен истинный , и назначение не имеет смысла.
Зачем вообще использовать конструкцию '? делать.
пока (ложь) '? Дело в том, что такая конструкция позволяет сделать досрочный выход из функции с помощью одного возвращаться .
Для функций с одним возвращаться , в свою очередь, с большей вероятностью будет применяться НРВО – названная оптимизация возвращаемого значения.
Такая оптимизация компилятора позволяет избежать ненужного копирования или перемещения возвращаемого объекта, создавая его сразу в том месте, где вызывается функция.
В этом случае функция возвращает облегченный тип логическое значение , поэтому выигрыш от NRVO будет незначительным.
Кроме того, современные компиляторы могут применять NRVO к функциям.
С несколькими возвращаться , если вообще возвращаться возвращается тот же объект. Метод getSymbolInfo не содержит ошибок и работает так, как задумал программист. Однако стоит провести рефакторинг метода getSymbolInfo и снимаем петлю делать пока вместе с переменной retVal .
Я предлагаю следующий код: bool NativeModule::getSymbolInfo( /* .
*/) const
{
uint32_t modId = 0, libId = 0;
if (modName == nullptr || libName == nullptr || nid == nullptr)
{
return false;
}
if (!isEncodedSymbol(encSymbol))
{
*modName = "";
*libName = "";
*nid = 0;
return true;
}
if (!decodeSymbol(encSymbol, &modId, &libId, nid))
{
LOG_ERR("fail to decode encoded symbol");
return false;
}
if (!getModNameFromId(modId, mods, modName))
{
LOG_ERR("fail to get module name for symbol: %s in %s",
encSymbol.c_str(), fileName.c_str());
return false;
}
if (!getLibNameFromId(libId, libs, libName))
{
LOG_ERR("fail to get library name");
return false;
}
return true;
}
Здесь я сделал следующее:
- удалил петлю делать пока и дополнительная переменная retVal ;
- каждая проверка переменной retVal заменяется проверкой результата вызова соответствующей функции;
- каждый перерыв цикл делать пока заменено возвратом желаемого значения истинный / ЛОЖЬ .
Мы точно знаем, какое значение вернуть из анализа переменных.
retVal что и было осуществлено ранее.
Заключение
Конечно, это не все ошибки и недостатки, которые мы нашли в GPCS4. Некоторые случаи было довольно сложно описать, поэтому я не включил их в статью.Желаем разработчикам GPCS4 успехов в дальнейшем развитии эмулятора и рекомендуем проверить текущую версию проекта анализатором PVS-Studio. Достаточно для этого скачать дистрибутив анализатор и запросите бесплатную лицензию для проектов с открытым исходным кодом.
Если вас интересует статический анализ вообще и PVS-Studio в частности, сейчас самое время попробовать.
Вы можете проверить GPCS4 после нас, а можете проверить свой проект :) Спасибо за внимание! Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой Проверяем эмулятор GPCS4: сможем ли мы когда-нибудь поиграть в «Bloodborne» на ПК? Теги: #открытый исходный код #gamedev #C++ #статистический анализ #статический анализ
-
Czi2
19 Oct, 24 -
Параметры
19 Oct, 24 -
Ex.ua Остался Один!
19 Oct, 24
