Localizando erros no LLVM 8 com PVS-Studio

Faz dois anos desde a última vez que verificamos o código do projeto LLVM com o PVS-Studio, então vamos ver se o PVS-Studio ainda é o líder entre as ferramentas para detectar bugs e pontos fracos na segurança. Faremos isso examinando a versão LLVM 8.0.0 em busca de novos bugs.

O artigo que deve ser escrito

Francamente, não estava com vontade de escrever este artigo. Não é muito divertido falar sobre o projeto que já verificamos mais de uma vez ( 1 , 2 , 3 ). Prefiro algo novo, mas não tenho escolha.

Sempre que uma nova versão do LLVM é lançada ou o Clang Static Analyzer é atualizado, recebemos e-mails com as seguintes linhas:

Ei, a nova versão do Clang Static Analyzer recebeu novos diagnósticos! O PVS-Studio parece estar ficando menos relevante. O Clang pode detectar mais erros do que antes e agora está atualizando o PVS-Studio. O que você disse?

A isso eu responderia com prazer:

Também não andamos à toa! Aumentamos significativamente os recursos do PVS-Studio, então não se preocupe - ainda somos os melhores.

Mas essa é uma resposta ruim, receio. Ele não oferece provas, e é por isso que estou escrevendo este artigo. Então, verifiquei o LLVM mais uma vez e encontrei toneladas de bugs de todos os tipos. Aqueles que eu mais gostei serão discutidos mais adiante. O Clang Static Analyzer não pode detectar esses bugs (ou torna o processo muito problemático) - e nós podemos. E, a propósito, levei apenas uma noite para escrever todos esses insetos.

O artigo, no entanto, levou várias semanas para ser concluído. Eu simplesmente não conseguia colocar o material reunido em texto :).

A propósito, se você se perguntar que técnicas o PVS-Studio emprega para detectar erros e vulnerabilidades, dê uma olhada nesta publicação .

Diagnósticos novos e existentes

Como eu já disse, a última das muitas verificações do LLVM foi feita há dois anos e os bugs encontrados foram corrigidos pelos autores. Este artigo mostrará uma nova parte dos erros. Como é que existem novos bugs? Existem três razões:

1. O projeto LLVM está evoluindo; os autores modificam o código existente e adicionam novo código. As peças modificadas e as novas têm naturalmente novos bugs. Esse fato é um forte argumento para executar análises estáticas regularmente, e não de vez em quando. O formato dos nossos artigos é perfeito para mostrar os recursos do PVS-Studio, mas não tem nada a ver com a melhoria da qualidade do código ou com a redução de custos da correção de erros. Use análises estáticas regularmente!
2. Modificamos e aprimoramos os diagnósticos existentes, permitindo que o analisador detecte erros que antes não era possível detectar.
3. O PVS-Studio foi aprimorado com novos diagnósticos, que não existiam há dois anos. Agrupei esses avisos em uma seção separada para que o progresso do PVS-Studio seja visto de maneira mais distinta.

Defeitos encontrados pelos diagnósticos existentes

Snippet no. 1: Copiar e colar

static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) { if (Name == "addcarryx.u32" || // Added in 8.0 .... Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0 Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0 Name == "avx512.cvtusi2sd" || // Added in 7.0 Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <= Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <= Name == "sse2.pmulu.dq" || // Added in 7.0 Name == "sse41.pmuldq" || // Added in 7.0 Name == "avx2.pmulu.dq" || // Added in 7.0 .... } 

Mensagem de diagnóstico do PVS-Studio: V501 [CWE-570] Existem sub-expressões idênticas 'Name.startwith ("avx512.mask.permvar.")' À esquerda e à direita da '||' operador. AutoUpgrade.cpp 73

A ocorrência do "avx512.mask.permvar." substring é verificado duas vezes. A segunda condição obviamente era verificar outra coisa, mas o programador esqueceu de alterar a linha copiada.

Snippet no. 2: erro de digitação

 enum CXNameRefFlags { CXNameRange_WantQualifier = 0x1, CXNameRange_WantTemplateArgs = 0x2, CXNameRange_WantSinglePiece = 0x4 }; void AnnotateTokensWorker::HandlePostPonedChildCursor( CXCursor Cursor, unsigned StartTokenIndex) { const auto flags = CXNameRange_WantQualifier | CXNameRange_WantQualifier; .... } 

Mensagem de diagnóstico do PVS-Studio: V501 Existem subexpressões idênticas 'CXNameRange_WantQualifier' à esquerda e à direita da '|' operador. CIndex.cpp 7245

A constante nomeada CXNameRange_WantQualifier é usada duas vezes devido a um erro de digitação.

Snippet no. 3: Confusão sobre a precedência do operador

 int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) { .... if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0) return 0; .... } 

Mensagem de diagnóstico do PVS-Studio: V502 [CWE-783] Talvez o operador '?:' Funcione de uma maneira diferente da esperada. O operador '?:' Tem uma prioridade mais baixa que o operador '=='. PPCTargetTransformInfo.cpp 404

Eu acho esse bug muito fofo. Sim, eu sei que tenho um gosto estranho :).

