Geekly articles weekly

Encontre erros no LLVM 8 usando o analisador PVS-Studio

PVS-Studio e LLVM 8.0.0

Mais de dois anos se passaram desde a última verificação do código do projeto LLVM usando nosso analisador PVS-Studio. Vamos garantir que o analisador PVS-Studio ainda seja a principal ferramenta para detectar erros e possíveis vulnerabilidades. Para fazer isso, verifique e encontre novos erros na versão LLVM 8.0.0.

Artigo a ser escrito


Para ser sincero, não queria escrever este artigo. Não é interessante escrever sobre um projeto que já testamos repetidamente ( 1 , 2 , 3 ). É melhor escrever sobre algo novo, mas não tenho escolha.

Sempre que uma nova versão do LLVM é lançada ou o Clang Static Analyzer é atualizado, as seguintes perguntas aparecem em nosso e-mail:

Olha, a nova versão do Clang Static Analyzer aprendeu a encontrar novos bugs! Parece-me que a relevância do uso do PVS-Studio está diminuindo. O Clang encontra mais erros do que antes e alcança os recursos do PVS-Studio. O que você acha disso?

Para isso, eu sempre quero responder algo no espírito:

Também não estamos ociosos! Melhoramos significativamente os recursos do analisador PVS-Studio. Portanto, não se preocupe, continuamos liderando, como antes.

Infelizmente, esta é uma resposta ruim. Não há provas nele. E é por isso que estou escrevendo este artigo agora. Portanto, o projeto LLVM foi novamente testado e vários erros foram encontrados nele. Aqueles que me pareciam interessantes, agora demonstrarei. Esses erros não podem ser encontrados pelo Clang Static Analyzer (ou é extremamente inconveniente). E nós podemos. E eu descobri e escrevi todos esses erros em uma noite.

Mas a redação do artigo se prolongou por várias semanas. Não pude me forçar a organizar tudo isso na forma de texto :).

A propósito, se você estiver interessado em quais tecnologias são usadas no analisador PVS-Studio para detectar erros e possíveis vulnerabilidades, sugiro que você se familiarize com esta nota .

Diagnósticos novos e antigos


Como já foi observado, há cerca de dois anos, o projeto LLVM foi novamente verificado e os erros encontrados foram corrigidos. Agora, neste artigo, uma nova porção de erros será apresentada. Por que novos bugs foram encontrados? Existem três razões para isso:

  1. O projeto LLVM se desenvolve, o código antigo é alterado e um novo é exibido. Naturalmente, há novos erros no código modificado e escrito. Isso demonstra bem que a análise estática deve ser aplicada regularmente, e não caso a caso. Nossos artigos mostram bem os recursos do analisador PVS-Studio, mas isso não tem nada a ver com melhorar a qualidade do código e reduzir o custo de correção de erros. Use um analisador de código estático regularmente!
  2. Estamos finalizando e aprimorando os diagnósticos existentes. Portanto, o analisador pode detectar erros que não foram notados durante as verificações anteriores.
  3. O PVS-Studio introduziu novos diagnósticos, não há 2 anos atrás. Decidi separá-los em uma seção separada para demonstrar claramente o desenvolvimento do PVS-Studio.

Defeitos identificados por diagnósticos que existiam 2 anos atrás


Fragmento N1: 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 .... }

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

É verificado que o nome começa com a subcadeia "avx512.mask.permvar". No segundo teste, eles claramente queriam escrever outra coisa, mas esqueceram de corrigir o texto copiado.

Fragmento N2: 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; .... }

PVS-Studio Warning: V501 Existem subexpressões idênticas 'CXNameRange_WantQualifier' à esquerda e à direita da '|' operador. CIndex.cpp 7245

Por causa de um erro de digitação, a mesma constante nomeada CXNameRange_WantQualifier é usada duas vezes .

Fragmento N3: confusão sobre prioridades do operador

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

Aviso 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

Na minha opinião, este é um erro muito bonito. Sim, eu sei que tenho idéias estranhas sobre beleza :).

