Amazon Lumberyard: el grito del alma

Los juegos son uno de los productos de software más populares. Esta es una gran industria en la que un nuevo motor de juegos: Amazon Lumberyard. El proyecto todavía está en estado beta y tiene tiempo para corregir errores y mejorar la calidad del código. Los desarrolladores del motor tienen mucho trabajo por hacer en el futuro cercano para no decepcionar a millones de jugadores y desarrolladores de juegos.

Introduccion

Amazon Lumberyard es un motor de juegos multiplataforma AAA gratuito desarrollado por Amazon y basado en la arquitectura del motor CryEngine , que fue licenciada por Crytek en 2015. Por cierto, el análisis de CryEngine ya lo hice dos veces en agosto de 2016 y abril de 2017 . Al mismo tiempo, debo señalar que después de un año, el código solo empeoró. Y el otro día, decidí ver qué hacía Amazon en función de este motor de juego. Funcionaron muy bien en el medio ambiente. La documentación para desarrolladores y software para implementar un entorno de trabajo es muy interesante y de alto nivel. ¡Pero el problema es nuevamente con el código! Espero que Amazon tenga muchos más recursos para trabajar con el proyecto y sigan prestando atención a la calidad del código. Con esta revisión, espero llamar la atención de los desarrolladores sobre la calidad del código y presionar por un nuevo enfoque en el desarrollo de este motor de juego. El estado actual del código estaba en un estado tan deplorable que cambié el título del artículo varias veces y volví a dibujar la imagen del título mientras miraba el informe con los resultados del análisis. La primera versión de la imagen fue menos emotiva:

Analizamos las fuentes de Amazon Lumberyard de la última versión disponible 1.14.0.1. El código fuente se toma del repositorio en Github . Uno de los primeros juegos en el motor de Lumberyard debería ser Star Citizen . Los jugadores potenciales que la están esperando, también los invito a echar un vistazo a lo que actualmente está "bajo el capó" del juego.

Integración con PVS-Studio

PVS-Studio se utilizó como analizador de código estático. Está disponible para Windows, Linux y macOS. Es decir Para el análisis de proyectos multiplataforma, incluso hay algo para elegir para un trabajo más cómodo. Además de C y C ++, se admite el análisis de proyectos en el lenguaje C #. Planes de Java . La gran mayoría del código en el mundo está escrito en los idiomas enumerados (no sin errores, por supuesto), así que pruebe el analizador PVS-Studio en su proyecto, aprenderá muchas cosas interesantes ;-).

Como sistema de ensamblaje de Lumberyard, se utiliza WAF, que también estaba en CryEngine. El analizador no tiene una forma especial de integrarse con este sistema de ensamblaje. Decidí trabajar con un proyecto en Windows y elegí este método para comenzar el análisis: un sistema de monitoreo de compilación . El archivo de proyecto para Visual Studio se genera automáticamente. Se puede usar para construir el proyecto y ver el informe del analizador.

La lista de comandos para el análisis se ve así:

cd /path/to/lumberyard/dev lmbr_waf.bat ... CLMonitor.exe monitor MSBuild.exe ... LumberyardSDK_vs15.sln ... CLMonitor.exe analyze --log /path/to/report.plog 

El informe, como dije, se puede ver en Visual Studio.

Sobre Igor y Qualcomm

Amazon Lumberyard se posiciona como un motor de juegos multiplataforma. Promover un proyecto a las masas con tal característica es fácil, pero mantenerlo es muy difícil. Una de las advertencias de PVS-Studio se emitió en un fragmento de código donde el programador Igor luchó con el compilador Qualcomm. Quizás resolvió su problema, pero dejó un código extremadamente sospechoso. Decidí dibujarlo con una imagen.

V523 La declaración 'then' es equivalente a la declaración 'else'. toglsloperand.c 700

Aquí se ejecuta el mismo código, independientemente de la condición calculada. En el contexto de los comentarios que quedan, tal decisión parece sospechosa.