Conforme determinado pela precedência do operador , a expressão original é avaliada da seguinte maneira:

 (ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0 

Do ponto de vista prático, porém, essa condição não faz sentido, pois pode ser reduzida para:

 (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian()) 

Obviamente, isso é um bug. Deve ter sido a variável Index que o programador quis verificar quanto a 0/1. Para corrigir o código, o operador ternário deve estar entre parênteses:

 if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0)) 

O operador ternário é realmente muito complicado e pode levar a erros de lógica. Use-o com cuidado e não hesite em colocar parênteses adicionais em torno dele. Este assunto é discutido em mais detalhes aqui , na seção "Cuidado com o operador ?: e coloque-o entre parênteses".

Fragmentos não. 4, 5: Ponteiro nulo

 Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) { .... TypedInit *LHS = dyn_cast<TypedInit>(Result); .... LHS = dyn_cast<TypedInit>( UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get()) ->Fold(CurRec)); if (!LHS) { Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() + "' to string"); return nullptr; } .... } 

Mensagem de diagnóstico do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'LHS' pode ocorrer. TGParser.cpp 2152

Se o ponteiro LHS for nulo, espera-se que o programa gere um aviso. Em vez disso, desreferirá esse ponteiro muito nulo: LHS-> getAsString () .

É uma situação bastante comum para os manipuladores de erros conterem erros, porque os desenvolvedores não os testam corretamente. Os analisadores estáticos verificam todo o código acessível, independentemente da frequência com que ele é realmente executado. Este é um bom exemplo de como a análise estática complementa outros meios de teste e proteção de código.

Um manipulador com falha semelhante para o ponteiro RHS é encontrado um pouco mais: V522 [CWE-476] A desreferenciação do ponteiro nulo 'RHS' pode ocorrer. TGParser.cpp 2186

Snippet no. 6: Usando um ponteiro após uma movimentação

 static Expected<bool> ExtractBlocks(....) { .... std::unique_ptr<Module> ProgClone = CloneModule(BD.getProgram(), VMap); .... BD.setNewProgram(std::move(ProgClone)); // <= MiscompiledFunctions.clear(); for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) { Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first); // <= assert(NewF && "Function not found??"); MiscompiledFunctions.push_back(NewF); } .... } 

Mensagem de diagnóstico do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'ProgClone' pode ocorrer. Miscompilation.cpp 601

O ponteiro inteligente ProgClone primeiro libera a propriedade do objeto:

 BD.setNewProgram(std::move(ProgClone)); 

De fato, ProgClone se tornou um ponteiro nulo - portanto, tecnicamente, um ponteiro nulo é desreferenciado um pouco mais:

 Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first); 

Mas isso não vai acontecer! Observe que o loop não é realmente executado.

O contêiner MiscompiledFunctions é limpo primeiro:

 MiscompiledFunctions.clear(); 

E então seu tamanho é usado na condição de loop:

 for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) { 

Obviamente, o loop simplesmente não inicia. Eu acho que é um bug também, e o código foi criado para ter uma aparência diferente.

Eu acho que o que vemos aqui é essa notória paridade de erros, em que um bug atua como disfarce para outro :).

Snippet no. 7: Usando um ponteiro após uma movimentação

 static Expected<bool> TestOptimizer(BugDriver &BD, std::unique_ptr<Module> Test, std::unique_ptr<Module> Safe) { outs() << " Optimizing functions being tested: "; std::unique_ptr<Module> Optimized = BD.runPassesOn(Test.get(), BD.getPassesToRun()); if (!Optimized) { errs() << " Error running this sequence of passes" << " on the input program!\n"; BD.setNewProgram(std::move(Test)); // <= BD.EmitProgressBitcode(*Test, "pass-error", false); // <= if (Error E = BD.debugOptimizerCrash()) return std::move(E); return false; } .... } 

Mensagem de diagnóstico do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Teste' pode ocorrer. Miscompilation.cpp 709

Este é semelhante ao caso anterior. O conteúdo do objeto é movido primeiro e depois usado como se nada tivesse acontecido. Esse erro tem se tornado cada vez mais comum depois que a semântica de movimentação foi adicionada ao C ++. É disso que eu gosto nessa linguagem! Você recebe novas maneiras de se dar um tiro no pé, o que significa que o PVS-Studio sempre terá trabalho a fazer :).

Snippet no. 8: Ponteiro nulo

 void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) { uint32_t TypeId = Symbol.getTypeId(); auto Type = Symbol.getSession().getSymbolById(TypeId); if (Type) Printer << "<unknown-type>"; else Type->dump(*this); } 

Mensagem de diagnóstico do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Tipo' pode ocorrer. PrettyFunctionDumper.cpp 233

Assim como os manipuladores de erro, as funções de teste que imprimem dados de depuração também não obtêm cobertura de teste adequada, e este é um exemplo disso. Em vez de ajudar o usuário a resolver seus problemas, a função está esperando que ele seja corrigido.

Código fixo:

 if (Type) Type->dump(*this); else Printer << "<unknown-type>"; 

