Amazon Lumberyard: o grito da alma

Os jogos são um dos produtos de software mais populares. Esta é uma indústria enorme em que um novo mecanismo de jogo - o Amazon Lumberyard. O projeto ainda está no status beta e tem tempo para corrigir erros e melhorar a qualidade do código. Os desenvolvedores do mecanismo têm muito trabalho a fazer em um futuro próximo para não decepcionar milhões de jogadores e desenvolvedores de jogos.

1. Introdução

O Amazon Lumberyard é um mecanismo de jogo gratuito de plataforma cruzada AAA desenvolvido pela Amazon e baseado na arquitetura de mecanismo CryEngine , que foi licenciada pela Crytek em 2015. A propósito, a análise do CryEngine já foi feita duas vezes por mim em agosto de 2016 e abril de 2017 . Ao mesmo tempo, devo observar que, após um ano, o código só piorou. E outro dia, decidi ver o que a Amazon fazia com base nesse mecanismo de jogo. Eles trabalharam muito bem no meio ambiente. A documentação para desenvolvedores e software para implantar um ambiente de trabalho é muito legal e de alto nível. Mas o problema está novamente com o código! Espero que a Amazon tenha muito mais recursos para trabalhar com o projeto, e eles ainda prestem atenção à qualidade do código. Com esta revisão, espero chamar a atenção dos desenvolvedores para a qualidade do código e pressionar por uma nova abordagem no desenvolvimento desse mecanismo de jogo. O estado atual do código estava em um estado tão deplorável que eu mudei o título do artigo várias vezes e redesenhou a imagem do título enquanto olhava o relatório com os resultados da análise. A primeira versão da imagem foi menos emocional:

Analisamos as fontes do Amazon Lumberyard da versão mais recente disponível 1.14.0.1. O código fonte é retirado do repositório no Github . Um dos primeiros jogos no mecanismo Lumberyard deve ser o Star Citizen . Jogadores potenciais que estão esperando por ela, também convido você a dar uma olhada no que está atualmente "sob o capô" do jogo.

Integração com PVS-Studio

O PVS-Studio foi usado como um analisador de código estático. Está disponível para Windows, Linux e macOS. I.e. para a análise do projeto de plataforma cruzada, ainda há algo a ser escolhido para um trabalho mais confortável. Além de C e C ++, a análise de projetos na linguagem C # é suportada. Planos Java . A grande maioria dos códigos do mundo está escrita nos idiomas listados (não sem erros, é claro); portanto, tente o analisador PVS-Studio em seu projeto, você aprenderá muitas coisas interessantes ;-).

Como sistema de montagem da Lumberyard, é utilizado o WAF, que também estava no CryEngine. O analisador não possui uma maneira especial de integrar-se com este sistema de montagem. Decidi trabalhar com um projeto no Windows e escolhi este método para iniciar a análise: um sistema de monitoramento de compilação . O arquivo de projeto para o Visual Studio é gerado automaticamente. Ele pode ser usado para criar o projeto e visualizar o relatório do analisador.

A lista de comandos para análise é mais ou menos assim:

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

O relatório, como eu disse, pode ser exibido no Visual Studio.

Sobre Igor e Qualcomm

O Amazon Lumberyard se posiciona como um mecanismo de jogo de plataforma cruzada. É fácil promover um projeto para as massas com esse recurso, mas é muito difícil mantê-lo. Um dos avisos do PVS-Studio foi emitido em um fragmento de código no qual o programador Igor lutou com o compilador Qualcomm. Talvez ele tenha resolvido o problema, mas deixou um código extremamente suspeito. Eu decidi desenhá-lo com uma imagem.

V523 A instrução 'then' é equivalente à instrução 'else'. toglsloperand.c 700

Aqui o mesmo código é executado, independentemente da condição calculada. No contexto dos comentários deixados, essa decisão parece suspeita.

Em geral, o projeto não é o único local em que as condições precisam ser simplificadas para maior clareza ou para corrigir um erro real. Aqui está uma lista desses lugares:

• V523 A instrução 'then' é equivalente à instrução 'else'. livingentity.cpp 1385
• V523 A instrução 'then' é equivalente à instrução 'else'. tometalinstruction.c 4201
• V523 A instrução 'then' é equivalente à instrução 'else'. scripttable.cpp 905
• V523 A instrução 'then' é equivalente à instrução 'else'. budgetingsystem.cpp 701
• V523 A instrução 'then' é equivalente à instrução 'else'. editorframeworkapplication.cpp 562
• V523 A instrução 'then' é equivalente à instrução 'else'. particleitem.cpp 130
• V523 A instrução 'then' é equivalente à instrução 'else'. trackviewnodes.cpp 1223
• V523 A instrução 'then' é equivalente à instrução 'else'. propertyoarchive.cpp 447