Agora, de acordo com as prioridades dos operadores , a expressão é calculada da seguinte maneira:

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

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

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

Este é um erro claro. Muito provavelmente, 0/1 queria comparar com a variável Index . Para corrigir o código, adicione colchetes ao redor do operador ternário:

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

A propósito, o operador ternário é muito perigoso e provoca erros lógicos. Tenha muito cuidado com ele e não seja ganancioso em colocar parênteses. Eu considerei esse tópico com mais detalhes aqui no capítulo “Tema o operador?: E coloque-o entre parênteses”.

Fragmento N4, N5: 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; } .... }

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

Se o ponteiro LHS for nulo, um aviso deve ser emitido. No entanto, em vez disso, desreferirá esse ponteiro muito nulo: LHS-> getAsString () .

Essa é uma situação muito típica quando o erro está oculto no manipulador de erros, pois ninguém os está testando. Os analisadores estáticos verificam todos os códigos acessíveis, independentemente da frequência com que são usados. Este é um exemplo muito bom de como a análise estática complementa outras técnicas de teste e proteção contra erros.

Um erro semelhante ao processar o ponteiro RHS é feito no código abaixo: V522 [CWE-476] A desreferenciação do ponteiro nulo 'RHS' pode ocorrer. TGParser.cpp 2186

Fragmento N6: usando o cursor após mover

 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); } .... }

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

No começo, o ponteiro inteligente ProgClone deixa de possuir o objeto:

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

De fato, agora ProgClone é um ponteiro nulo. Portanto, uma desreferência do ponteiro nulo deve ocorrer logo abaixo:

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

Mas, de fato, isso não vai acontecer! Observe que o loop não está realmente em execução.

No início, o contêiner MiscompiledFunctions é limpo:

 MiscompiledFunctions.clear();

Em seguida, o tamanho desse contêiner é usado na condição de loop:

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

É fácil ver que o loop não inicia. Eu acho que isso também é um erro, e o código deve ser escrito de maneira diferente.

Parece que conhecemos a famosa paridade de erros! Um erro disfarça outro :).

Fragmento N7: Usando o cursor depois de mover

 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; } .... }

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

Mais uma vez a mesma situação. No início, o conteúdo do objeto é movido e, em seguida, é usado como se nada tivesse acontecido. Cada vez mais vejo essa situação no código do programa, após a semântica do movimento aparecer em C ++. Por isso, adoro a linguagem C ++! Existem mais e mais novas maneiras de fotografar sua própria perna. O analisador PVS-Studio sempre funcionará :).

Fragmento N8: 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); }

PVS-Studio Warning: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Type' pode ocorrer. PrettyFunctionDumper.cpp 233

Além dos manipuladores de erro, as funções para depuração de impressões geralmente não são testadas. Diante de nós é exatamente esse caso. A função está aguardando um usuário que, em vez de resolver seus problemas, seja forçado a corrigi-la.

Corretamente:

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

Fragmento N9: 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()); .... }

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

Eu acho, e então tudo está claro e não requer explicações.

Fragmento N10: 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; }

PVS-Studio Warning: V570 A variável 'Identifier-> Type' é atribuída a si mesma. FormatTokenLexer.cpp 249

Não faz sentido atribuir uma variável a si mesma. Muito provavelmente eles queriam escrever:

 Identifier->Type = Question->Type;

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

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

No início, há uma declaração de interrupção muito suspeita. Você esqueceu de escrever outra coisa aqui?

Fragmento N12: Verificando o ponteiro após a 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"); .... }

Aviso 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 no início é desreferenciado quando a função getTTI é chamada .

E acontece que esse ponteiro deve ser verificado quanto à igualdade nullptr :

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

Mas é tarde demais ...

Fragmento N13 - N ...: Verificando o ponteiro após desreferenciação

A situação discutida no snippet de código anterior não é exclusiva. Ela é encontrada aqui:

 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()) { // <= .... }

Aviso 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 aqui:

 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()); // <= .... }