Snippet no. 9: Ponteiro nulo

 void SearchableTableEmitter::collectTableEntries( GenericTable &Table, const std::vector<Record *> &Items) { .... RecTy *Ty = resolveTypes(Field.RecType, TI->getType()); if (!Ty) // <= PrintFatalError(Twine("Field '") + Field.Name + "' of table '" + Table.Name + "' has incompatible type: " + Ty->getAsString() + " vs. " + // <= TI->getType()->getAsString()); .... } 

Mensagem de diagnóstico do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Ty' pode ocorrer. SearchableTableEmitter.cpp 614

Eu não acho que você precise de comentários sobre este.

Snippet no. 10: erro de digitação

 bool FormatTokenLexer::tryMergeCSharpNullConditionals() { .... auto &Identifier = *(Tokens.end() - 2); auto &Question = *(Tokens.end() - 1); .... Identifier->ColumnWidth += Question->ColumnWidth; Identifier->Type = Identifier->Type; // <= Tokens.erase(Tokens.end() - 1); return true; } 

Mensagem de diagnóstico do PVS-Studio: V570 A variável 'Identifier-> Type' é atribuída a si mesma. FormatTokenLexer.cpp 249

Atribuir uma variável a si mesma é uma operação sem sentido. O programador deve ter feito o seguinte:

 Identifier->Type = Question->Type; 

Snippet no. 11: Intervalo suspeito

 void SystemZOperand::print(raw_ostream &OS) const { switch (Kind) { break; case KindToken: OS << "Token:" << getToken(); break; case KindReg: OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg()); break; .... } 

Mensagem de diagnóstico do PVS-Studio: V622 [CWE-478] Considere inspecionar a instrução 'switch'. É possível que o primeiro operador 'case' esteja ausente. SystemZAsmParser.cpp 652

Há uma declaração de interrupção muito suspeita no início. Não deveria haver algo mais aqui?

Snippet no. 12: Verificando um ponteiro após desreferenciação

 InlineCost AMDGPUInliner::getInlineCost(CallSite CS) { Function *Callee = CS.getCalledFunction(); Function *Caller = CS.getCaller(); TargetTransformInfo &TTI = TTIWP->getTTI(*Callee); if (!Callee || Callee->isDeclaration()) return llvm::InlineCost::getNever("undefined callee"); .... } 

Mensagem de diagnóstico do PVS-Studio: V595 [CWE-476] O ponteiro 'Callee' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 172, 174. AMDGPUInline.cpp 172

O ponteiro Callee é primeiro desreferenciado quando a função getTTI é chamada.

E então acontece que o ponteiro deve ser verificado quanto a nullptr :

 if (!Callee || Callee->isDeclaration()) 

Tarde demais ...

Fragmentos não. 13 - Não ....: Verificando um ponteiro após desreferenciação

O exemplo anterior não é exclusivo. O mesmo problema é encontrado neste trecho:

 static Value *optimizeDoubleFP(CallInst *CI, IRBuilder<> &B, bool isBinary, bool isPrecise = false) { .... Function *CalleeFn = CI->getCalledFunction(); StringRef CalleeNm = CalleeFn->getName(); // <= AttributeList CalleeAt = CalleeFn->getAttributes(); if (CalleeFn && !CalleeFn->isIntrinsic()) { // <= .... } 

Mensagem de diagnóstico do PVS-Studio: V595 [CWE-476] O ponteiro 'CalleeFn' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 1079, 1081. SimplifyLibCalls.cpp 1079

E este:

 void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, const Decl *Tmpl, Decl *New, LateInstantiatedAttrVec *LateAttrs, LocalInstantiationScope *OuterMostScope) { .... NamedDecl *ND = dyn_cast<NamedDecl>(New); CXXRecordDecl *ThisContext = dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <= CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(), ND && ND->isCXXInstanceMember()); // <= .... } 

Mensagem de diagnóstico do PVS-Studio: V595 [CWE-476] O ponteiro 'ND' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 532, 534. SemaTemplateInstantiateDecl.cpp 532

E aqui:

• V595 [CWE-476] O ponteiro 'U' foi utilizado antes de ser verificado com relação ao nullptr. Verifique as linhas: 404, 407. DWARFFormValue.cpp 404
• V595 [CWE-476] O ponteiro 'ND' foi utilizado antes de ser verificado com relação ao nullptr. Verifique as linhas: 2149, 2151. SemaTemplateInstantiate.cpp 2149

Então, perdi o interesse em rastrear avisos do V595, portanto, não posso dizer se há outros erros desse tipo além dos mostrados acima. Eu aposto que existem.

Fragmentos não. 17, 18: mudança suspeita

 static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize, uint64_t &Encoding) { .... unsigned Size = RegSize; .... uint64_t NImms = ~(Size-1) << 1; .... } 

Mensagem de diagnóstico do PVS-Studio: V629 [CWE-190] Considere inspecionar a expressão '~ (Tamanho - 1) << 1'. Mudança de bit do valor de 32 bits com uma expansão subsequente para o tipo de 64 bits. AArch64AddressingModes.h 260

Na verdade, esse código pode estar correto, mas parece estranho e precisa ser examinado.

Suponha que a variável Size tenha o valor 16; espera-se que a variável NImms obtenha o seguinte valor:

111111111111111111111111111111111111111111111111111111111111100000000

Mas, na realidade, obterá o valor:

0000000000000000000000000000000000001111111111111111111111111111100000

Isso acontece porque todos os cálculos são feitos no tipo não assinado de 32 bits e somente então é implicitamente promovido para uint64_t , com os bits mais significativos zerados.

O problema pode ser corrigido da seguinte maneira:

 uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1; 

Outro bug desse tipo: V629 [CWE-190] Considere inspecionar a expressão 'Immr << 6'. Mudança de bit do valor de 32 bits com uma expansão subsequente para o tipo de 64 bits. AArch64AddressingModes.h 269

Snippet no. 19: Falta outra palavra-chave ?

 void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) { .... if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) { // VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token. // Skip it. continue; } if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <= Op.addRegWithFPInputModsOperands(Inst, 2); } else if (Op.isDPPCtrl()) { Op.addImmOperands(Inst, 1); } else if (Op.isImm()) { // Handle optional arguments OptionalIdx[Op.getImmTy()] = I; } else { llvm_unreachable("Invalid operand type"); } .... } 

Mensagem de diagnóstico do PVS-Studio: V646 [CWE-670] Considere inspecionar a lógica do aplicativo. É possível que a palavra-chave 'else' esteja ausente. AMDGPUAsmParser.cpp 5655

Este não é um bug. Como o bloco then da primeira instrução if termina com continue , não importa se ela possui a palavra-chave else ou não. O comportamento será o mesmo em qualquer caso. No entanto, a falta de mais torna o código menos legível e, portanto, potencialmente perigoso. Se continuar desaparecer um dia, o comportamento mudará drasticamente. Eu recomendo fortemente adicionar mais .

Snippet no. 20: Quatro erros de digitação idênticos

 LLVM_DUMP_METHOD void Symbol::dump(raw_ostream &OS) const { std::string Result; if (isUndefined()) Result += "(undef) "; if (isWeakDefined()) Result += "(weak-def) "; if (isWeakReferenced()) Result += "(weak-ref) "; if (isThreadLocalValue()) Result += "(tlv) "; switch (Kind) { case SymbolKind::GlobalSymbol: Result + Name.str(); // <= break; case SymbolKind::ObjectiveCClass: Result + "(ObjC Class) " + Name.str(); // <= break; case SymbolKind::ObjectiveCClassEHType: Result + "(ObjC Class EH) " + Name.str(); // <= break; case SymbolKind::ObjectiveCInstanceVariable: Result + "(ObjC IVar) " + Name.str(); // <= break; } OS << Result; } 

Mensagens de diagnóstico do PVS-Studio:

• V655 [CWE-480] As cadeias foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Resultado + Nome.str ()'. Symbol.cpp 32
• V655 [CWE-480] As cadeias foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Resultado + "(Classe ObjC)" + Nome.str ()'. Symbol.cpp 35
• V655 [CWE-480] As cadeias foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Result + "(ObjC Class EH)" + Name.str ()'. Symbol.cpp 38
• V655 [CWE-480] As cadeias foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Resultado + "(ObjC IVar)" + Name.str ()'. Symbol.cpp 41

O programador acidentalmente usou o operador + em vez de + = e terminou com quatro construções sem sentido.

Snippet no. 21: Comportamento indefinido

 static void getReqFeatures(std::map<StringRef, int> &FeaturesMap, const std::vector<Record *> &ReqFeatures) { for (auto &R : ReqFeatures) { StringRef AsmCondString = R->getValueAsString("AssemblerCondString"); SmallVector<StringRef, 4> Ops; SplitString(AsmCondString, Ops, ","); assert(!Ops.empty() && "AssemblerCondString cannot be empty"); for (auto &Op : Ops) { assert(!Op.empty() && "Empty operator"); if (FeaturesMap.find(Op) == FeaturesMap.end()) FeaturesMap[Op] = FeaturesMap.size(); } } } 

Tente identificar o bug por conta própria primeiro. Adicionei a imagem para que você não espreite a resposta imediatamente:

Mensagem de diagnóstico do PVS-Studio: V708 [CWE-758] Construção perigosa é usada: 'FeaturesMap [Op] = FeaturesMap.size ()', onde 'FeaturesMap' é da classe 'map'. Isso pode levar a um comportamento indefinido. RISCVCompressInstEmitter.cpp 490

A linha defeituosa é esta:

 FeaturesMap[Op] = FeaturesMap.size(); 

Se o elemento Op não foi encontrado, o programa cria um novo elemento no mapa e atribui a ele o número total de elementos nesse mapa. Você simplesmente não sabe se a função size será chamada antes ou depois de adicionar o novo elemento.

Fragmentos não. 22 - Não. 24: atribuições duplicadas

 Error MachOObjectFile::checkSymbolTable() const { .... } else { MachO::nlist STE = getSymbolTableEntry(SymDRI); NType = STE.n_type; // <= NType = STE.n_type; // <= NSect = STE.n_sect; NDesc = STE.n_desc; NStrx = STE.n_strx; NValue = STE.n_value; } .... } 

Mensagem de diagnóstico do PVS-Studio: V519 [CWE-563] A variável 'NType' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1663, 1664. MachOObjectFile.cpp 1664

Não acho que seja um erro verdadeiro - é uma atribuição duplicada. Mas ainda é um defeito.

Dois outros casos:

• V519 [CWE-563] A variável 'B.NDesc' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1488, 1489. llvm-nm.cpp 1489
• V519 [CWE-563] A variável recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 59, 61. coff2yaml.cpp 61

Fragmentos não. 25 - Não. 27: Mais tarefas duplicadas

Estes tratam de versões ligeiramente diferentes de atribuições duplicadas.

 bool Vectorizer::vectorizeLoadChain( ArrayRef<Instruction *> Chain, SmallPtrSet<Instruction *, 16> *InstructionsProcessed) { .... unsigned Alignment = getAlignment(L0); .... unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(), StackAdjustedAlignment, DL, L0, nullptr, &DT); if (NewAlign != 0) Alignment = NewAlign; Alignment = NewAlign; .... } 

Mensagem de diagnóstico do PVS-Studio: V519 [CWE-563] A variável 'Alinhamento' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1158, 1160. LoadStoreVectorizer.cpp 1160

Este é um trecho muito estranho e provavelmente contém um erro lógico. A variável Alignment recebe primeiro o valor com base na condição e, em seguida, recebe o valor mais uma vez, mas sem nenhuma verificação prévia.

Defeitos semelhantes:

• V519 [CWE-563] A variável 'Effects' recebe valores atribuídos duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 152, 165. WebAssemblyRegStackify.cpp 165
• V519 [CWE-563] A variável 'ExpectNoDerefChunk' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 4970, 4973. SemaType.cpp 4973

Snippet no. 28: Sempre verdadeira condição

 static int readPrefixes(struct InternalInstruction* insn) { .... uint8_t byte = 0; uint8_t nextByte; .... if (byte == 0xf3 && (nextByte == 0x88 || nextByte == 0x89 || nextByte == 0xc6 || nextByte == 0xc7)) { insn->xAcquireRelease = true; if (nextByte != 0x90) // PAUSE instruction support // <= break; } .... } 

Mensagem de diagnóstico do PVS-Studio: V547 [CWE-571] A expressão 'nextByte! = 0x90' é sempre verdadeira. X86DisassemblerDecoder.cpp 379

A verificação não faz sentido. A variável nextByte nunca é igual a 0x90 : apenas segue logicamente da verificação anterior. Isso deve ser algum erro lógico.

Fragmentos não. 29 - Não ....: sempre verdadeiras / falsas condições

Existem muitos avisos de que uma condição inteira ( V547 ) ou parte de uma condição ( V560 ) é sempre verdadeira ou falsa. Em vez de erros genuínos, geralmente são apenas códigos incorretos, os efeitos da expansão de macro e assim por diante. Dito isto, todos esses avisos ainda devem ser verificados, pois alguns deles podem apontar para erros de lógica genuínos. Por exemplo, o seguinte trecho não parece correto:

 static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo, uint64_t Address, const void *Decoder) { DecodeStatus S = MCDisassembler::Success; if (RegNo > 13) return MCDisassembler::Fail; if ((RegNo & 1) || RegNo == 0xe) S = MCDisassembler::SoftFail; .... } 

Mensagem de diagnóstico do PVS-Studio: V560 [CWE-570] Uma parte da expressão condicional é sempre falsa: RegNo == 0xe. ARMDisassembler.cpp 939

A constante 0xE é o número decimal 14. A verificação RegNo == 0xe não faz sentido, porque se RegNo> 13 , a função retornará.

Vi muitos outros avisos do V547 e V560, mas, como no V595 , não me senti animado em verificá-los, pois já tinha material suficiente para um artigo :). Portanto, não há números para o número total de erros desse tipo no LLVM.

Aqui está um exemplo para ilustrar por que a verificação desses avisos é chata. O analisador está totalmente correto ao emitir um aviso no código a seguir. Mas ainda não é um bug.

 bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons, tok::TokenKind ClosingBraceKind) { bool HasError = false; .... HasError = true; if (!ContinueOnSemicolons) return !HasError; .... } 

Mensagem de diagnóstico do PVS-Studio: V547 [CWE-570] A expressão '! HasError' é sempre falsa. UnwrappedLineParser.cpp 1635

Snippet no. 30: Retorno suspeito

 static bool isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) { for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg), E = MRI.def_instr_end(); It != E; ++It) { return (*It).isImplicitDef(); } .... } 