Python ++

O analisador encontrou um código tão engraçado:

V709 CWE-682 Comparação suspeita encontrada: 'a == b == c'. Lembre-se de que 'a == b == c' não é 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) // <= { .... } .... } 

Infelizmente, em C ++, esse código é compilado com êxito, mas não é executado como pode parecer. Expressões em C ++ são avaliadas em ordem de prioridade, executando castas implícitas, se necessário.

Essa verificação estaria correta, por exemplo, na linguagem Python. Mas nessa situação, o desenvolvedor "deu um tiro no pé".

Mais 3 tiros de controle:

• V709 CWE-682 Comparação suspeita encontrada: 'a == b == c'. Lembre-se de que 'a == b == c' não é igual a 'a == b && b == c'. toglslinstruction.c 654
• V709 CWE-682 Comparação suspeita encontrada: 'a == b == c'. Lembre-se de que 'a == b == c' não é igual a 'a == b && b == c'. toglslinstruction.c 469
• V709 CWE-682 Comparação suspeita encontrada: 'a == b == c'. Lembre-se de que 'a == b == c' não é igual a 'a == b && b == c'. tometalinstruction.c 539

O primeiro e um dos melhores diagnósticos

Será sobre o aviso V501 - o primeiro diagnóstico de uso geral no PVS-Studio. Os erros encontrados apenas neste diagnóstico podem ser suficientes para escrever um artigo. E o projeto Amazon Lumberyard demonstra isso muito bem.

Infelizmente, é chato olhar para erros do mesmo tipo por um longo tempo; portanto, nesta seção, comentarei apenas alguns fragmentos de código e o restante dos avisos serão listados.

V501 Existem subexpressões idênticas à esquerda e à direita do '||' 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; } .... } 

A condição não possui a variável Y quente . Erro de digitação clássico.

V501 Existem subexpressões idênticas 'sp.m_pTexture == m_pTexture' à esquerda e à direita do operador '&&'. shadercomponents.h 487

V501 Existem subexpressões idênticas 'sp.m_eCGTextureType == m_eCGTextureType' à esquerda e à direita do 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; } 

Duas cópias de pastas foram encontradas neste fragmento de uma só vez. Para maior clareza, pintei flechas.

V501 Existem subexpressões idênticas à esquerda e à direita do 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; } 

Aqui, quero dizer olá aos desenvolvedores da AUTODESK . Este erro é da biblioteca do SDK do FBX . Variáveis ​​confusas pSrc e pDst . Acho que, além do Lumberyard, também existem muitos outros usuários cujos projetos dependem do código com esse erro.

V501 Existem subexpressões idênticas à esquerda e à direita do 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); } } .... } 

Voltar para o código Lumberyard. Na condição, o mesmo ponteiro pTS-> pRT_ALD_1 é verificado . Um deles deveria ter sido pTS-> pRT_RGB_1 . Talvez, mesmo após a explicação, não seja imediatamente possível ver a diferença, mas existe uma: a diferença está nos substratos curtos ALD e RGB . Se você for informado de que a Revisão de código manual é suficiente, mostre este exemplo.

E se este exemplo não for suficiente, existem mais 5 similares.

• V501 Existem subexpressões idênticas à esquerda e à direita do '||' operador :! pTS-> pRT_ALD_0 ||! pTS-> pRT_ALD_0 d3d_svo.cpp 1041
• V501 Existem subexpressões idênticas à esquerda e à direita do operador '&&': m_pRT_AIR_MIN && m_pRT_AIR_MIN d3d_svo.cpp 1808
• V501 Existem subexpressões idênticas à esquerda e à direita do operador '&&': m_pRT_AIR_MAX && m_pRT_AIR_MAX d3d_svo.cpp 1819
• V501 Existem subexpressões idênticas à esquerda e à direita do operador '&&': m_pRT_AIR_SHAD && m_pRT_AIR_SHAD d3d_svo.cpp 1830
• V501 Existem subexpressões idênticas à esquerda e à direita do operador '&&': s_pPropertiesPanel && s_pPropertiesPanel entityobject.cpp 1700

E como prometi, aqui está uma lista dos avisos restantes da V501 sem exemplos de código:

Sobre a posição da câmera nos jogos