Aviso 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

E então não me interessou estudar os avisos com o número V595. Portanto, não sei se existem erros semelhantes além dos listados aqui. Provavelmente existe.

Fragmento N17, N18: mudança suspeita

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

Aviso 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

Talvez isso não seja um erro, e o código funcione exatamente como pretendido. Mas este é claramente um lugar muito suspeito e precisa ser verificado.

Suponha que a variável Size seja 16 e, em seguida, o autor do código planejado para obter o valor na variável NImms :

111111111111111111111111111111111111111111111111111111111111100000000

No entanto, de fato, o resultado é:

0000000000000000000000000000000000001111111111111111111111111111100000

O fato é que todos os cálculos ocorrem usando o tipo não assinado de 32 bits. E somente então, esse tipo não assinado de 32 bits será implicitamente expandido para uint64_t . Nesse caso, os bits mais significativos serão zero.

Você pode corrigir a situação assim:

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

Situação semelhante: 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 N19: palavra-chave com mais falta ?

 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"); } .... }

Aviso 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

Não há erro aqui. Como o bloco então do primeiro se termina com continue , não importa se há uma palavra-chave else ou não. De qualquer forma, o código funcionará da mesma maneira. No entanto, a falta de outra pessoa torna o código mais obscuro e perigoso. Se no futuro continuar desaparecer, o código começará a funcionar de uma maneira completamente diferente. Na minha opinião, é melhor adicionar mais .

Fragmento N20: quatro erros de digitação do mesmo tipo

 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; }

Avisos 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

Por acaso, o operador + é usado em vez do operador + =. O resultado são designs sem sentido.

Fragmento N21: 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 encontrar o código perigoso você mesmo. E esta é uma imagem de distração, para não olhar imediatamente para a resposta:

Hummm ...


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

Linha do problema:

 FeaturesMap[Op] = FeaturesMap.size();

Se o elemento Op não for encontrado, um novo elemento será criado no mapa e o número de elementos nesse mapa será gravado lá. Não se sabe se a função size será chamada antes ou depois da adição de um novo elemento.

Fragmento N22-N24: Transferências

 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; } .... }

Aviso 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

Eu acho que não há nenhum erro real aqui. Apenas tarefa repetida supérflua. Mas ainda é um erro.

Da mesma forma:

  • 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

Fragmento N25-N27: Mais reatribuições

Agora considere uma opção de reatribuição um pouco diferente.

 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; .... }

PVS-Studio Warning: V519 [CWE-563] A variável 'Alignment' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1158, 1160. LoadStoreVectorizer.cpp 1160

Este é um código muito estranho que parece conter um erro lógico. No início, a variável Alinhamento recebe um valor dependendo da condição. E a atribuição ocorre novamente, mas agora sem nenhuma verificação.

Situações semelhantes podem ser vistas aqui:

  • 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

Fragmento N28: Sempre uma condição verdadeira

 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; } .... }

Aviso 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 nem sempre é igual a 0x90 , que segue da verificação anterior. Isso é algum tipo de erro lógico.

Fragmento N29 - N ...: sempre verdadeiras / falsas condições

O analisador emite muitos avisos de que toda a condição ( V547 ) ou parte dela ( V560 ) é sempre verdadeira ou falsa. Frequentemente, esses erros não são reais, mas simplesmente códigos imprecisos, o resultado da implantação de macros e similares. No entanto, faz sentido olhar para todos esses avisos, pois de tempos em tempos existem erros lógicos reais. Por exemplo, este trecho de código é suspeito:

 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; .... }

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

A constante 0xE é o valor 14 no sistema decimal. Verificar RegNo == 0xe não faz sentido, porque se RegNo> 13 , a função concluirá sua execução.

Havia muitos outros avisos com os identificadores V547 e V560, mas, como no V595 , eu não estava interessado em estudar esses avisos. Já estava claro que eu tinha material suficiente para escrever um artigo :). Portanto, não se sabe quantos erros desse tipo podem ser detectados no LLVM usando o PVS-Studio.