Mensagem de diagnóstico do PVS-Studio: V612 [CWE-670] Um 'retorno' incondicional dentro de um loop. R600OptimizeVectorRegisters.cpp 63

É um bug ou uma técnica de codificação específica destinada a comunicar alguma idéia aos colegas programadores. Para mim, isso não diz nada, exceto que é um pedaço de código muito suspeito. Por favor, não escreva código assim :).

Sentindo-se cansado? OK, é hora de fazer um chá ou café.

Defeitos encontrados por novos diagnósticos

Eu acho que 30 exemplos são suficientes para o diagnóstico existente. Agora vamos ver se podemos encontrar algo interessante com os novos diagnósticos, que foram adicionados após a verificação anterior . Nos últimos dois anos, o módulo analisador C ++ foi estendido com 66 novos diagnósticos.

Snippet no. 31: Código inacessível

 Error CtorDtorRunner::run() { .... if (auto CtorDtorMap = ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names), NoDependenciesToRegister, true)) { .... return Error::success(); } else return CtorDtorMap.takeError(); CtorDtorsByPriority.clear(); return Error::success(); } 

Mensagem de diagnóstico do PVS-Studio: V779 [CWE-561] Código inacessível detectado. É possível que haja um erro. ExecutionUtils.cpp 146

Como você pode ver, as duas ramificações da instrução if terminam com uma declaração de retorno , o que significa que o contêiner CtorDtorsByPriority nunca será limpo.