En general, el proyecto no es el único lugar donde las condiciones deben simplificarse para mayor claridad o para corregir un error real. Aquí hay una lista de dichos lugares:

• V523 La declaración 'then' es equivalente a la declaración 'else'. livingentity.cpp 1385
• V523 La declaración 'then' es equivalente a la declaración 'else'. tometalinstruction.c 4201
• V523 La declaración 'then' es equivalente a la declaración 'else'. scripttable.cpp 905
• V523 La declaración 'then' es equivalente a la declaración 'else'. budgettingsystem.cpp 701
• V523 La declaración 'then' es equivalente a la declaración 'else'. editorframeworkapplication.cpp 562
• V523 La declaración 'then' es equivalente a la declaración 'else'. Particleitem.cpp 130
• V523 La declaración 'then' es equivalente a la declaración 'else'. trackviewnodes.cpp 1223
• V523 La declaración 'then' es equivalente a la declaración 'else'. propertyyoarchive.cpp 447

Python ++

El analizador encontró un código tan divertido:

V709 CWE-682 Comparación sospechosa encontrada: 'a == b == c'. Recuerde que 'a == b == c' no es igual a 'a == b && b == c'. toglslinstruction.c 564

 void CallBinaryOp(....) { .... uint32_t src1SwizCount = GetNumSwizzleElements(....); uint32_t src0SwizCount = GetNumSwizzleElements(....); uint32_t dstSwizCount = GetNumSwizzleElements(....); .... if (src1SwizCount == src0SwizCount == dstSwizCount) // <= { .... } .... } 

En C ++, dicho código, desafortunadamente, se compila con éxito, pero no se ejecuta en absoluto como podría parecer. Las expresiones en C ++ se evalúan en orden de prioridad, ejecutando castas implícitas, si es necesario.

Tal verificación sería correcta, por ejemplo, en el lenguaje Python. Pero en esta situación, el desarrollador "se pegó un tiro en el pie".

3 tomas de control más:

• V709 CWE-682 Comparación sospechosa encontrada: 'a == b == c'. Recuerde que 'a == b == c' no es igual a 'a == b && b == c'. toglslinstruction.c 654
• V709 CWE-682 Comparación sospechosa encontrada: 'a == b == c'. Recuerde que 'a == b == c' no es igual a 'a == b && b == c'. toglslinstruction.c 469
• V709 CWE-682 Comparación sospechosa encontrada: 'a == b == c'. Recuerde que 'a == b == c' no es igual a 'a == b && b == c'. tometalinstruction.c 539

El primero y uno de los mejores diagnósticos.

Se tratará de la advertencia V501 , el primer diagnóstico de uso general en PVS-Studio. Los errores encontrados solo por este diagnóstico pueden ser suficientes para escribir un artículo. Y el proyecto Amazon Lumberyard lo demuestra muy bien.

Desafortunadamente, es aburrido mirar los errores del mismo tipo durante mucho tiempo, por lo que en esta sección solo comentaré un par de fragmentos de código, y el resto de las advertencias se enumerarán.

V501 Hay subexpresiones idénticas a la izquierda y a la derecha de '||' operador: hotX <0 || hotX <0 editorutils.cpp 166

 QCursor CMFCUtils::LoadCursor(....) { .... if (!pm.isNull() && (hotX < 0 || hotX < 0)) { QFile f(path); f.open(QFile::ReadOnly); QDataStream stream(&f); stream.setByteOrder(QDataStream::LittleEndian); f.read(10); quint16 x; stream >> x; hotX = x; stream >> x; hotY = x; } .... } 

La condición carece de la variable Y caliente . Error tipográfico clásico.

V501 Hay subexpresiones idénticas 'sp.m_pTexture == m_pTexture' a la izquierda y a la derecha del operador '&&'. shadercomponents.h 487

V501 Hay subexpresiones idénticas 'sp.m_eCGTextureType == m_eCGTextureType' a la izquierda y a la derecha del operador '&&'. shadercomponents.h 487

 bool operator != (const SCGTexture& sp) const { if (sp.m_RegisterOffset == m_RegisterOffset && sp.m_Name == m_Name && sp.m_pTexture == m_pTexture && // <= 1 sp.m_RegisterCount == m_RegisterCount && sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_BindingSlot == m_BindingSlot && sp.m_Flags == m_Flags && sp.m_pAnimInfo == m_pAnimInfo && sp.m_pTexture == m_pTexture && // <= 1 sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_bSRGBLookup == m_bSRGBLookup && sp.m_bGlobal == m_bGlobal) { return false; } return true; } 

Se encontraron dos copias en este fragmento a la vez. Para mayor claridad, pinté flechas.

V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '==': pSrc.GetLen () == pSrc.GetLen () fbxpropertytypes.h 978

 inline bool FbxTypeCopy(FbxBlob& pDst, const FbxString& pSrc) { bool lCastable = pSrc.GetLen() == pSrc.GetLen(); FBX_ASSERT( lCastable ); if( lCastable ) pDst.Assign(pSrc.Buffer(), (int)pSrc.GetLen()); return lCastable; } 

Aquí quiero saludar a los desarrolladores de AUTODESK . Este error es de su biblioteca FBX SDK . Variables confusas pSrc y pDst . Creo que, además de Lumberyard, también hay muchos otros usuarios cuyos proyectos dependen del código con este error.

V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '&&': pTS-> pRT_ALD_1 && pTS-> pRT_ALD_1 d3d_svo.cpp 857

 void CSvoRenderer::ConeTracePass(SSvoTargetsSet* pTS) { .... if (pTS->pRT_ALD_1 && pTS->pRT_ALD_1) { static int nPrevWidth = 0; if (....) { .... } else { pTS->pRT_ALD_1->Apply(10, m_nTexStateLinear); pTS->pRT_RGB_1->Apply(11, m_nTexStateLinear); } } .... } 

De vuelta al código de Lumberyard. En la condición, se marca el mismo puntero pTS-> pRT_ALD_1 . Uno de ellos debería haber sido pTS-> pRT_RGB_1 . Quizás incluso después de la explicación no es posible ver la diferencia de inmediato, pero hay una: la diferencia está en las subcadenas cortas ALD y RGB . Si le dicen que la revisión de código manual es suficiente, muestre este ejemplo.

Y si este ejemplo no es suficiente, entonces hay 5 más similares.

• V501 Hay subexpresiones idénticas a la izquierda y a la derecha de '||' operador :! pTS-> pRT_ALD_0 ||! pTS-> pRT_ALD_0 d3d_svo.cpp 1041
• V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '&&': m_pRT_AIR_MIN && m_pRT_AIR_MIN d3d_svo.cpp 1808
• V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '&&': m_pRT_AIR_MAX && m_pRT_AIR_MAX d3d_svo.cpp 1819
• V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '&&': m_pRT_AIR_SHAD && m_pRT_AIR_SHAD d3d_svo.cpp 1830
• V501 Hay subexpresiones idénticas a la izquierda y a la derecha del operador '&&': s_pPropertiesPanel && s_pPropertiesPanel entityobject.cpp 1700

Y como prometí, aquí hay una lista de las advertencias V501 restantes sin ejemplos de código:

Sobre la posición de la cámara en los juegos

Existe el segundo diagnóstico más rápido en PVS-Studio: V502 . Este diagnóstico es más antiguo que algunos lenguajes de programación nuevos, en los que ya no se puede cometer dicho error. Y para C ++, este error será relevante, tal vez siempre.

Comencemos con un pequeño ejemplo simple.

V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '+'. zipencryptor.cpp 217

 bool ZipEncryptor::ParseKey(....) { .... size_t pos = i * 2 + (v1 == 0xff) ? 1 : 2; RCLogError("....", pos); return false; .... } 

La operación de adición tiene una prioridad más alta que el operador ternario. Por lo tanto, esta expresión tiene una lógica de cálculo completamente diferente de lo que el autor asumió.

Puede corregir el error de esta manera:

 size_t pos = i * 2 + (v1 == 0xff ? 1 : 2); 

V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '-'. 3dengine.cpp 1898

 float C3DEngine::GetDistanceToSectorWithWater() { .... return (bCameraInTerrainBounds && (m_pTerrain && m_pTerrain->GetDistanceToSectorWithWater() > 0.1f)) ? m_pTerrain->GetDistanceToSectorWithWater() : max(camPostion.z - OceanToggle::IsActive() ? OceanRequest::GetOceanLevel() : GetWaterLevel(), 0.1f); } 

Y aquí hay un código de ejemplo donde trabajan con la posición de la cámara. Un ejemplo es difícil de percibir con los ojos y hay un error en él. Para la publicación, el formato del código ha cambiado, pero en el archivo fuente este código es aún más ilegible.

Hay un error en esta subcadena:

 camPostion.z - OceanToggle::IsActive() ? .... : .... 

Si la cámara en su juego de repente comenzó a comportarse de manera inapropiada, entonces debe saber que los desarrolladores guardaron en el análisis de código estático: D.

Otros ejemplos con advertencias similares:

• V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '-'. scriptbind_ai.cpp 5203
• V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '+'. qcolumnwidget.cpp 136
• V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '&&'. shapetool.h 98

Legado de CryEngine

Amazon Lumberyard se basa en el código CryEngine . Y no en su mejor versión. Llegué a tal conclusión mirando el informe del analizador. Algunos errores encontrados ya se han corregido en la última versión de CryEngine para mis dos revisiones de código, pero todavía están presentes en el código de Lumberyard. Además, durante el año pasado, el analizador se mejoró significativamente y fue posible encontrar errores adicionales que ahora están presentes en dos motores de juego. Pero con Lumberyard la situación es peor. Amazon esencialmente heredó toda la deuda técnica de CryEngine. Pero su propio deber técnico, por supuesto, aparece en cada empresa por su cuenta :).

En esta sección, le daré un par de errores que se corrigieron en la última versión de CryEngine, y ahora siguen siendo solo un problema del proyecto Lumberyard.

V519 La variable 'BlendFactor [2]' tiene valores asignados dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 1283, 1284. ccrydxgldevicecontext.cpp 1284

Aproximadamente tales emociones serán experimentadas por los desarrolladores de Lumberyard cuando descubran que este error se mantuvo solo con ellos.

Por cierto, hay dos más:

• V519 La variable 'm_auBlendFactor [2]' tiene valores asignados dos veces sucesivamente. Quizás esto sea un error. Líneas de verificación: 919, 920. ccrydxgldevicecontext.cpp 920
• V519 La variable 'm_auBlendFactor [2]' tiene valores asignados dos veces sucesivamente. Quizás esto sea un error. Verifique las líneas: 926, 927. ccrydxgldevicecontext.cpp 927

Hay tal error:

V546 El miembro de una clase se inicializa por sí mismo: 'eConfigMax (eConfigMax.VeryHigh)'. Partícula de partículas. 1837

 ParticleParams() : .... fSphericalApproximation(1.f), fVolumeThickness(1.0f), fSoundFXParam(1.f), eConfigMax(eConfigMax.VeryHigh), // <= fFadeAtViewCosAngle(0.f) {} 

En CryEngine, esta clase generalmente se reescribió, pero aquí se mantuvo el error de inicialización.

V521 Tales expresiones que usan el operador ',' son peligrosas. Asegúrese de que la expresión '! SWords [iWord] .empty (), iWord ++' sea correcta. tacticalpointsystem.cpp 3376

 bool CTacticalPointSystem::Parse(....) const { string sInput(sSpec); const int MAXWORDS = 8; string sWords[MAXWORDS]; int iC = 0, iWord = 0; for (; iWord < MAXWORDS; !sWords[iWord].empty(), iWord++) { sWords[iWord] = sInput.Tokenize("_", iC); } .... } 

El ciclo sospechoso que CryEngine ya ha reescrito también.

Los errores viven más de lo que piensas

Los usuarios que comienzan a usar PVS-Studio por primera vez tienen una situación aproximadamente idéntica: encuentran un error, descubren que se agregó hace unos meses y están felices de darse cuenta de que milagrosamente evitaron la manifestación del problema entre sus usuarios. Muchos de nuestros clientes llegaron al uso regular de PVS-Studio precisamente después de tales historias.

A veces, para iniciar los procesos de control de calidad del código, una empresa debe visitar esas situaciones varias veces. Aquí hay un ejemplo sobre CryEngine y Lumberyard:

V557 CWE-119 Arreglo de arrastre es posible. El índice 'id' apunta más allá de la matriz. gameobjectsystem.cpp 113

 uint32 CGameObjectSystem::GetExtensionSerializationPriority(....) { if (id > m_extensionInfo.size()) { return 0xffffffff; // minimum possible priority } else { return m_extensionInfo[id].serializationPriority; } } 

Como saben, Amazon Lumberyard no se basa en la última versión de CryEngine. Sin embargo, con la ayuda del analizador PVS-Studio, fue posible encontrar un error que ahora está presente en dos motores de juego. Era necesario verificar el índice utilizando el operador '> =' ...

El error de indexación es grave. ¡Además, hay seis de esos lugares ! Aquí hay otro ejemplo:

V557 CWE-119 Arreglo de arrastre es posible. El índice 'índice' apunta más allá de la matriz. vehicleeatgroup.cpp 73

 CVehicleSeat* CVehicleSeatGroup::GetSeatByIndex(unsigned index) { if (index >= 0 && index <= m_seats.size()) { return m_seats[index]; } return NULL; } 

Alguien cometió los errores del mismo tipo, y no se corrigieron solo porque no se habían incluido una vez en las revisiones de errores de CryEngine.

Advertencias restantes:

• V557 CWE-119 Arreglo de arrastre es posible. El índice 'id' apunta más allá de la matriz. gameobjectsystem.cpp 195
• V557 CWE-119 Arreglo de arrastre es posible. El índice 'id' apunta más allá de la matriz. gameobjectsystem.cpp 290
• V557 CWE-119 Arreglo de arrastre es posible. El índice 'stateId' apunta más allá del límite de la matriz. vehicleanimation.cpp 311
• V557 CWE-119 Arreglo de arrastre es posible. El índice 'stateId' apunta más allá del límite de la matriz. vehicleanimation.cpp 354

La larga existencia de errores en el código solo puede explicarse por el nivel correspondiente de prueba del proyecto. Se cree que el análisis estático encuentra errores solo en el código no utilizado. Entonces, esto no es así. Los desarrolladores olvidan que la mayoría de los usuarios sufren silenciosamente de errores no evidentes en los programas, y la manifestación de estos errores muy raros a menudo afecta negativamente el trabajo de toda la empresa, la reputación y sus ventas, si las hay.

Diferentes tonos de programación de copiar y pegar

Cuando llegaste a esta sección del artículo, probablemente notaste que la programación de copiar y pegar es la causa de muchos problemas. En PVS-Studio, la búsqueda de tales errores se implementa en varios diagnósticos. Esta sección dará ejemplos de copiar y pegar encontrados con el V561 .

Aquí hay un ejemplo de código sospechoso al declarar variables con el mismo nombre en ámbitos superpuestos.

V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'pLibrary' que declararlo de nuevo. Declaración previa: entityobject.cpp, línea 4703. entityobject.cpp 4706

 void CEntityObject::OnMenuConvertToPrefab() { .... IDataBaseLibrary* pLibrary = GetIEditor()->Get....; if (pLibrary == NULL) { IDataBaseLibrary* pLibrary = GetIEditor()->Get....; } if (pLibrary == NULL) { QString sError = tr(....); CryMessageBox(....); return; } .... } 

El puntero pLibrary no se sobrescribe como se esperaba. La inicialización de este puntero se copió completamente bajo la condición junto con la declaración de tipo.

Voy a enumerar todos los lugares similares:

• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'eType' que declararlo de nuevo. Declaración previa: toglsloperand.c, línea 838. toglsloperand.c 1224
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'eType' que declararlo de nuevo. Declaración previa: toglsloperand.c, línea 838. toglsloperand.c 1305
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'rSkelPose' que declararlo de nuevo. Declaración previa: attachmanager.cpp, línea 409. attachmanager.cpp 458
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'nThreadID' que declararlo de nuevo. Declaración previa: d3dmeshbaker.cpp, línea 797. d3dmeshbaker.cpp 867
• V561 CWE-563 Probablemente sea mejor asignar un valor a la variable 'directoryNameList' que declararlo de nuevo. Declaración previa: assetimportermanager.cpp, línea 720. assetimportermanager.cpp 728
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'pNode' que declararlo de nuevo. Declaración previa: breakpointsctrl.cpp, línea 340. breakpointsctrl.cpp 349
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'pLibrary' que declararlo de nuevo. Declaración previa: prefabobject.cpp, línea 1443. prefabobject.cpp 1446
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'pLibrary' que declararlo de nuevo. Declaración previa: prefabobject.cpp, línea 1470. prefabobject.cpp 1473
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'cmdLine' que declararlo de nuevo. Declaración previa: fileutil.cpp, línea 110. fileutil.cpp 130
• V561 CWE-563 Probablemente sea mejor asignar valor a la variable 'sfunctionArgs' que declararlo de nuevo. Declaración anterior: attributeitemlogiccallbacks.cpp, línea 291. attributeitemlogiccallbacks.cpp 303
• V561 CWE-563 Probablemente sea mejor asignar un valor a la variable 'curveName' que declararlo de nuevo. Declaración previa: qgradientselectorwidget.cpp, línea 475. qgradientselectorwidget.cpp 488

Una larga lista ... algunos de los lugares enumerados son incluso copias completas del ejemplo descrito.

Inicialización del valor propio

En el código del motor del juego, se encontraron muchos lugares donde la variable se asigna a sí misma. En algún lugar, este código permaneció para la depuración, en algún lugar el código está simplemente bellamente diseñado (a menudo también es una fuente de errores), por lo que le daré una pieza de código que es más sospechosa para mí.

V570 La variable 'behaviourParams.ignoreOnVehicleDestroyed' se asigna a sí misma. vehiclecomponent.cpp 168

 bool CVehicleComponent::Init(....) { .... if (!damageBehaviorTable.getAttr(....) { behaviorParams.ignoreOnVehicleDestroyed = false; } else { behaviorParams.ignoreOnVehicleDestroyed = // <= behaviorParams.ignoreOnVehicleDestroyed; // <= } .... } 

En la vista actual, la rama else no es necesaria en absoluto. Pero quizás este código contiene un error: querían asignar el valor opuesto a la variable:

 bValue = !bValue 

Pero es mejor que los desarrolladores se familiaricen con los resultados de este diagnóstico.

Manejo de problemas

Esta sección dará muchos ejemplos cuando algo salió mal durante el manejo de errores.

Ejemplo 1

V606 token sin propietario 'nullptr'. dx12rootsignature.cpp 599

 RootSignature* RootSignatureCache::AcquireRootSignature(....) { .... RootSignature* result = new RootSignature(m_pDevice); if (!result->Init(params)) { DX12_ERROR("Could not create root signature!"); nullptr; } m_RootSignatureMap[hash] = result; return result; } } 

Olvidé escribir return nullptr; . Ahora, el valor no válido de la variable de resultado se usará en otra parte del código.

El mismo código exacto se copió a un lugar más:

• V606 token sin propietario 'nullptr'. dx12rootsignature.cpp 621

Ejemplo 2

V606 token sin propietario 'falso'. fillspacetool.cpp 191

 bool FillSpaceTool::FillHoleBasedOnSelectedElements() { .... if (validEdgeList.size() == 2) { .... } if (validEdgeList.empty()) { .... for (int i = 0, iVertexSize(....); i < iVertexSize; ++i) { validEdgeList.push_back(....); } } if (validEdgeList.empty()) // <= { false; // <= fail } std::vector<BrushEdge3D> linkedEdgeList; std::set<int> usedEdgeSet; linkedEdgeList.push_back(validEdgeList[0]); // <= fail .... } 

Un ejemplo muy interesante de un error con una declaración de retorno faltante. Ahora es posible una situación de acceso a un contenedor vacío al índice.

Ejemplo 3

V564 CWE-480 El operador '&' se aplica al valor de tipo bool. Probablemente haya olvidado incluir paréntesis o tenga la intención de utilizar el operador '&&'. toglslinstruction.c 2914

 void SetDataTypes(....) { .... // Check assumption that both the values which MOVC might pick // have the same basic data type. if(!psContext->flags & HLSLCC_FLAG_AVOID_TEMP_REGISTER_ALIASING) { ASSERT(GetOperandDataType(psContext, &psInst->asOperands[2]) == GetOperandDataType(psContext, &psInst->asOperands[3])); } .... } 

Se verificó incorrectamente la presencia de bits en la bandera. El operador de negación se aplica al valor del indicador, no a la expresión completa. Escribe correctamente así:

 if(!(psContext->flags & ....)) 

Más advertencias similares:

• V564 CWE-480 El '|' El operador se aplica al valor de tipo bool. Probablemente te hayas olvidado de incluir paréntesis o tengas la intención de usar '||' operador d3dhwshader.cpp 1832
• V564 CWE-480 El operador '&' se aplica al valor de tipo bool. Probablemente haya olvidado incluir paréntesis o tenga la intención de utilizar el operador '&&'. trackviewdialog.cpp 2112
• V564 CWE-480 El '|' El operador se aplica al valor de tipo bool. Probablemente te hayas olvidado de incluir paréntesis o tengas la intención de usar '||' operador imagecompiler.cpp 1039

Ejemplo 4

V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1491

 static std::vector<std::string> PyGetPrefabLibrarys() { CPrefabManager* pPrefabManager = GetIEditor()->GetPrefabMa....; if (!pPrefabManager) { std::runtime_error("Invalid Prefab Manager."); } .... } 

Error al lanzar la excepción. Era necesario escribir así:

 throw std::runtime_error("Invalid Prefab Manager."); 

La lista completa de tales errores:

• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1515
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1521
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1543
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1549
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1603
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1619
• V596 CWE-390 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw runtime_error (FOO); prefabobject.cpp 1644

Un par de problemas al trabajar con memoria

V549 CWE-688 El primer argumento de la función 'memcmp' es igual al segundo argumento. meshutils.h 894

 struct VertexLess { .... bool operator()(int a, int b) const { .... if (m.m_links[a].links.size() != m.m_links[b].links.size()) { res = (m.m_links[a].links.size() < m.m_links[b].links.size()) ? -1 : +1; } else { res = memcmp(&m.m_links[a].links[0], &m.m_links[a].links[0], sizeof(m.m_links[a].links[0]) * m.m_links[a].links.size()); } .... } .... }; 

La condición compara los tamaños de dos vectores. Si son iguales, en la rama else los valores de los primeros elementos de los vectores se comparan utilizando la función memcmp () . ¡Pero los argumentos primero y segundo para esta función son los mismos! El acceso a los elementos de la matriz es bastante engorroso. Hay índices a y b . Lo más probable es que haya un error tipográfico en ellos.

V611 CWE-762 La memoria se asignó usando el operador 'nuevo T []' pero se liberó usando el operador 'borrar'. Considere inspeccionar este código. Probablemente sea mejor usar 'borrar [] datos;'. vectorn.h 102

 ~vectorn_tpl() { if (!(flags & mtx_foreign_data)) { delete[] data; } } vectorn_tpl& operator=(const vectorn_tpl<ftype>& src) { if (src.len != len && !(flags & mtx_foreign_data)) { delete data; // <= data = new ftype[src.len]; } .... } 

La memoria del puntero de datos se está liberando con una declaración no válida. El operador delete [] debe usarse en todas partes .

Código inalcanzable

V779 CWE-561 Código inalcanzable detectado. Es posible que haya un error presente. fbxskinimporter.cpp 67

 Events::ProcessingResult FbxSkinImporter::ImportSkin(....) { .... if (BuildSceneMeshFromFbxMesh(....) { context.m_createdData.push_back(std::move(createdData)); return Events::ProcessingResult::Success; // <= } else { return Events::ProcessingResult::Failure; // <= } context.m_createdData.push_back(); // <= fail return Events::ProcessingResult::Success; } 

Todas las ramas de la declaración condicional terminan con la salida de la función. Sin embargo, algunos códigos no se ejecutan.

V779 CWE-561 Código inalcanzable detectado. Es posible que haya un error presente. dockablelibrarytreeview.cpp 153

 bool DockableLibraryTreeView::Init(IDataBaseLibrary* lib) { .... if (m_treeView && m_titleBar && m_defaultView) { if (m_treeView->topLevelItemCount() > 0) { ShowTreeView(); } else { ShowDefaultView(); } return true; // <= } else { return false; // <= } emit SignalFocused(this); // <= fail } 

Es fácil detectar un error en este fragmento de código. Pero si escribe el código durante mucho tiempo, entonces la atención se reduce bruscamente y esos errores caen fácilmente en el proyecto.

V622 CWE-478 Considere la posibilidad de inspeccionar la declaración 'switch'. Es posible que falte el primer operador de 'caso'. datum.cpp 872

 AZ_INLINE bool IsDataGreaterEqual(....) { switch (type.GetType()) { AZ_Error("ScriptCanvas", false, "...."); return false; case Data::eType::Number: return IsDataGreaterEqual<Data::NumberType>(lhs, rhs); .... case Data::eType::AABB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::AABBType>::GetName()); return false; case Data::eType::OBB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::OBBType>::GetName()); return false; .... } 

Si el modificador contiene código fuera de la declaración case / default , nunca se ejecuta.

Conclusión

El artículo incluye 95 advertencias del analizador, 25 de ellas con ejemplos de código. ¿Cuánto material proviene del informe completo del analizador? Rápidamente desplacé solo un tercio de las alertas de alto nivel . También hay Mediano y Bajo, un grupo de diagnósticos para buscar optimizaciones y otras oportunidades sin explotar del analizador: estos son cientos de errores más obvios y miles de advertencias inexploradas.

Y luego el lector debe hacerse la pregunta: "¿Es posible con este enfoque del proyecto lanzar un buen motor de juego?". No hay código de control de calidad. El código CryEngine con errores antiguos se tomó como base, se agregaron nuevos errores. CryEngine se finaliza solo después de la próxima revisión del código. ¡Amazon tiene todas las oportunidades con sus recursos para trabajar en la dirección de la calidad del código y lanzar el motor de juego más genial!

No te enfades mucho. Entre los clientes de PVS-Studio hay más de treinta compañías que participan en juegos. Puede familiarizarse con ellos y sus productos en la página de nuestro sitio web " Clientes " seleccionando el filtro "Desarrollo de juegos". Entonces, estamos mejorando gradualmente el mundo. Quizás podamos mejorar Amazon Lumberyard :).

Un colega recientemente escribió un artículo sobre el tema de la calidad del software de juegos, sugiero que le interese leer: " Análisis estático en la industria de los videojuegos: los 10 principales errores de software ".

Enlace para descargar el analizador PVS-Studio , ¿cómo puede prescindir de él? ;-)

Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov. Amazon Lumberyard: Un grito de angustia