Vou dar um exemplo de por que é chato estudar essas respostas. O analisador está absolutamente certo ao emitir um aviso para o código a seguir. Mas isso não é um erro.

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

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

Fragmento N30: ​​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(); } .... }

PVS-Studio Warning: V612 [CWE-670] Um 'retorno' incondicional dentro de um loop. R600OptimizeVectorRegisters.cpp 63

Isso é um erro ou um truque específico que visa explicar algo aos programadores que leem o código. Esse design não me explica nada e parece muito suspeito. É melhor não escrever assim :).

Esta cansado Então é hora de fazer chá ou café.

Café


Defeitos detectados por novos diagnósticos


Acho que 30 disparos de diagnósticos antigos são suficientes. Vamos agora ver o que é interessante, com novos diagnósticos que apareceram no analisador após uma verificação anterior . No total, 66 diagnósticos de uso geral foram adicionados ao analisador C ++ durante esse período.

Fragmento N31: 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(); }

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

Como você pode ver, os dois ramos da instrução if terminam com uma chamada para a instrução return . Portanto, o contêiner CtorDtorsByPriority nunca será esvaziado .

Fragmento N32: 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; }

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

Uma situação interessante. Vejamos o começo deste lugar:

 return ParseTypeIdEntry(SummaryID); break;

À primeira vista, parece que não há erro. Parece que a instrução break é supérflua aqui e você pode simplesmente excluí-la. No entanto, nem tudo é tão simples.

O analisador gera um aviso nas linhas:

 Lex.setIgnoreColonInIdentifiers(false); return false;

Na verdade, esse código é inacessível. Todos os casos no switch terminam com uma chamada para a declaração de retorno . E agora a pausa solitária e sem sentido não parece tão inofensiva! Talvez um dos galhos termine com quebra , e não com retorno ?

Fragmento N33: Acidentalização de bits altos

 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); .... }

PVS-Studio Warning: 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 tipo não assinado . Calculamos o valor da expressão se assumirmos que a função retorna o valor 8:

~ (getStubAlignment () - 1)

~ (8u-1)

0xFFFFFFF8u

Agora observe que a variável DataSize possui um tipo não assinado de 64 bits. Acontece que durante a operação DataSize & 0xFFFFFFF88, todos os trinta e dois bits de ordem superior serão redefinidos. Provavelmente, não é isso que o programador queria. Eu suspeito que ele queria calcular: DataSize & 0xFFFFFFFFFFFFFFFFFFF8u.

Para corrigir o erro, você deve escrever assim:

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

Ou então:

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

Fragmento N34: falha na conversão explícita

 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); .... }

Aviso 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 impedir o estouro ao multiplicar variáveis ​​do tipo int . No entanto, a conversão explícita aqui não protege contra o estouro.No início, as variáveis ​​serão multiplicadas e somente então o resultado da multiplicação de 32 bits será expandido para o tipo size_t .

Fragmento N35: copiar e colar malsucedido

 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 interessante revela situações em que um pedaço de código foi copiado e alguns nomes começaram a mudar nele, mas não foram corrigidos em um só lugar.

Note-se que na segunda mudança de bloco op0 em Op1 . Mas em um lugar eles não consertaram. Provavelmente, deveria ter sido escrito assim:

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

Fragmento N36: Confusão em variáveis

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

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

é muito perigoso dar argumentos para a função o mesmo nome que os membros da classe. Muito fácil ficar confuso. Diante de nós é exatamente esse caso. Esta expressão não faz sentido:

 Mode &= Mask;

O argumento da função muda. E é isso. Este argumento não é mais usado. Provavelmente, foi necessário escrever assim:

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

Fragmento N37: Confusão em variáveis

 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; }

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

A situação é semelhante à anterior. Deve ser escrito:

 this->Size += this->EntrySize;

Fragmento N38-N47: O ponteiro esqueceu de verificar