Snippet no. 32: Código inacessível

 bool LLParser::ParseSummaryEntry() { .... switch (Lex.getKind()) { case lltok::kw_gv: return ParseGVEntry(SummaryID); case lltok::kw_module: return ParseModuleEntry(SummaryID); case lltok::kw_typeid: return ParseTypeIdEntry(SummaryID); // <= break; // <= default: return Error(Lex.getLoc(), "unexpected summary kind"); } Lex.setIgnoreColonInIdentifiers(false); // <= return false; } 

Mensagem de diagnóstico do PVS-Studio: V779 [CWE-561] Código inacessível detectado. É possível que haja um erro. LLParser.cpp 835

Este é interessante. Dê uma olhada nesta parte primeiro:

 return ParseTypeIdEntry(SummaryID); break; 

Parece não haver nada de estranho nesse código; a declaração de interrupção é desnecessária e pode ser removida com segurança. Mas não é assim tão simples.

O aviso é acionado pelas seguintes linhas:

 Lex.setIgnoreColonInIdentifiers(false); return false; 

Na verdade, esse código é inacessível. Todos os rótulos de maiúsculas e minúsculas da instrução switch terminam com um retorno , e a quebra solitária sem sentido não parece mais inofensiva! E se um dos galhos fosse destinado a terminar com uma pausa em vez de retornar ?

Snippet no. 33: Limpeza acidental dos bits mais significativos

 unsigned getStubAlignment() override { if (Arch == Triple::systemz) return 8; else return 1; } Expected<unsigned> RuntimeDyldImpl::emitSection(const ObjectFile &Obj, const SectionRef &Section, bool IsCode) { .... uint64_t DataSize = Section.getSize(); .... if (StubBufSize > 0) DataSize &= ~(getStubAlignment() - 1); .... } 

Mensagem de diagnóstico do PVS-Studio: V784 O tamanho da máscara de bits é menor que o tamanho do primeiro operando. Isso causará a perda de bits mais altos. RuntimeDyld.cpp 815

Observe que a função getStubAlignment retorna um valor não assinado . Vamos ver como a expressão será avaliada, assumindo que a função retornará o valor 8:

~ (getStubAlignment () - 1)

~ (8u-1)

0xFFFFFFF8u

Observe agora que o tipo da variável DataSize não está assinado de 64 bits. Portanto, executar a operação DataSize & 0xFFFFFFF8 resultará na limpeza de todos os 32 bits mais significativos do valor. Eu não acho que o programador quis isso. Talvez eles quisessem que fosse DataSize & 0xFFFFFFFFFFFFFFFFF8u.

Para corrigir o erro, o código deve ser reescrito da seguinte maneira:

 DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1); 

Ou assim:

 DataSize &= ~(getStubAlignment() - 1ULL); 

Snippet no. 34: Conversão de tipo explícito incorreta

 template <typename T> void scaleShuffleMask(int Scale, ArrayRef<T> Mask, SmallVectorImpl<T> &ScaledMask) { assert(0 < Scale && "Unexpected scaling factor"); int NumElts = Mask.size(); ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1); .... } 