Há o segundo diagnóstico mais rápido no PVS-Studio - V502 . Esse diagnóstico é mais antigo que algumas novas linguagens de programação, nas quais esse erro não pode mais ser cometido. E para C ++, esse erro será relevante, talvez sempre.

Vamos começar com um pequeno exemplo simples.

V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '+'. zipencryptor.cpp 217

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

A operação de adição tem uma prioridade mais alta que o operador ternário. Portanto, essa expressão tem uma lógica de cálculo completamente diferente da assumida pelo autor.

Você pode corrigir o erro desta maneira:

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

V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o 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); } 

E aqui está um código de exemplo onde eles trabalham com a posição da câmera. Um exemplo é difícil de perceber com os olhos e há um erro nele. Para publicação, a formatação do código foi alterada, mas no arquivo de origem esse código é ainda mais ilegível.

Existe um erro nesta substring:

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

Se a câmera do seu jogo de repente começar a se comportar de forma inadequada, você deve saber que os desenvolvedores salvaram na análise de código estático:

Outros exemplos com avisos semelhantes:

• V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '-'. scriptbind_ai.cpp 5203
• V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '+'. qcolumnwidget.cpp 136
• V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '&&'. shapetool.h 98

CryEngine Legacy

O Amazon Lumberyard é baseado no código CryEngine . E não em sua melhor versão. Eu tirei essa conclusão olhando o relatório do analisador. Alguns erros encontrados já foram corrigidos na versão mais recente do CryEngine para minhas duas revisões de código, mas ainda estão presentes no código Lumberyard. Além disso, no ano passado, o analisador foi significativamente aprimorado e foi possível encontrar erros adicionais que agora estão presentes em dois mecanismos de jogo. Mas com Lumberyard a situação é pior. A Amazon herdou essencialmente toda a dívida técnica da CryEngine. Mas seu dever técnico próprio, é claro, aparece em cada empresa por conta própria :).

Nesta seção, apresentarei alguns erros que foram corrigidos na versão mais recente do CryEngine e agora permanecem apenas um problema do projeto Lumberyard.

V519 A variável 'BlendFactor [2]' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1283, 1284. ccrydxgldevicecontext.cpp 1284

Aproximadamente essas emoções serão experimentadas pelos desenvolvedores do Lumberyard quando descobrirem que esse erro permaneceu apenas com eles.

A propósito, existem mais dois:

• V519 A variável 'm_auBlendFactor [2]' recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 919, 920. ccrydxgldevicecontext.cpp 920
• V519 A variável 'm_auBlendFactor [2]' recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 926, 927. ccrydxgldevicecontext.cpp 927

Existe um erro:

V546 O membro de uma classe é inicializado por si só: 'eConfigMax (eConfigMax.VeryHigh)'. particleparams.h 1837

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

No CryEngine, essa classe geralmente era reescrita, mas aqui o erro de inicialização permanecia.

V521 Tais expressões usando o operador ',' são perigosas. Verifique se a expressão '! SWords [iWord] .empty (), iWord ++' está correta. 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); } .... } 

O ciclo suspeito que o CryEngine já reescreveu também.

Erros vivem mais do que você pensa

Para os usuários que começam a usar o PVS-Studio pela primeira vez, a situação é aproximadamente a mesma: eles encontram um erro, descobrem que foi adicionado há alguns meses e ficam felizes ao perceber que milagrosamente evitaram a manifestação do problema entre seus usuários. Muitos de nossos clientes passaram a usar regularmente o PVS-Studio precisamente após essas histórias.

Às vezes, para iniciar os processos de controle de qualidade de código, a empresa deve visitar essas situações várias vezes. Aqui está um exemplo sobre o CryEngine e o Lumberyard:

É possível saturação da matriz V557 CWE-119. O índice 'id' está apontando além do limite da matriz. gameobjectsystem.cpp 113

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

Como você sabe, o Amazon Lumberyard não se baseia na versão mais recente do CryEngine. No entanto, com a ajuda do analisador PVS-Studio, foi possível encontrar um erro que está presente agora em dois mecanismos de jogo. Foi necessário verificar o índice usando o operador '> =' ...

O erro de indexação é sério. Além disso, existem seis lugares assim ! Aqui está outro exemplo:

É possível saturação da matriz V557 CWE-119. O índice 'index' está apontando além do limite da matriz. vehicleseatgroup.cpp 73

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

Alguém cometeu os erros do mesmo tipo, e eles não foram corrigidos apenas porque não haviam sido incluídos nas análises de erros do CryEngine.

Avisos restantes:

• É possível saturação da matriz V557 CWE-119. O índice 'id' está apontando além do limite da matriz. gameobjectsystem.cpp 195
• É possível saturação da matriz V557 CWE-119. O índice 'id' está apontando além do limite da matriz. gameobjectsystem.cpp 290
• É possível saturação da matriz V557 CWE-119. O índice 'stateId' está apontando além do limite da matriz. vehicleanimation.cpp 311
• É possível saturação da matriz V557 CWE-119. O índice 'stateId' está apontando além do limite da matriz. vehicleanimation.cpp 354

A longa existência de erros no código pode ser explicada apenas pelo nível correspondente de teste do projeto. Acredita-se que a análise estática encontre erros apenas no código não utilizado. Então, isso não é verdade. Os desenvolvedores esquecem que a maioria dos usuários sofre silenciosamente de bugs não óbvios nos programas, e a manifestação desses bugs muito raros geralmente afeta adversamente o trabalho de toda a empresa, a reputação e suas vendas, se houver.

Diferentes tons de programação de copiar e colar

Quando você chegou a esta seção do artigo, provavelmente percebeu que a programação de copiar e colar é a causa de muitos problemas. No PVS-Studio, a busca por esses erros é implementada em vários diagnósticos. Esta seção fornecerá exemplos de copiar e colar encontrados no V561 .

Aqui está um exemplo de código suspeito ao declarar variáveis ​​com o mesmo nome em escopos sobrepostos.

V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'pLibrary' do que declará-lo novamente. Declaração anterior: entityobject.cpp, linha 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; } .... } 

O ponteiro pLibrary não substitui conforme o esperado. A inicialização desse ponteiro foi completamente copiada sob a condição junto com a declaração de tipo.

Vou listar todos os lugares semelhantes:

• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'eType' do que declará-la novamente. Declaração anterior: toglsloperand.c, linha 838. toglsloperand.c 1224
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'eType' do que declará-la novamente. Declaração anterior: toglsloperand.c, linha 838. toglsloperand.c 1305
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'rSkelPose' do que declará-lo novamente. Declaração anterior: attachmentmanager.cpp, linha 409. attachmentmanager.cpp 458
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'nThreadID' do que declará-la novamente. Declaração anterior: d3dmeshbaker.cpp, linha 797. d3dmeshbaker.cpp 867
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'directoryNameList' do que declará-lo novamente. Declaração anterior: assetimportermanager.cpp, linha 720. assetimportermanager.cpp 728
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'pNode' do que declará-la novamente. Declaração anterior: breakpointsctrl.cpp, linha 340. breakpointsctrl.cpp 349
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'pLibrary' do que declará-lo novamente. Declaração anterior: prefabobject.cpp, linha 1443. prefabobject.cpp 1446
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'pLibrary' do que declará-lo novamente. Declaração anterior: prefabobject.cpp, linha 1470. prefabobject.cpp 1473
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'cmdLine' do que declará-lo novamente. Declaração anterior: fileutil.cpp, linha 110. fileutil.cpp 130
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'sfunctionArgs' do que declará-lo novamente. Declaração anterior: attributeitemlogiccallbacks.cpp, linha 291. attributeitemlogiccallbacks.cpp 303
• V561 CWE-563 Provavelmente é melhor atribuir valor à variável 'curveName' do que declará-la novamente. Declaração anterior: qgradientselectorwidget.cpp, linha 475. qgradientselectorwidget.cpp 488

Uma lista longa ... alguns dos lugares listados são até cópias completas do exemplo descrito.

Inicialização de autovalor

No código do mecanismo de jogo, muitos locais foram encontrados onde a variável é atribuída a si mesma. Em algum lugar em que esse código permaneceu para depuração, em algum lugar o código foi simplesmente projetado com muito bom gosto (geralmente também é uma fonte de erros); portanto, darei a você um pedaço de código que é mais suspeito para mim.

V570 A variável 'behaviorParams.ignoreOnVehicleDestroyed' é atribuída a si mesma. vehiclecomponent.cpp 168

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

Na visão atual, a ramificação else não é necessária. Mas talvez esse trecho de código contenha um erro: eles queriam atribuir o valor oposto à variável:

 bValue = !bValue 

Mas é melhor que os desenvolvedores se familiarizem com os resultados desse diagnóstico.

Manipulação de Problemas

Esta seção dará muitos exemplos quando algo deu errado durante o tratamento de erros.

Exemplo 1

V606 token sem proprietário '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; } } 

Esqueceu-se de escrever return nullptr; . Agora, o valor inválido da variável resultante será usado em outra parte do código.

O mesmo código exato foi copiado para mais um local:

• V606 token sem proprietário 'nullptr'. dx12rootsignature.cpp 621

Exemplo 2

V606 Token sem dono 'false'. 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 .... } 

Um exemplo muito interessante de um erro com uma declaração de retorno ausente. Agora é possível acessar o índice de um contêiner vazio.

Exemplo 3

V564 CWE-480 O operador '&' é aplicado ao valor do tipo bool. Você provavelmente esqueceu de incluir parênteses ou pretende usar o 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])); } .... } 

Verificado incorretamente a presença de bits no sinalizador. O operador de negação se aplica ao valor do sinalizador, não à expressão inteira. Escreva corretamente assim:

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

Mais avisos semelhantes:

• V564 CWE-480 O '|' O operador é aplicado ao valor do tipo bool. Você provavelmente esqueceu de incluir parênteses ou pretende usar o '||' operador. d3dhwshader.cpp 1832
• V564 CWE-480 O operador '&' é aplicado ao valor do tipo bool. Você provavelmente esqueceu de incluir parênteses ou pretende usar o operador '&&'. trackviewdialog.cpp 2112
• V564 CWE-480 O '|' O operador é aplicado ao valor do tipo bool. Você provavelmente esqueceu de incluir parênteses ou pretende usar o '||' operador. imagecompiler.cpp 1039

Exemplo 4

V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: 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."); } .... } 

Erro ao lançar exceção. Era necessário escrever assim:

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

A lista completa de tais erros:

• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1515
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1521
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1543
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1549
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1603
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1619
• V596 CWE-390 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: throw runtime_error (FOO); prefabobject.cpp 1644

Alguns problemas ao trabalhar com memória

V549 CWE-688 O primeiro argumento da função 'memcmp' é igual ao 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()); } .... } .... }; 

A condição compara os tamanhos de dois vetores. Se forem iguais, no ramo else os valores dos primeiros elementos dos vetores serão comparados usando a função memcmp () . Mas o primeiro e o segundo argumentos para essa função são os mesmos! O acesso aos elementos da matriz é bastante complicado. Existem índices a e b . Muito provavelmente, um erro de digitação está neles.

V611 CWE-762 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] data;'. 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]; } .... } 

A memória do ponteiro de dados está sendo liberada com uma instrução inválida. O operador delete [] deve ser usado em qualquer lugar .

Código inacessível

V779 CWE-561 Código inacessível detectado. É possível que haja um erro. 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; } 

Todos os ramos da instrução condicional terminam com a saída da função. No entanto, algum código não é executado.

V779 CWE-561 Código inacessível detectado. É possível que haja um erro. 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 } 

É fácil detectar um erro neste pedaço de código. Mas se você escrever o código por um longo período de tempo, a atenção cai drasticamente e esses erros caem facilmente no projeto.

V622 CWE-478 Considere inspecionar a instrução 'switch'. É possível que o primeiro operador 'case' esteja ausente. 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; .... } 

Se o switch contiver código fora da instrução case / default , ele nunca será executado.

Conclusão

O artigo inclui 95 avisos do analisador, 25 deles com exemplos de código. Quanto material existe em todo o relatório do analisador? Eu rapidamente rolei apenas um terço dos alertas de alto nível . Há também Médio e Baixo, um grupo de diagnósticos para busca de otimizações e outras oportunidades inexploradas do analisador - esses são centenas de erros mais óbvios e milhares de avisos inexplorados.

E então o leitor precisa se perguntar: “É possível, com essa abordagem do projeto, lançar um bom mecanismo de jogo?”. Não há controle de qualidade do código. O código CryEngine com erros antigos foi usado como base, novos erros foram adicionados. O próprio CryEngine é finalizado somente após a próxima revisão do código. A Amazon tem todas as chances, com seus recursos, de trabalhar na direção da qualidade do código e liberar o melhor mecanismo de jogo!

Não fique muito chateado. Entre os clientes do PVS-Studio, existem mais de trinta outras empresas envolvidas em jogos. Você pode se familiarizar com eles e seus produtos na página do nosso site " Clientes " selecionando o filtro "Desenvolvimento de jogos". Então, estamos gradualmente melhorando o mundo. Talvez possamos melhorar o Amazon Lumberyard :).

Um colega escreveu recentemente um artigo sobre o tema da qualidade do software para jogos. Sugiro que você esteja interessado em ler: " Análise estática na indústria de videogames: os 10 principais erros de software ".

Link para baixar o analisador PVS-Studio , como você pode ficar sem ele ;-)

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Svyatoslav Razmyslov. Amazon Lumberyard: um grito de angústia