Antes, examinamos exemplos de disparo do diagnóstico V595 . Sua essência é que o ponteiro é desreferenciado no início e somente então verificado. O diagnóstico jovem do V1004 é o contrário de seu significado, mas também detecta muitos erros. Ele identifica situações em que o ponteiro foi verificado no início e depois esqueceu de fazê-lo. Considere esses casos encontrados dentro 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()); // <= .... }

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

variável Ptr pode ser igual nullptr , como evidenciado por meio de cheque:

 if (Ptr != nullptr)

No entanto, abaixo deste ponteiro é desreferenciado sem verificação prévia:

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

Considere 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(); // <= .... }

Aviso 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

Preste atenção ao ponteiro FD . Estou certo de que o problema é claramente visível e nenhuma explicação especial é necessária.

E também:

 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()); // <= .... }

Aviso 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 se proteger de tais erros? Tenha cuidado com a revisão de código e use o analisador estático PVS-Studio para verificar regularmente seu código.

Não faz sentido trazer outros fragmentos de código com erros desse tipo. Deixarei apenas uma lista de avisos no artigo:

  • 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] The 'StatepointCall' pointer was used unsafely after it was verified against nullptr. Check lines: 4371, 4379. Verifier.cpp 4379
  • V1004 [CWE-476] The 'RV' pointer was used unsafely after it was verified against nullptr. Check lines: 2263, 2268. TGParser.cpp 2268
  • V1004 [CWE-476] The 'CalleeFn' pointer was used unsafely after it was verified against nullptr. Check lines: 1081, 1096. SimplifyLibCalls.cpp 1096
  • V1004 [CWE-476] The 'TC' pointer was used unsafely after it was verified against nullptr. Check lines: 1819, 1824. Driver.cpp 1824

N48-N60: , ( )

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

PVS-Studio Warning: 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

Para adicionar um item ao final de um contêiner como std :: vector <std :: unique_ptr <X>>, você não pode simplesmente escrever xxx.push_back (novo X) , pois não há conversão implícita de X * para std: : unique_ptr <X> .

Uma solução comum é escrever xxx.emplace_back (novo X) , porque ele compila: o método emplace_back constrói um elemento diretamente dos argumentos e, portanto, pode usar construtores explícitos.

Isso não é seguro. Se o vetor estiver cheio, a memória será alocada. Uma operação de realocação de memória pode falhar, resultando no lançamento de uma exceção std :: bad_alloc . Nesse caso, o ponteiro será perdido e o objeto criado nunca será excluído.

Uma solução segura é criar unique_ptr , que possuirá o ponteiro antes que o vetor tente realocar a memória:

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

A partir do C ++ 14, você pode usar 'std :: make_unique':

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

Esse tipo de defeito não é crítico para o LLVM. Se a memória não puder ser alocada, o compilador simplesmente parará de funcionar. No entanto, para aplicativos com um longo tempo de atividade que não pode terminar apenas se a alocação de memória falhar, isso pode ser um bug muito desagradável.

Portanto, embora esse código não represente um perigo prático para o LLVM, achei útil falar sobre esse padrão de erro e que o analisador PVS-Studio aprendeu a detectá-lo.

Outros avisos deste tipo:

  • 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


No total, escrevi 60 avisos, após o que parei. Existem outros defeitos que o analisador PVS-Studio detecta no LLVM? Sim existe. No entanto, quando escrevi os trechos de código do artigo, era tarde da noite, ou melhor, até noite, e decidi que era hora de terminar.

Espero que você tenha se interessado e queira experimentar o analisador PVS-Studio.

Você pode baixar o analisador e obter uma chave de avaliação nesta página .

Mais importante, use a análise estática regularmente. As verificações únicas realizadas por nós para popularizar a metodologia de análise estática e o PVS-Studio não são um cenário normal.

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



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Andrey Karpov. Localizando erros no LLVM 8 com PVS-Studio .

Source: https://habr.com/ru/post/pt450008/

More articles:


All Articles