Mensagem de diagnóstico do PVS-Studio: V1028 [CWE-190] Possível estouro. Considere converter operandos do operador 'NumElts * Scale' no tipo 'size_t', não no resultado. X86ISelLowering.h 1577

A conversão explícita de tipo é usada para evitar um estouro ao multiplicar variáveis ​​do tipo int . Nesse caso, porém, não funciona porque a multiplicação ocorrerá primeiro e somente então o resultado de 32 bits será promovido para o tamanho size_t .

Snippet no. 35: Copiar e colar incorreto

 Instruction *InstCombiner::visitFCmpInst(FCmpInst &I) { .... if (!match(Op0, m_PosZeroFP()) && isKnownNeverNaN(Op0, &TLI)) { I.setOperand(0, ConstantFP::getNullValue(Op0->getType())); return &I; } if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) { I.setOperand(1, ConstantFP::getNullValue(Op0->getType())); // <= return &I; } .... } 

V778 [CWE-682] Dois fragmentos de código semelhantes foram encontrados. Talvez seja um erro de digitação e a variável 'Op1' deva ser usada em vez de 'Op0'. InstCombineCompares.cpp 5507

Esse novo diagnóstico legal detecta situações em que um fragmento de código é gravado usando copiar e colar, com todos os nomes alterados, exceto um.

Observe que todos os Op0 , exceto um, foram alterados para Op1 no segundo bloco. O código provavelmente deve ficar assim:

 if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) { I.setOperand(1, ConstantFP::getNullValue(Op1->getType())); return &I; } 

