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 uma ferramenta líder para identificar erros e vulnerabilidades potenciais. Para fazer isso, verificaremos e encontraremos 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á verificamos diversas vezes (, , ). É melhor escrever sobre algo novo, mas não tenho escolha.
Cada vez que uma nova versão do LLVM é lançada ou atualizada , recebemos perguntas do seguinte tipo em nosso e-mail:
Olha, a nova versão do Clang Static Analyzer aprendeu a encontrar novos erros! Parece-me que a relevância do uso do PVS-Studio está diminuindo. Clang encontra mais erros do que antes e alcança os recursos do PVS-Studio. O que você pensa sobre isso?
Para isso eu sempre quero responder algo como:
Nós também não ficamos parados! Melhoramos significativamente os recursos do analisador PVS-Studio. Então não se preocupe, continuamos liderando como antes.
Infelizmente, esta é uma resposta ruim. Não há provas nisso. E é por isso que estou escrevendo este artigo agora. Assim, o projeto LLVM foi verificado mais uma vez e vários erros foram encontrados nele. Vou agora demonstrar aqueles que me pareceram interessantes. O Clang Static Analyzer não consegue encontrar esses erros (ou é extremamente inconveniente fazê-lo com sua ajuda). Mas nós podemos. Além disso, encontrei e anotei todos esses erros em uma noite.
Mas escrever o artigo demorou várias semanas. Eu simplesmente não consegui colocar tudo isso em texto :).
Aliás, se você estiver interessado em saber quais tecnologias são utilizadas no analisador PVS-Studio para identificar erros e possíveis vulnerabilidades, sugiro que você se familiarize com este .
Diagnósticos novos e antigos
Conforme já referido, há cerca de dois anos o projecto LLVM foi novamente verificado e os erros encontrados foram corrigidos. Agora este artigo apresentará um novo lote de erros. Por que novos bugs foram encontrados? Existem 3 razões para isso:
- O projeto LLVM está evoluindo, alterando códigos antigos e adicionando novos códigos. Naturalmente, existem novos erros no código modificado e escrito. Isto demonstra claramente que a análise estática deve ser usada regularmente, e não ocasionalmente. Nossos artigos mostram bem as capacidades 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!
- Estamos finalizando e melhorando os diagnósticos existentes. Portanto, o analisador pode identificar erros que não percebeu durante as verificações anteriores.
- Surgiram novos diagnósticos no PVS-Studio que não existiam 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: 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
....
}Aviso do PVS-Studio: [CWE-570] Existem subexpressões idênticas 'Name.startswith("avx512.mask.permvar.")' à esquerda e à direita de '||' operador. Atualização automática.cpp 73
É verificado novamente se o nome começa com a substring "avx512.mask.permvar.". Na segunda verificação, obviamente 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;
....
}Aviso PVS-Studio: V501 Existem subexpressões idênticas 'CXNameRange_WantQualifier' à esquerda e à direita do '|' operador. CIndex.cpp 7245
Devido a um erro de digitação, a mesma constante nomeada é usada duas vezes CXNameRange_WantQualifier.
Fragmento N3: Confusão com precedência de 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: [CWE-783] Talvez o operador '?:' funcione de maneira diferente do esperado. 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 ideias estranhas sobre beleza :).
Agora, de acordo com , a expressão é avaliada 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 reduzida a:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Este é um erro claro. Muito provavelmente, eles queriam comparar 0/1 com uma variável Índice. Para corrigir o código, você precisa adicionar parênteses ao redor do operador ternário:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Aliás, o operador ternário é muito perigoso e provoca erros lógicos. Tenha muito cuidado com isso e não seja ganancioso com parênteses. Eu olhei este tópico com mais detalhes , 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<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;
}
....
}Aviso do PVS-Studio: [CWE-476] Pode ocorrer desreferenciação do ponteiro nulo 'LHS'. TGParser.cpp 2152
Se o ponteiro LHS for nulo, um aviso deverá ser emitido. No entanto, em vez disso, esse mesmo ponteiro nulo será desreferenciado: LHS->getAsString().
Esta é uma situação muito típica quando um erro está oculto em um manipulador de erros, já que ninguém o testa. Os analisadores estáticos verificam todos os códigos acessíveis, independentemente da frequência com que são usados. Este é um bom exemplo de como a análise estática complementa outras técnicas de teste e proteção contra erros.
Erro semelhante de manipulação de ponteiro RHS permitido no código abaixo: V522 [CWE-476] A desreferenciação do ponteiro nulo 'RHS' pode ocorrer. TGParser.cpp 2186
Fragmento N6: Usando o ponteiro 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);
}
....
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'ProgClone' pode ocorrer. Compilação incorreta.cpp 601
No início, um ponteiro inteligente ProgClone deixa de possuir o objeto:
BD.setNewProgram(std::move(ProgClone));Na verdade, agora ProgClone é um ponteiro nulo. Portanto, uma desreferência de ponteiro nulo deve ocorrer logo abaixo:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Mas, na realidade, isso não vai acontecer! Observe que o loop não é realmente executado.
No início do contêiner Funções mal compiladas limpo:
MiscompiledFunctions.clear();A seguir, o tamanho deste 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. Acho que isso também é um bug e o código deveria ser escrito de forma diferente.
Parece que encontramos aquela famosa paridade de erros! Um erro mascara outro :).
Fragmento N7: Usando o ponteiro após 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;
}
....
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Teste' pode ocorrer. Compilação incorreta.cpp 709
A mesma situação novamente. A princípio, o conteúdo do objeto é movido e depois ele é utilizado como se nada tivesse acontecido. Vejo essa situação cada vez com mais frequência no código do programa depois que a semântica de movimento apareceu em C++. É por isso que adoro a linguagem C++! Existem cada vez mais novas maneiras de atirar na 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);
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Tipo' pode ocorrer. PrettyFunctionDumper.cpp 233
Além dos manipuladores de erros, as funções de impressão de depuração geralmente não são testadas. Temos exatamente um caso assim diante de nós. A função fica à espera do usuário, que, ao invés de resolver seus problemas, será obrigado a corrigi-la.
Correto:
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());
....
}Aviso do PVS-Studio: V522 [CWE-476] A desreferenciação do ponteiro nulo 'Ty' pode ocorrer. SearchableTableEmitter.cpp 614
Acho que tudo está claro e não requer explicação.
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 do PVS-Studio: A variável 'Identificador->Tipo' é atribuída a si mesma. FormatTokenLexer.cpp 249
Não faz sentido atribuir uma variável a si mesma. 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: [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 quebrar. Você esqueceu de escrever mais alguma coisa aqui?
Fragmento N12: Verificando um ponteiro após desreferenciar
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: [CWE-476] O ponteiro 'Callee' foi utilizado antes de ser verificado em nullptr. Verifique as linhas: 172, 174. AMDGPUInline.cpp 172
Ponteiro Requerido no início é desreferenciado no momento em que a função é chamada obterTTI.
E então acontece que este ponteiro deve ser verificado quanto à igualdade nullptr:
if (!Callee || Callee->isDeclaration())Mas é muito tarde…
Fragmento N13 - N...: Verificando um ponteiro após desreferenciar
A situação discutida no fragmento de código anterior não é única. Aparece 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 em nullptr. Verifique 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 usado antes de ser verificado em nullptr. Verifique as linhas: 532, 534. SemaTemplateInstantiateDecl.cpp 532
E aqui:
- V595 [CWE-476] O ponteiro 'U' foi utilizado antes de ser verificado em nullptr. Verifique as linhas: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] O ponteiro 'ND' foi utilizado antes de ser verificado em nullptr. Verifique linhas: 2149, 2151. SemaTemplateInstantiate.cpp 2149
E então perdi o interesse em estudar os avisos com o número V595. Portanto, não sei se existem mais erros semelhantes além dos listados aqui. Muito 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: [CWE-190] Considere inspecionar a expressão '~(Size - 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
Pode não ser um bug e o código funciona exatamente como pretendido. Mas este é claramente um lugar muito suspeito e precisa ser verificado.
Digamos que a variável Tamanho é igual a 16, e então o autor do código planejou obtê-lo em uma variável Nimms valor:
1111111111111111111111111111111111111111111111111111111111100000
Porém, na realidade o resultado será:
0000000000000000000000000000000011111111111111111111111111100000
O fato é que todos os cálculos ocorrem no tipo não assinado de 32 bits. E só então, esse tipo não assinado de 32 bits será expandido implicitamente para uint64_t. Neste 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
Fragmento N19: palavra-chave ausente outro?
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: [CWE-670] Considere inspecionar a lógica do aplicativo. É possível que a palavra-chave 'else' esteja faltando. AMDGPUAsmParser.cpp 5655
Não há erro aqui. Desde o então bloco do primeiro if termina em continuar, então não importa, há uma palavra-chave outro ou não. De qualquer forma, o código funcionará da mesma forma. Ainda sinto falta outro torna o código mais confuso e perigoso. Se no futuro continuar desaparecer, o código começará a funcionar de maneira completamente diferente. Na minha opinião é melhor adicionar outro.
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 strings foram concatenadas, mas não são usadas. Considere inspecionar a expressão 'Result + Name.str()'. Símbolo.cpp 32
- V655 [CWE-480] As strings foram concatenadas, mas não são usadas. Considere inspecionar a expressão 'Result + "(Classe ObjC)" + Name.str()'. Símbolo.cpp 35
- V655 [CWE-480] As strings foram concatenadas, mas não são usadas. Considere inspecionar a expressão 'Result + "(ObjC Class EH) " + Name.str()'. Símbolo.cpp 38
- V655 [CWE-480] As strings foram concatenadas, mas não são usadas. Considere inspecionar a expressão 'Result + "(ObjC IVar)" + Name.str()'. Símbolo.cpp 41
Por acidente, o operador + é usado em vez do operador +=. O resultado são designs desprovidos de significado.
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 você mesmo o código perigoso. E esta é uma imagem para distrair a atenção para não olhar imediatamente para a resposta:
Aviso do PVS-Studio: [CWE-758] Uma 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 elemento Op não for encontrado, então um novo elemento é criado no mapa e o número de elementos neste mapa é escrito lá. Não se sabe se a função será chamada tamanho antes ou depois de adicionar um novo elemento.
Fragmento N22-N24: Atribuições repetidas
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: [CWE-563] A variável 'NType' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique linhas: 1663, 1664. MachOObjectFile.cpp 1664
Não creio que haja um erro real aqui. Apenas uma tarefa repetida desnecessária. Mas ainda assim um erro.
Da mesma forma:
- V519 [CWE-563] A variável 'B.NDesc' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique 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 vejamos uma versão ligeiramente diferente de reatribuição.
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;
....
}Aviso do PVS-Studio: 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 aparentemente contém um erro lógico. No início, variável Alinhamento um valor é atribuído dependendo da condição. E então a atribuição ocorre novamente, mas agora sem qualquer verificação.
Situações semelhantes podem ser vistas aqui:
- V519 [CWE-563] A variável ‘Effects’ recebe valores 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 sucessivamente. Talvez isso seja um erro. Verifique 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) // PAUSE instruction support // <=
break;
}
....
}Aviso do PVS-Studio: [CWE-571] A expressão 'nextByte != 0x90' é sempre verdadeira. X86DisassemblerDecoder.cpp 379
Verificar não faz sentido. Variável próximoByte sempre não é igual ao valor 0x90, que segue da verificação anterior. Isso é algum tipo de erro lógico.
Fragmento N29 - N...: Sempre condições verdadeiras/falsas
O analisador emite muitos avisos de que toda a condição () ou parte dele () é sempre verdadeiro ou falso. Freqüentemente, esses não são erros reais, mas simplesmente códigos desleixados, resultado de expansão de macro e coisas do gênero. No entanto, faz sentido observar todos esses avisos, uma vez que erros lógicos genuínos ocorrem de tempos em tempos. Por exemplo, esta seção do código é suspeita:
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: [CWE-570] Uma parte da expressão condicional é sempre falsa: RegNo == 0xe. ARMDisassembler.cpp 939
A constante 0xE é o valor 14 em decimal. Exame RegNo == 0xe não faz sentido porque se Nº Reg > 13, então a função completará sua execução.
Houve muitos outros avisos com os IDs V547 e V560, mas como acontece com , eu não estava interessado em estudar esses avisos. 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 utilizando o PVS-Studio.
Vou dar um exemplo de por que estudar esses gatilhos é chato. 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();
}
....
}Aviso do PVS-Studio: [CWE-670] Um 'retorno' incondicional dentro de um loop. R600OptimizeVectorRegisters.cpp 63
Este é um erro ou uma técnica específica que visa explicar algo aos programadores que leem o código. Este design não me explica nada e parece muito suspeito. É melhor não escrever assim :).
Cansado? Então é hora de fazer chá ou café.
Defeitos identificados por novos diagnósticos
Acho que 30 ativações de diagnósticos antigos são suficientes. Vamos agora ver que coisas interessantes podem ser encontradas com os novos diagnósticos que apareceram no analisador após Verificações. 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: [CWE-561] Código inacessível detectado. É possível que um erro esteja presente. ExecutionUtils.cpp 146
Como você pode ver, ambos os ramos da operadora if termina com uma chamada para a operadora retorno. Assim, o recipiente CtorDtorsByPriority nunca será apagado.
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 um erro esteja presente. LLParser.cpp 835
Situação interessante. Vejamos este lugar primeiro:
return ParseTypeIdEntry(SummaryID);
break;À primeira vista, parece que não há erro aqui. Parece que o operador quebrar há um extra aqui e você pode simplesmente excluí-lo. Porém, nem tudo é tão simples.
O analisador emite um aviso nas linhas:
Lex.setIgnoreColonInIdentifiers(false);
return false;E, de fato, esse código está inacessível. Todos os casos em interruptor termina com uma chamada da operadora retorno. E agora sem sentido sozinho quebrar não parece tão inofensivo! Talvez um dos ramos deva terminar com quebrarmas não em retorno?
Fragmento N33: Redefinição aleatória 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);
....
}Aviso do 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 tipo de retorno não assinado. Vamos calcular o valor da expressão, assumindo que a função retorna o valor 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Agora observe que a variável Tamanho dos dados tem um tipo não assinado de 64 bits. Acontece que ao executar a operação DataSize & 0xFFFFFFF8u, todos os trinta e dois bits de ordem superior serão redefinidos para zero. Muito provavelmente, não era isso que o programador queria. Suspeito que ele queria calcular: DataSize & 0xFFFFFFFFFFFFFFF8u.
Para corrigir o erro, você deve escrever isto:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Ou então:
DataSize &= ~(getStubAlignment() - 1ULL);Fragmento N34: Falha na conversão de tipo explícito
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: [CWE-190] Possível estouro. Considere converter operandos do operador 'NumElts * Scale' para o tipo 'size_t', não para o resultado. X86ISelLowering.h 1577
A conversão de tipo explícito é usada para evitar estouro ao multiplicar variáveis de tipo int. No entanto, a conversão explícita de tipo aqui não protege contra overflow. Primeiro, as variáveis serão multiplicadas, e só então o resultado da multiplicação de 32 bits será expandido para o tipo .
Fragmento N35: Falha ao copiar e colar
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] Foram encontrados dois fragmentos de código semelhantes. Talvez este seja um erro de digitação e a variável 'Op1' deva ser usada em vez de 'Op0'. InstCombineCompares.cpp 5507
Este novo e interessante diagnóstico identifica situações em que um trecho de código foi copiado e alguns nomes nele contidos começaram a ser alterados, mas em um local não foram corrigidos.
Observe que no segundo bloco eles mudaram Op0 em Op1. Mas em um lugar eles não consertaram. Muito 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 Variável
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Aviso do PVS-Studio: [CWE-563] A variável 'Mode' é atribuída, mas não é usada no final da função. SIModeRegister.cpp 48
É muito perigoso dar aos argumentos de função os mesmos nomes dos membros da classe. É muito fácil ficar confuso. Temos exatamente um caso assim diante de nós. Esta expressão não faz sentido:
Mode &= Mask;O argumento da função muda. Isso é tudo. Este argumento não é mais usado. Provavelmente você deveria ter escrito assim:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragmento N37: Confusão Variável
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;
}Aviso PVS-Studio: V1001 [CWE-563] A variável 'Size' é atribuída, mas não é usada no final da função. Objeto.cpp 424
A situação é semelhante à anterior. Deve ser escrito:
this->Size += this->EntrySize;Fragmento N38-N47: Esqueceram de verificar o índice
Anteriormente, vimos exemplos de desencadeamento diagnóstico . Sua essência é que o ponteiro seja desreferenciado no início e só então verificado. Diagnóstico jovem tem o significado oposto, mas também revela muitos erros. Identifica situações em que o ponteiro foi verificado no início e depois esquecido de fazê-lo. Vejamos 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 de forma insegura após ser verificado em relação ao nullptr. Verifique as linhas: 729, 738. TargetTransformInfoImpl.h 738
Variável Ptr pode ser igual nullptr, conforme evidenciado pela verificação:
if (Ptr != nullptr)No entanto, abaixo deste ponteiro é desreferenciado sem verificação preliminar:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Vamos considerar 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 de forma insegura após ser verificado em relação ao nullptr. Verifique linhas: 3228, 3231. CGDebugInfo.cpp 3231
Preste atenção ao sinal FD. Tenho certeza de que o problema é claramente visível e nenhuma explicação especial é necessária.
E mais:
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 de forma insegura após ser verificado em relação ao nullptr. Verifique linhas: 960, 965. InterleavedLoadCombinePass.cpp 965
Como se proteger de tais erros? Fique mais atento ao Code-Review e use o analisador estático PVS-Studio para verificar regularmente seu código.
Não adianta citar 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 de forma insegura após ser verificado em nullptr. Verifique as linhas: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] O ponteiro 'PI' foi usado de forma insegura após ser verificado em nullptr. Verifique linhas: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] O ponteiro 'StatepointCall' foi usado de forma insegura após ser verificado em nullptr. Verifique linhas: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] O ponteiro 'RV' foi usado de forma insegura após ser verificado em nullptr. Verifique linhas: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] O ponteiro 'CalleeFn' foi usado de forma insegura após ser verificado em nullptr. Verifique linhas: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] O ponteiro 'TC' foi usado de forma insegura após ser verificado em nullptr. Verifique linhas: 1819, 1824. Driver.cpp 1824
Fragmento N48-N60: Não é crítico, mas é 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()));
....
}Aviso do PVS-Studio: [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Estratégias' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-isel-fuzzer.cpp 58
Para adicionar um elemento ao final de um contêiner como std::vetor > você não pode simplesmente escrever xxx.push_back(novo X), uma vez que não há conversão implícita de X* в std::unique_ptr.
Uma solução comum é escrever xxx.emplace_back(novo X)já que compila: método emplace_back constrói um elemento diretamente a partir de seus argumentos e pode, portanto, usar construtores explícitos.
Não é seguro. Se o vetor estiver cheio, a memória será realocada. A operação de realocação de memória pode falhar, resultando no lançamento de uma exceção std :: bad_alloc. Neste caso, o ponteiro será perdido e o objeto criado nunca será excluído.
Uma solução segura é criar único_ptrque será o proprietário do ponteiro antes que o vetor tente realocar a memória:
xxx.push_back(std::unique_ptr<X>(new X))Desde C++ 14, você pode usar 'std::make_unique':
xxx.push_back(std::make_unique<X>())Este tipo de defeito não é crítico para o LLVM. Se a memória não puder ser alocada, o compilador simplesmente irá parar. No entanto, para aplicações com longos , que não pode ser encerrado se a alocação de memória falhar, isso pode ser um bug realmente desagradável.
Portanto, embora esse código não represente uma ameaça prática ao LLVM, achei útil falar sobre esse padrão de erro e que o analisador PVS-Studio aprendeu a identificá-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á no 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á no 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á no 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á no 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á no 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á no 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á no caso de uma exceção. MóduloMap.cpp 810
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Objetos' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. DebugMap.cpp 88
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Estratégias' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 685
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 686
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 688
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 689
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 690
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 691
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 692
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 693
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Modificadores' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. llvm-stress.cpp 694
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Operandos' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Stash' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Um ponteiro sem proprietário é adicionado ao contêiner 'Matchers' pelo método 'emplace_back'. Um vazamento de memória ocorrerá no caso de uma exceção. GlobalISelEmitter.cpp 2702
Conclusão
Emiti 60 avisos no total e depois parei. Existem outros defeitos que o analisador PVS-Studio detecta no LLVM? Sim, eu tenho. No entanto, quando eu estava escrevendo fragmentos de código para o artigo, já era tarde da noite, ou melhor, noite, e decidi que era hora de encerrar o dia.
Espero que você tenha achado interessante e queira experimentar o analisador PVS-Studio.
Você pode baixar o analisador e obter a chave do caça-minas em .
Mais importante ainda, use a análise estática regularmente. Verificações únicas, realizados 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 confiabilidade do seu código!
Se você quiser compartilhar este artigo com um público que fala inglês, use o link de tradução: Andrey Karpov. .
Fonte: habr.com