Passaram-se mais de dois anos desde a última verificação do código do projeto LLVM com nosso analisador PVS-Studio. Vamos garantir que o analisador PVS-Studio ainda seja a ferramenta líder na identificação de erros e vulnerabilidades potenciais. Para isso, vamos verificar e encontrar novos erros na versão 8.0.0 do LLVM.
Um artigo que precisa ser escrito
Para ser honesto, eu não queria escrever este artigo. Não é interessante escrever sobre um projeto que já verificamos várias vezes (, , ). É melhor escrever sobre algo novo, mas eu não tenho escolha.
Cada vez que uma nova versão do LLVM é lançada ou uma versão é atualizada , surgem em nosso e-mail perguntas do tipo:
Veja, a nova versão do Clang Static Analyzer aprendeu a encontrar novos erros! Acho que a relevância de usar o PVS-Studio está diminuindo. O Clang está encontrando mais erros do que antes e está se aproximando das capacidades do PVS-Studio. O que você pensa sobre isso?
Para isso, sempre quero responder algo do tipo:
Nós também não ficamos parados! Nós melhoramos significativamente as capacidades do analisador PVS-Studio. Portanto, não se preocupe, continuamos a liderar, como sempre.
Infelizmente, essa é uma resposta ruim. Não há provas. E é exatamente por isso que estou escrevendo este artigo agora. Assim, o projeto LLVM foi verificado novamente e diversos erros foram encontrados. Aqueles que me pareceram interessantes, eu vou demonstrar agora. Esses erros não podem ser encontrados pelo Clang Static Analyzer (ou é extremamente inconveniente fazê-lo). E nós podemos. Adicionalmente, eu encontrei e anotei todos esses erros em uma única noite.
Já a redação do artigo se estendeu por várias semanas. Não conseguia me forçar a colocar tudo isso em forma de texto :).
Aliás, se você está interessado em saber quais tecnologias são usadas no analisador PVS-Studio para detectar erros e vulnerabilidades potenciais, sugiro que você se familiarize com esta .
Novas e antigas diagnósticas
Como já foi mencionado, há cerca de dois anos, o projeto LLVM foi verificado novamente e os erros encontrados foram corrigidos. Agora, este artigo apresentará uma nova porção de erros. Por que novos erros foram encontrados? Existem 3 razões para isso:
- O projeto LLVM está em desenvolvimento, o código antigo é modificado e um novo é criado. Naturalmente, o código alterado e escrito contém novos erros. Isso demonstra bem que a análise estática deve ser aplicada regularmente, e não ocasionalmente. Nossos artigos mostram bem as capacidades do analisador PVS-Studio, mas isso não tem relação com a melhoria da qualidade do código e a diminuição dos custos para corrigir erros. Use o analisador de código estático regularmente!
- Estamos aprimorando e aprimorando os diagnósticos já existentes. Portanto, o analisador pode identificar erros que não foram detectados nas verificações anteriores.
- Novos diagnósticos foram adicionados ao PVS-Studio, que não estavam presentes há 2 anos. Decidi destacá-los em uma seção separada para mostrar claramente o desenvolvimento do PVS-Studio.
Defeitos identificados por diagnósticos que existiam há 2 anos
Fragmento N1: Copy-Paste
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Adicionado em 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Adicionado em 7.0
Name == "avx512.mask.cvtps2pd.256" || // Adicionado em 7.0
Name == "avx512.cvtusi2sd" || // Adicionado em 7.0
Name.startswith("avx512.mask.permvar.") || // Adicionado em 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Adicionado em 7.0 // <=
Name == "sse2.pmulu.dq" || // Adicionado em 7.0
Name == "sse41.pmuldq" || // Adicionado em 7.0
Name == "avx2.pmulu.dq" || // Adicionado em 7.0
....
}Aviso PVS-Studio: [CWE-570] Há sub-expressões idênticas ‘Name.startswith(«avx512.mask.permvar.»)’ à esquerda e à direita do operador ‘||’. AutoUpgrade.cpp 73
Está sendo verificado duas vezes se o nome começa com a substring «avx512.mask.permvar.». Na segunda verificação, claramente queriam escrever algo diferente, 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;
....
}Aviso PVS-Studio: V501 Há sub-expressões idênticas ‘CXNameRange_WantQualifier’ à esquerda e à direita do operador ‘|’. CIndex.cpp 7245
Devido a um erro de digitação, a mesma constante nomeada é utilizada duas vezes CXNameRange_WantQualifier.
Fragmento N3: Confusão com a prioridade dos operadores
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Aviso PVS-Studio: [CWE-783] Talvez o operador ‘?:’ funcione de maneira diferente do que se esperava. O operador ‘?:’ tem uma prioridade mais baixa do que o operador ‘==’. PPCTargetTransformInfo.cpp 404
Na minha opinião, esse é um erro muito bonito. Sim, eu sei que tenho uma visão estranha sobre beleza :).
Atualmente, de acordo com , a expressão é calculada da seguinte forma:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Do ponto de vista prático, tal condição não faz sentido, pois pode ser simplificada para:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Esse é um erro claro. O mais provável é que 0/1 quisessem ser comparados com a variável Index. Para corrigir o código, é necessário adicionar parênteses 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 pode provocar erros lógicos. Seja muito cuidadoso com ele e não hesite em usar parênteses. Mais sobre esse tema eu abordei , no capítulo 'Cuidado com 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(Result);
....
LHS = dyn_cast(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("não é possível converter '") + LHS->getAsString() +
"' para string");
return nullptr;
}
....
}Aviso PVS-Studio: [CWE-476] A desreferenciação do ponteiro nulo 'LHS' pode ocorrer. TGParser.cpp 2152
Se o ponteiro LHS for for nulo, uma advertência deve ser emitida. Porém, ao invés disso, haverá a desreferenciação do próprio ponteiro nulo: LHS->getAsString().
Essa é uma situação bastante típica, onde o erro se esconde no manipulador de erros, pois ninguém o testa. Os analisadores estáticos verificam todo o código alcançável, independentemente de quão frequentemente ele é utilizado. Este é um excelente exemplo de como a análise estática complementa outras metodologias de teste e proteção contra erros.
Um erro semelhante de manipulação de ponteiro RHS ocorre no código logo abaixo: V522 [CWE-476] A desreferenciação do ponteiro nulo 'RHS' pode ocorrer. TGParser.cpp 2186
Fragmento N6: Uso do ponteiro após a movimentação
static Expected
ExtractBlocks(....)
{
....
std::unique_ptr ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Função não encontrada??");
MiscompiledFunctions.push_back(NewF);
}
....
}Advertência PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'ProgClone' pode ocorrer. Miscompilation.cpp 601
No início, o ponteiro inteligente ProgClone deixa de possuir o objeto:
BD.setNewProgram(std::move(ProgClone));Na verdade, agora ProgClone — é um ponteiro nulo. Portanto, um pouco mais abaixo a desreferenciação do ponteiro nulo deve ocorrer:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Mas, na verdade, isso não vai acontecer! Note que o loop na verdade não é executado.
No início do contêiner MiscompiledFunctions é limpo:
MiscompiledFunctions.clear();Em seguida, o tamanho deste contêiner é usado na condição do loop:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {É fácil ver que o loop não é iniciado. Acho que isso também é um erro, e o código deveria ser escrito de outra forma.
Parece que encontramos aquela famosa paridade de erros! Um erro mascara o outro :).
Fragmento N7: Uso de um ponteiro após a movimentação
static Expected TestOptimizer(BugDriver &BD, std::unique_ptr Test,
std::unique_ptr Safe) {
outs() << " Otimizando funções em teste: ";
std::unique_ptr Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Erro ao executar esta sequência de passes"
<< " no programa de entrada!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo ‘Test’ pode ocorrer. Miscompilation.cpp 709
Novamente a mesma situação. No início, o conteúdo do objeto é movido, e depois ele é usado como se nada tivesse acontecido. Estou cada vez mais encontrando essa situação no código dos programas desde que a semântica de movimentação foi introduzida no C++. É isso que eu amo na linguagem C++! Sempre surgem novas maneiras de se machucar. O analisador PVS-Studio sempre terá trabalho :).
Fragmento N8: Ponteiro nulo
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "";
else
Type->dump(*this);
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo ‘Type’ pode ocorrer. PrettyFunctionDumper.cpp 233
Além dos manipuladores de erros, geralmente funções de impressão de depuração de dados também não são testadas. Este é um caso exatamente assim. A função espera pelo usuário, que em vez de resolver seus problemas, terá que se dedicar a corrigi-la.
Correto:
if (Type)
Type->dump(*this);
else
Printer << "";Fragmento N9: Ponteiro nulo
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector &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());
....
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Ty' pode ocorrer. SearchableTableEmitter.cpp 614
Acho que isso já está claro e não necessita de 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;
}Aviso PVS-Studio: A variável 'Identifier->Type' está sendo atribuída a si mesma. FormatTokenLexer.cpp 249
Não faz sentido atribuir uma variável a si mesma. Provavelmente queria escrever:
Identifier->Type = Question->Type;Fragmento N11: break 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;
....
}Aviso PVS-Studio: [CWE-478] Considere inspecionar a instrução 'switch'. É possível que o primeiro operador 'case' esteja faltando. SystemZAsmParser.cpp 652
Há um operador muito suspeito no início break. Não se esqueceram de escrever mais alguma coisa aqui?
Fragmento N12: Verificação de 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("chamador indefinido");
....
}Aviso PVS-Studio: [CWE-476] O ponteiro 'Callee' foi utilizado antes de ser verificado contra nullptr. Verifique as linhas: 172, 174. AMDGPUInline.cpp 172
Ponteiro Callee é desreferenciado no momento da chamada da função getTTI.
E então percebe-se que este ponteiro deve ser verificado quanto à igualdade nullptr:
if (!Callee || Callee->isDeclaration())Mas já é tarde...
Fragmento N13 — N…: Verificação de ponteiro após desreferenciação
A situação abordada no fragmento de código anterior não é única. Ela ocorre 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 contra 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(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null(ND->getDeclContext()); // isCXXInstanceMember()); // <=
....
}Aviso PVS-Studio: V595 [CWE-476] O ponteiro 'ND' foi utilizado antes de ser verificado contra nullptr. Verifique as linhas: 532, 534. SemaTemplateInstantiateDecl.cpp 532
E aqui:
- V595 [CWE-476] O ponteiro 'U' foi utilizado antes de ser verificado contra nullptr. Verifique as linhas: 404, 407. DWARFFormValue.cpp 404
- V595 [CWE-476] O ponteiro 'ND' foi utilizado antes de ser verificado contra nullptr. Verifique as linhas: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Depois disso, perdi o interesse em estudar os avisos com o número V595. Portanto, não sei se há outros erros semelhantes além dos mencionados aqui. Provavelmente sim.
Fragmento N17, N18: Deslocamento suspeito
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t & Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Aviso PVS-Studio: [CWE-190] Considere inspecionar a expressão '~(Size — 1) << 1'. Deslocamento de bits do valor de 32 bits com uma subsequente expansão para o tipo de 64 bits. AArch64AddressingModes.h 260
Pode ser que não seja um erro e o código funcione exatamente como pretendido. Mas é claramente um lugar muito suspeito e precisa ser verificado.
Suponha que a variável Tamanho seja igual a 16, e então o autor do código planejou obter na variável NImms o valor:
1111111111111111111111111111111111111111111111111111111111100000
No entanto, na verdade, obter-se-á o valor:
0000000000000000000000000000000011111111111111111111111111100000
O fato é que todos os cálculos são feitos usando o tipo unsigned de 32 bits. E somente depois, esse tipo unsigned de 32 bits será implicitamente expandido para uint64_t. Nesse processo, os bits mais altos ficarão zerados.
A situação pode ser corrigida assim:
uint64_t NImms = ~static_cast(Size-1) << 1;Situação semelhante: V629 [CWE-190] Considere inspecionar a expressão 'Immr << 6'. Deslocamento de bits do valor de 32 bits com uma subsequente expansão para o tipo de 64 bits. AArch64AddressingModes.h 269
Fragmento N19: Palavra-chave ausente else?
void AMDGPUAsmParser::cvtDPP(MCInst & Inst, const OperandVector & Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) uso de dpp "vcc".
// Ignorar.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Tratar argumentos opcionais
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Tipo de operando inválido");
}
....
}Aviso PVS-Studio: [CWE-670] Considere inspecionar a lógica da aplicação. É possível que a palavra-chave 'else' esteja faltando. AMDGPUAsmParser.cpp 5655
Não há erros aqui. Pois o bloco then do primeiro if termina em continue, então não importa se há uma palavra-chave else ou não. De qualquer forma, o código funcionará da mesma forma. No entanto, a omissão else torna o código mais confuso e arriscado. Se, no futuro, continue desaparecer, o código começará a funcionar de forma totalmente diferente. Na minha opinião, é melhor adicionar else.
Fragmento N20: Quatro erros de digitação semelhantes
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 strings foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Result + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] As strings foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Result + "(ObjC Class) " + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] As strings 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 strings foram concatenadas, mas não são utilizadas. Considere inspecionar a expressão 'Result + "(ObjC IVar) " + Name.str()'. Symbol.cpp 41
Acidentalmente, o operador += é substituído pelo operador +. Como resultado, obtemos construções sem sentido.
Fragmento N21: Comportamento indefinido
static void getReqFeatures(std::map &FeaturesMap,
const std::vector &ReqFeatures) {
for (auto &R : ReqFeatures) {
StringRef AsmCondString = R->getValueAsString("AssemblerCondString");
SmallVector Ops;
SplitString(AsmCondString, Ops, ",");
assert(!Ops.empty() && "AssemblerCondString não pode estar vazio");
for (auto &Op : Ops) {
assert(!Op.empty() && "Operador vazio");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Tente encontrar o código perigoso por conta própria. E esta é uma imagem para distrair a atenção, para não olhar imediatamente a resposta:
Aviso PVS-Studio: [CWE-758] Uma construção perigosa é utilizada: 'FeaturesMap[Op] = FeaturesMap.size()', onde 'FeaturesMap' é da classe 'map'. Isso pode levar a comportamento indefinido. RISCVCompressInstEmitter.cpp 490
Linha problemática:
FeaturesMap[Op] = FeaturesMap.size();Se o elemento Op não for encontrado, um novo elemento é criado no mapa e o número de elementos neste mapa é gravado. O único problema é que não se sabe se a função tamanho será chamada antes ou depois da adição de um novo elemento.
Fragmento N22-N24: Atribuições repetidas
Erro 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 PVS-Studio: [CWE-563] A variável ‘NType’ está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 1663, 1664. MachOObjectFile.cpp 1664
Acredito que não haja um erro real aqui. Apenas uma atribuição duplicada. Mas ainda assim é um deslize.
De maneira similar:
- V519 [CWE-563] A variável ‘B.NDesc’ está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] A variável está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 59, 61. coff2yaml.cpp 61
Fragmento N25-N27: Ainda atribuições repetidas.
Agora vamos considerar uma variante um pouco diferente de atribuição repetida.
bool Vectorizer::vectorizeLoadChain(
ArrayRef Chain,
SmallPtrSet *InstructionsProcessed) {
....
unsigned Alignment = getAlignment(L0);
....
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
StackAdjustedAlignment,
DL, L0, nullptr, &DT);
if (NewAlign != 0)
Alignment = NewAlign;
Alignment = NewAlign;
....
}Aviso PVS-Studio: V519 [CWE-563] A variável ‘Alignment’ está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 1158, 1160. LoadStoreVectorizer.cpp 1160
Este é um código muito estranho, que aparentemente contém um erro lógico. No início, a variável Alignment recebe um valor dependendo da condição. E então há uma nova atribuição, mas agora sem qualquer verificação.
Situações semelhantes podem ser vistas aqui:
- V519 [CWE-563] A variável ‘Effects’ está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] A variável ‘ExpectNoDerefChunk’ está recebendo valores duas vezes sucessivamente. Isso pode ser um erro. Verifique as linhas: 4970, 4973. SemaType.cpp 4973
Fragmento N28: Condição sempre 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) // Suporte a instrução PAUSE // <=
break;
}
....
}Aviso PVS-Studio: [CWE-571] A expressão ‘nextByte != 0x90’ é sempre verdadeira. X86DisassemblerDecoder.cpp 379
A verificação não faz sentido. A variável nextByte é sempre diferente do valor 0x90, o que decorre da verificação anterior. Isso é um erro lógico.
Fragmento N29 — N…: Condições sempre verdadeiras/falsas.
O analisador fornece muitos avisos sobre o fato de que toda a condição () ou parte dela () sempre verdadeiro ou falso. Muitas vezes, isso não são erros reais, mas apenas código desleixado, resultado da expansão de macros e coisas do tipo. No entanto, vale a pena observar todos esses avisos, pois de tempos em tempos aparecem verdadeiros erros lógicos. 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 PVS-Studio: [CWE-570] Uma parte da expressão condicional é sempre falsa: RegNo == 0xe. ARMDisassembler.cpp 939
A constante 0xE é o valor 14 no sistema decimal. A verificação RegNo == 0xe não faz sentido, pois se RegNo > 13, então a função encerrará sua execução.
Houve muitos outros avisos com os identificadores V547 e V560, mas, assim como no caso de , estudar esses avisos não me interessava. Já estava claro que tinha material suficiente para escrever um artigo :). Portanto, não se sabe quantos erros desse tipo podem ser identificados no LLVM com a ajuda do PVS-Studio.
Vou dar um exemplo de por que estudar esses disparos é maçante. O analisador está absolutamente correto ao emitir um aviso sobre o seguinte código. Mas isso não é um erro.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Aviso PVS-Studio: V547 [CWE-570] A expressão ‘!HasError’ é sempre falsa. UnwrappedLineParser.cpp 1635
Fragmento N30: Return 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();
}
....
}Aviso PVS-Studio: [CWE-670] Um ‘return’ incondicional dentro de um loop. R600OptimizeVectorRegisters.cpp 63
Isso é um erro ou uma técnica específica destinada a esclarecer algo para os programadores que leem o código. Para mim, tal construção não esclarece nada e parece muito suspeita. Melhor não escrever assim :).
Cansado? Então é hora de fazer um chá ou café.
Defeitos identificados por novos diagnósticos
Acho que 30 disparos de diagnósticos antigos são suficientes. Agora vamos ver o que podemos encontrar de interessante com os novos diagnósticos que foram adicionados ao analisador após a verificação. Durante esse tempo, foram adicionados 66 diagnósticos de uso geral no analisador de C++.
Fragmento N31: Código inacessível
Erro 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 PVS-Studio: [CWE-561] Código inatingível detectado. É possível que um erro esteja presente. ExecutionUtils.cpp 146
Como você pode ver, ambos os ramos do operador if terminam com a chamada do operador return. Assim, o contêiner CtorDtorsByPriority nunca será limpo.
Fragmento N32: Código inatingí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(), "kind de resumo inesperado");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}Aviso PVS-Studio: V779 [CWE-561] Código inatingível detectado. É possível que um erro esteja presente. LLParser.cpp 835
Situação interessante. Vamos analisar primeiro este ponto:
return ParseTypeIdEntry(SummaryID);
break;À primeira vista, parece que não há erro aqui. Parece que o operador break está de mais e pode ser simplesmente removido. No entanto, não é tão simples.
O analisador gera um aviso para as linhas:
Lex.setIgnoreColonInIdentifiers(false);
return false;E realmente, este código é inatingível. Todos os casos em switch terminam com a chamada do operador return. E agora este solitário sem sentido break não parece tão inofensivo! Talvez um dos ramos devesse terminar em break, e não em return?
Fragmento N33: Zeragem aleatória de bits mais altos
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}Aviso PVS-Studio: 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 o tipo unsigned. Vamos calcular o valor da expressão, supondo que a função retorne o valor 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFF8u
Agora, observe que a variável DataSize possui um tipo sem sinal de 64 bits. Portanto, ao executar a operação DataSize & 0xFFFFFFF8u todos os trinta e dois bits mais altos serão zerados. Provavelmente, isso não é o que o programador queria. Suspeito que ele queria calcular: DataSize & 0xFFFFFFFFFFFFFFF8u.
Para corrigir o erro, deve-se escrever assim:
DataSize &= ~(static_cast(getStubAlignment()) - 1);Ou assim:
DataSize &= ~(getStubAlignment() - 1ULL);Fragmento N34: Conversão de tipo explícita falha
template
void scaleShuffleMask(int Scale, ArrayRef Mask,
SmallVectorImpl &ScaledMask) {
assert(0 < Scale && "Fator de escala inesperado");
int NumElts = Mask.size();
ScaledMask.assign(static_cast(NumElts * Scale), -1);
....
}Aviso PVS-Studio: [CWE-190] Possível estouro. Considere converter os operandos do operador ‘NumElts * Scale’ para o tipo ‘size_t’, e não o resultado. X86ISelLowering.h 1577
A conversão de tipo explícita é utilizada para evitar que ocorram estouros ao multiplicar variáveis do tipo int. No entanto, aqui a conversão de tipo explícita não protege contra o estouro. Inicialmente, as variáveis serão multiplicadas, e apenas depois o resultado de 32 bits da multiplicação será ampliado para o tipo .
Fragmento N35: Copy-Paste falho
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;
}
....
}[CWE-682] Duas fragmentos de código semelhantes foram encontrados. Talvez, essa seja uma digitação e a variável ‘Op1’ devesse ser utilizada em vez de ‘Op0’. InstCombineCompares.cpp 5507
Esse novo diagnóstico interessante revela situações em que um fragmento de código foi copiado, e alguns nomes começaram a ser alterados, mas em um lugar não foi corrigido.
Observe que no segundo bloco mudaram Op0 para Op1. Mas em um lugar não foi corrigido. 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 de variáveis
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Aviso PVS-Studio: [CWE-563] A variável ‘Mode’ é atribuída, mas não é utilizada até o final da função. SIModeRegister.cpp 48
É muito perigoso dar aos argumentos das funções os mesmos nomes que os membros da classe. É muito fácil se confundir. Este é exatamente um caso assim. Esta expressão não faz sentido:
Mode &= Mask;Muda apenas o argumento da função. E tudo. Este argumento não é mais utilizado. Provavelmente, deveria ter sido escrito assim:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragmento N37: Confusão de 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(Sym));
Size += this->EntrySize;
}Aviso PVS-Studio: V1001 [CWE-563] A variável 'Size' é atribuída, mas não é utilizada até o final da função. Object.cpp 424
A situação é semelhante à anterior. Deve ser escrito:
this->Size += this->EntrySize;Fragmento N38-N47: O ponteiro foi esquecido de ser verificado
Anteriormente, examinamos exemplos de detecção de diagnóstico . Sua essência é que o ponteiro é desreferenciado no início e apenas depois verificado. Um novo diagnóstico é o oposto, mas também detecta muitos erros. Ele identifica situações onde o ponteiro foi verificado no início e, em seguida, esqueceram de fazer isso. Vamos considerar tais casos encontrados dentro do LLVM.
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}Aviso PVS-Studio: V1004 [CWE-476] O ponteiro 'Ptr' foi usado de maneira insegura após ter sido verificado contra nullptr. Verifique as linhas: 729, 738. TargetTransformInfoImpl.h 738
Variável Ptr pode ser igual a nullptr, como evidenciado pela verificação:
if (Ptr != nullptr)No entanto, abaixo esse ponteiro é desreferenciado sem verificação prévia:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Vamos considerar outro caso semelhante.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast(GD.getDecl());
SmallVector ArgTypes;
if (FD) // parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs()->getCallConv(); // <=
....
}Aviso PVS-Studio: V1004 [CWE-476] O ponteiro 'FD' foi usado de maneira insegura após ter sido verificado contra nullptr. Verifique as linhas: 3228, 3231. CGDebugInfo.cpp 3231
Observe o ponteiro FD. Estou certo de que o problema é bem visível, e explicações especiais não são necessárias.
E ainda:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast(Ptr.getType());
if (!PtrTy) {
Result = Polynomial();
BasePtr = nullptr;
}
unsigned PointerBits =
DL.getIndexSizeInBits(PtrTy->getPointerAddressSpace());
....
}Advertência PVS-Studio: V1004 [CWE-476] O ponteiro 'PtrTy' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 960, 965. InterleavedLoadCombinePass.cpp 965
Como se proteger contra esses erros? Preste mais atenção na Revisão de Código e use um analisador de código estático PVS-Studio para verificar regularmente.
Não faz sentido apresentar outros trechos de código com erros desse tipo. Vou deixar no artigo apenas a lista de avisos:
- V1004 [CWE-476] O ponteiro 'Expr' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] O ponteiro 'PI' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] O ponteiro 'StatepointCall' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] O ponteiro 'RV' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] O ponteiro 'CalleeFn' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] O ponteiro 'TC' foi usado de forma insegura após ter sido verificado em relação a nullptr. Verifique as linhas: 1819, 1824. Driver.cpp 1824
Trecho N48-N60: Não é crítico, mas é um defeito (possível vazamento de memória)
std::unique_ptr createISelMutator() {
....
std::vector<std::unique_ptr> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Aviso PVS-Studio: [CWE-460] Um ponteiro sem dono é adicionado ao contêiner 'Strategies' pelo método 'emplace_back'. Um vazamento de memória ocorrerá em caso de exceção. llvm-isel-fuzzer.cpp 58
Para adicionar um elemento ao final de um contêiner do tipo std::vector<std::unique_ptr> não se pode simplesmente escrever xxx.push_back(new X), pois não há conversão implícita de X* em std::unique_ptr.
Uma solução comum é escrever xxx.emplace_back(new X), pois ele se compila: o método emplace_back constrói o elemento diretamente dos argumentos e, portanto, pode utilizar construtores explícitos.
Isso não é seguro. Se o vetor estiver cheio, ocorrerá uma realocação de memória. A operação de realocação de memória pode falhar, resultando na geração de uma exceção std::bad_alloc. Neste caso, o ponteiro será perdido e o objeto criado nunca será removido.
Uma solução segura é criar um unique_ptr, que manterá a posse do ponteiro até que o vetor tente realocar memória:
xxx.push_back(std::unique_ptr(new X))A partir do C++14, é possível usar 'std::make_unique':
xxx.push_back(std::make_unique())Esse tipo de defeito não é crítico para o LLVM. Se a alocação de memória falhar, o funcionamento do compilador simplesmente será interrompido. No entanto, para aplicativos com longos , que não podem simplesmente ser finalizados caso a alocação de memória falhe, isso pode ser um erro realmente desagradável.
Portanto, embora esse código não apresente um perigo prático para o LLVM, achei útil discutir esse padrão de erros e como o analisador PVS-Studio aprendeu a identificá-lo.
Outros avisos desse 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] Um ponteiro sem proprietário é adicionado ao contêiner ‘AAs’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. AliasAnalysis.h 324
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Entries’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘AllEdges’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. CFGMST.h 268
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘VMaps’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Records’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. FDRLogBuilder.h 30
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘PendingSubmodules’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. ModuleMap.cpp 810
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Objects’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. DebugMap.cpp 88
- 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 60
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 685
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 686
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 688
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 689
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 690
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner ‘Modifiers’ pelo método ‘emplace_back’. Um vazamento de memória ocorrerá em caso de uma exceção. llvm-stress.cpp 691
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Modifiers’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. llvm-stress.cpp 692
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Modifiers’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. llvm-stress.cpp 693
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Modifiers’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. llvm-stress.cpp 694
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Operands’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Stash’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao container ‘Matchers’ pelo método ’emplace_back’. Um vazamento de memória ocorrerá em caso de exceção. GlobalISelEmitter.cpp 2702
Conclusão
No total, anotei 60 avisos, depois do que parei. Existem outros defeitos que o analisador PVS-Studio detecta no LLVM? Sim, existem. No entanto, enquanto anotava os trechos de código para o artigo, já era tarde da noite e decidi que era hora de parar.
Espero que tenha sido interessante para vocês e que queiram experimentar o analisador PVS-Studio.
Você pode baixar o analisador e obter uma chave de teste em .
O mais importante é usar a análise estática regularmente. Verificações pontuais, realizadas por nós com o objetivo de popularizar a metodologia de análise estática e PVS-Studio, não são um cenário normal.
Boa sorte na melhoria da qualidade e confiabilidade do código!
Se você quiser compartilhar este artigo com uma audiência de língua inglesa, por favor, use o link para a tradução: Andrey Karpov. .
Fonte: habr.com