Snippet no. 36: Variáveis ​​misturadas

 struct Status { unsigned Mask; unsigned Mode; Status() : Mask(0), Mode(0){}; Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { Mode &= Mask; }; .... }; 

Mensagem de diagnóstico do PVS-Studio: V1001 [CWE-563] A variável 'Mode' está atribuída, mas não é usada no final da função. SIModeRegister.cpp 48

É muito perigoso ter o mesmo nome para argumentos de função e para membros da classe, porque você corre o risco de confundi-los. O que você vê aqui é um exemplo disso. A seguinte expressão não tem sentido:

 Mode &= Mask; 

O argumento é alterado, mas nunca usado depois disso. Esse snippet provavelmente deve ficar assim:

 Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; }; 

Snippet no. 37: Variáveis ​​misturadas

 class SectionBase { .... uint64_t Size = 0; .... }; class SymbolTableSection : public SectionBase { .... }; void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type, SectionBase *DefinedIn, uint64_t Value, uint8_t Visibility, uint16_t Shndx, uint64_t Size) { .... Sym.Value = Value; Sym.Visibility = Visibility; Sym.Size = Size; Sym.Index = Symbols.size(); Symbols.emplace_back(llvm::make_unique<Symbol>(Sym)); Size += this->EntrySize; } 

Mensagem de diagnóstico do PVS-Studio: V1001 [CWE-563] A variável 'Size' está atribuída, mas não é usada no final da função. Object.cpp 424

Este é semelhante ao exemplo anterior. Versão correta:

 this->Size += this->EntrySize; 

Fragmentos não. 38 - Não. 47: Falta verificação do ponteiro

Vimos alguns exemplos do aviso do V595 um pouco antes. O que ele detecta é uma situação em que um ponteiro é primeiro desreferenciado e somente depois verificado. O novo diagnóstico V1004 é o oposto disso e também detecta toneladas de erros. Ele procura por ponteiros já testados que não são testados novamente quando necessário. Aqui estão alguns erros desse tipo encontrados no código do LLVM.

 int getGEPCost(Type *PointeeType, const Value *Ptr, ArrayRef<const Value *> Operands) { .... if (Ptr != nullptr) { // <= assert(....); BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts()); } bool HasBaseReg = (BaseGV == nullptr); auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <= .... } 

Mensagem de diagnóstico do PVS-Studio: V1004 [CWE-476] O ponteiro 'Ptr' foi usado sem segurança após ser verificado no nullptr. Verifique as linhas: 729, 738. TargetTransformInfoImpl.h 738

Ptr pode ser nullptr , o que é indicado pela verificação:

 if (Ptr != nullptr) 

No entanto, o mesmo ponteiro é desreferenciado sem essa verificação um pouco mais:

 auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); 

Outro caso semelhante.

 llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD, bool Stub) { .... auto *FD = dyn_cast<FunctionDecl>(GD.getDecl()); SmallVector<QualType, 16> ArgTypes; if (FD) // <= for (const ParmVarDecl *Parm : FD->parameters()) ArgTypes.push_back(Parm->getType()); CallingConv CC = FD->getType()->castAs<FunctionType>()->getCallConv(); // <= .... } 

Mensagem de diagnóstico do PVS-Studio: V1004 [CWE-476] O ponteiro 'FD' foi usado sem segurança após ser verificado no nullptr. Verifique as linhas: 3228, 3231. CGDebugInfo.cpp 3231

Observe o ponteiro FD . Este erro é direto, portanto, não há comentários sobre este.

Mais um aqui:

 static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result, Value *&BasePtr, const DataLayout &DL) { PointerType *PtrTy = dyn_cast<PointerType>(Ptr.getType()); if (!PtrTy) { // <= Result = Polynomial(); BasePtr = nullptr; } unsigned PointerBits = DL.getIndexSizeInBits(PtrTy->getPointerAddressSpace()); // <= .... } 

Mensagem de diagnóstico do PVS-Studio: V1004 [CWE-476] O ponteiro 'PtrTy' foi usado sem segurança após ser verificado no nullptr. Verifique as linhas: 960, 965. InterleavedLoadCombinePass.cpp 965

Como você evita erros como esse? Tenha muito cuidado ao revisar seu código e verifique-o regularmente com o PVS-Studio.

Acho que não devemos examinar outros exemplos desse tipo, então aqui está apenas uma lista dos avisos:

• V1004 [CWE-476] O ponteiro 'Expr' foi usado sem segurança após ser verificado contra nullptr. Verifique as linhas: 1049, 1078. DebugInfoMetadata.cpp 1078
• V1004 [CWE-476] O ponteiro 'PI' foi usado sem segurança após ser verificado contra nullptr. Verifique as linhas: 733, 753. LegacyPassManager.cpp 753
• V1004 [CWE-476] O ponteiro 'StatepointCall' foi usado sem segurança após ser verificado com relação ao nullptr. Verifique as linhas: 4371, 4379. Verifier.cpp 4379
• V1004 [CWE-476] O ponteiro 'RV' foi usado sem segurança após ser verificado contra nullptr. Verifique as linhas: 2263, 2268. TGParser.cpp 2268
• V1004 [CWE-476] O ponteiro 'CalleeFn' foi usado sem segurança após ser verificado contra nullptr. Verifique as linhas: 1081, 1096. SimplifyLibCalls.cpp 1096
• V1004 [CWE-476] O ponteiro 'TC' foi usado sem segurança após ser verificado contra nullptr. Verifique as linhas: 1819, 1824. Driver.cpp 1824

Fragmentos não. 48 - Não. 60: Não é crítico, mas ainda é um defeito (possível vazamento de memória)

 std::unique_ptr<IRMutator> createISelMutator() { .... std::vector<std::unique_ptr<IRMutationStrategy>> Strategies; Strategies.emplace_back( new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps())); .... } 

Mensagem de diagnóstico do PVS-Studio: V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Strategies' pelo método 'emplace_back'. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-isel-fuzzer.cpp 58

Você não pode simplesmente escrever xxx.push_back (novo X) para anexar um elemento a um contêiner do tipo std :: vector <std :: unique_ptr <X>> porque não há conversão implícita de X * para std :: unique_ptr < X> .

A solução popular é escrever xxx.emplace_back (novo X), uma vez que é compilável: o método emplace_back constrói o elemento diretamente a partir dos argumentos e, portanto, pode usar construtores explícitos.

Mas essa solução não é segura. Se o vetor estiver cheio, a memória será realocada. Esta operação pode falhar e acabar gerando uma exceção std :: bad_alloc . Nesse caso, o ponteiro será perdido e o programa não poderá excluir o objeto criado.

Uma solução mais segura é criar um unique_ptr , que reterá o ponteiro até que o vetor tente realocar a memória:

 xxx.push_back(std::unique_ptr<X>(new X)) 

O padrão C ++ 14 permite que você use 'std :: make_unique':

 xxx.push_back(std::make_unique<X>()) 

Esse tipo de defeito não tem efeito no LLVM. A compilação simplesmente terminará se a alocação de memória falhar. Dito isto, pode ser bastante crítico em aplicativos com um longo tempo de atividade , que não pode simplesmente terminar quando ocorre uma falha na alocação de memória.

Portanto, mesmo que esse código não seja perigoso para o LLVM, pensei em continuar falando sobre esse padrão de bug e o fato de o PVS-Studio agora poder detectá-lo.

Outros casos semelhantes:

• V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Passes' pelo método 'emplace_back'. Um vazamento de memória ocorrerá em caso de uma exceção. PassManager.h 546
• V1023 [CWE-460] A pointer without owner is added to the 'AAs' container by the 'emplace_back' method. A memory leak will occur in case of an exception. AliasAnalysis.h 324
• V1023 [CWE-460] A pointer without owner is added to the 'Entries' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DWARFDebugFrame.cpp 519
• V1023 [CWE-460] A pointer without owner is added to the 'AllEdges' container by the 'emplace_back' method. A memory leak will occur in case of an exception. CFGMST.h 268
• V1023 [CWE-460] A pointer without owner is added to the 'VMaps' container by the 'emplace_back' method. A memory leak will occur in case of an exception. SimpleLoopUnswitch.cpp 2012
• V1023 [CWE-460] A pointer without owner is added to the 'Records' container by the 'emplace_back' method. A memory leak will occur in case of an exception. FDRLogBuilder.h 30
• V1023 [CWE-460] A pointer without owner is added to the 'PendingSubmodules' container by the 'emplace_back' method. A memory leak will occur in case of an exception. ModuleMap.cpp 810
• V1023 [CWE-460] A pointer without owner is added to the 'Objects' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DebugMap.cpp 88
• V1023 [CWE-460] A pointer without owner is added to the 'Strategies' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-isel-fuzzer.cpp 60
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 685
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 686
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 688
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 689
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 690
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 691
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 692
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 693
• V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 694
• V1023 [CWE-460] A pointer without owner is added to the 'Operands' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 1911
• V1023 [CWE-460] A pointer without owner is added to the 'Stash' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2100
• V1023 [CWE-460] A pointer without owner is added to the 'Matchers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2702

Conclusão

Anotei 60 avisos e parei com isso. O PVS-Studio encontrou outros bugs no LLVM? Sim sim. Mas, enquanto escrevia os exemplos, a noite caiu, então decidi parar.

Espero que você tenha gostado de ler este artigo e tenha incentivado você a experimentar o analisador PVS-Studio.

Visite esta página para baixar o analisador e obter uma chave de avaliação.

Mais importante, use a análise estática regularmente. Verificações únicas , como as que fazemos para popularizar a análise estática e promover o PVS-Studio, não são o cenário normal.

Boa sorte em melhorar a qualidade e a confiabilidade do seu código!