Han pasado más de dos años desde la última verificación del código del proyecto LLVM utilizando nuestro analizador PVS-Studio. Asegurémonos de que el analizador PVS-Studio siga siendo una herramienta líder para identificar errores y posibles vulnerabilidades. Para hacer esto, verificaremos y encontraremos nuevos errores en la versión LLVM 8.0.0.
Artículo por escribir
Para ser honesto, no quería escribir este artículo. No es interesante escribir sobre un proyecto que ya hemos revisado varias veces (, , ). Es mejor escribir sobre algo nuevo, pero no tengo otra opción.
Cada vez que se lanza o actualiza una nueva versión de LLVM , recibimos preguntas del siguiente tipo en nuestro correo:
¡Mira, la nueva versión de Clang Static Analyzer ha aprendido a encontrar nuevos errores! Me parece que la relevancia del uso de PVS-Studio está disminuyendo. Clang encuentra más errores que antes y se pone al día con las capacidades de PVS-Studio. ¿Qué piensas sobre esto?
A esto siempre quiero responder algo como:
¡Nosotros tampoco nos quedamos de brazos cruzados! Hemos mejorado significativamente las capacidades del analizador PVS-Studio. Así que no os preocupéis, seguimos liderando como hasta ahora.
Desafortunadamente, esta es una mala respuesta. No hay pruebas en ello. Y es por eso que estoy escribiendo este artículo ahora. Entonces, el proyecto LLVM se revisó una vez más y se encontraron varios errores en él. Ahora demostraré los que me parecieron interesantes. Clang Static Analyzer no puede encontrar estos errores (o es extremadamente inconveniente hacerlo con su ayuda). Pero nosotros podemos. Además, encontré y anoté todos estos errores en una noche.
Pero escribir el artículo me llevó varias semanas. Simplemente no me atrevía a poner todo esto en texto :).
Por cierto, si está interesado en saber qué tecnologías se utilizan en el analizador PVS-Studio para identificar errores y posibles vulnerabilidades, le sugiero que se familiarice con esto. .
Diagnósticos nuevos y antiguos.
Como ya se señaló, hace unos dos años se revisó nuevamente el proyecto LLVM y se corrigieron los errores encontrados. Ahora este artículo presentará una nueva tanda de errores. ¿Por qué se encontraron nuevos errores? Hay 3 razones para esto:
- El proyecto LLVM está evolucionando, cambiando el código antiguo y agregando código nuevo. Naturalmente, hay nuevos errores en el código modificado y escrito. Esto demuestra claramente que el análisis estático debe utilizarse con regularidad y no ocasionalmente. Nuestros artículos muestran bien las capacidades del analizador PVS-Studio, pero esto no tiene nada que ver con mejorar la calidad del código y reducir el costo de corregir errores. ¡Utilice un analizador de código estático con regularidad!
- Estamos ultimando y mejorando los diagnósticos existentes. Por lo tanto, el analizador puede identificar errores que no detectó durante análisis anteriores.
- En PVS-Studio han aparecido nuevos diagnósticos que no existían hace 2 años. Decidí resaltarlos en una sección separada para mostrar claramente el desarrollo de PVS-Studio.
Defectos identificados por diagnósticos que existieron hace 2 años.
Fragmento N1: Copiar y pegar
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
....
}Advertencia de PVS-Studio: [CWE-570] Hay subexpresiones idénticas 'Name.startswith("avx512.mask.permvar.")' a la izquierda y a la derecha de '||' operador. Actualización automática.cpp 73
Se comprueba dos veces que el nombre comience con la subcadena "avx512.mask.permvar". En la segunda comprobación, obviamente querían escribir algo más, pero se olvidaron de corregir el texto copiado.
Fragmento N2: error tipográfico
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;
....
}Advertencia PVS-Studio: V501 Hay subexpresiones idénticas 'CXNameRange_WantQualifier' a la izquierda y a la derecha de '|' operador. CIndex.cpp 7245
Debido a un error tipográfico, la misma constante con el mismo nombre se utiliza dos veces CXNameRange_WantQualifier.
Fragmento N3: Confusión con la precedencia de operadores
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Advertencia de PVS-Studio: [CWE-783] Quizás el operador '?:' funcione de una manera diferente a la esperada. El operador '?:' tiene una prioridad menor que el operador '=='. PPCTargetTransformInfo.cpp 404
En mi opinión, este es un error muy hermoso. Sí, sé que tengo ideas extrañas sobre la belleza :).
Ahora, según , la expresión se evalúa de la siguiente manera:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Desde un punto de vista práctico, tal condición no tiene sentido, ya que puede reducirse a:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Este es un claro error. Lo más probable es que querían comparar 0/1 con una variable. Home. Para corregir el código, debe agregar paréntesis alrededor del operador ternario:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Por cierto, el operador ternario es muy peligroso y provoca errores lógicos. Ten mucho cuidado con esto y no seas codicioso con los paréntesis. Miré este tema con más detalle. , en el capítulo “Cuidado con el operador ?: Enciérrelo entre paréntesis”.
Fragmento N4, N5: puntero 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;
}
....
}Advertencia de PVS-Studio: [CWE-476] Es posible que se produzca una desreferenciación del puntero nulo 'LHS'. TGParser.cpp 2152
si el puntero LHS es nulo, se debe emitir una advertencia. Sin embargo, en su lugar, se eliminará la referencia a este mismo puntero nulo: LHS->getAsString().
Esta es una situación muy típica cuando un error está oculto en un controlador de errores, ya que nadie los prueba. Los analizadores estáticos comprueban todo el código accesible, sin importar la frecuencia con la que se utilice. Este es un muy buen ejemplo de cómo el análisis estático complementa otras técnicas de prueba y protección contra errores.
Error de manejo de puntero similar RHS permitido en el código justo debajo: V522 [CWE-476] Es posible que se produzca la desreferenciación del puntero nulo 'RHS'. TGParser.cpp 2186
Fragmento N6: Usando el puntero después de moverse
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);
}
....
}Advertencia de PVS-Studio: V522 [CWE-476] Es posible que se produzca una desreferenciación del puntero nulo 'ProgClone'. Mala compilación.cpp 601
Al principio un puntero inteligente. ProgClon deja de ser propietario del objeto:
BD.setNewProgram(std::move(ProgClone));De hecho, ahora ProgClon es un puntero nulo. Por lo tanto, debería producirse una desreferencia de puntero nulo justo debajo:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Pero, en realidad, ¡esto no sucederá! Tenga en cuenta que el bucle en realidad no se ejecuta.
Al inicio del contenedor Funciones mal compiladas despejado:
MiscompiledFunctions.clear();A continuación, el tamaño de este contenedor se utiliza en la condición de bucle:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Es fácil ver que el ciclo no comienza. Creo que esto también es un error y el código debería escribirse de manera diferente.
¡Parece que nos hemos topado con esa famosa paridad de errores! Un error enmascara otro :).
Fragmento N7: Usando el puntero después de moverse
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;
}
....
}Advertencia de PVS-Studio: V522 [CWE-476] Es posible que se produzca una desreferenciación del puntero nulo 'Prueba'. Mala compilación.cpp 709
Otra vez la misma situación. Al principio se mueve el contenido del objeto y luego se utiliza como si nada hubiera pasado. Veo esta situación cada vez más a menudo en el código de programa después de que apareció la semántica de movimiento en C++. ¡Por eso amo el lenguaje C++! Cada vez hay más formas nuevas de dispararte una pierna. El analizador PVS-Studio siempre funcionará :).
Fragmento N8: puntero 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);
}Advertencia de PVS-Studio: V522 [CWE-476] Es posible que se produzca una desreferenciación del puntero nulo 'Tipo'. PrettyFunctionDumper.cpp 233
Además de los controladores de errores, las funciones de impresión de depuración normalmente no se prueban. Tenemos ante nosotros un caso así. La función está esperando al usuario, quien, en lugar de solucionar sus problemas, se verá obligado a solucionarlo.
Correcto:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragmento N9: puntero 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());
....
}Advertencia de PVS-Studio: V522 [CWE-476] Es posible que se produzca una desreferenciación del puntero nulo 'Ty'. SearchableTableEmitter.cpp 614
Creo que todo está claro y no requiere explicación.
Fragmento N10: error tipográfico
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;
}Advertencia de PVS-Studio: La variable 'Identificador->Tipo' se asigna a sí misma. FormatTokenLexer.cpp 249
No tiene sentido asignarse una variable a sí mismo. Lo más probable es que quisieran escribir:
Identifier->Type = Question->Type;Fragmento N11: Rotura sospechosa
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Advertencia de PVS-Studio: [CWE-478] Considere inspeccionar la declaración de 'cambio'. Es posible que falte el primer operador 'caso'. SystemZAsmParser.cpp 652
Hay un operador muy sospechoso al principio. romper. ¿Olvidaste escribir algo más aquí?
Fragmento N12: Comprobación de un puntero después de la desreferenciación
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");
....
}Advertencia de PVS-Studio: [CWE-476] El puntero 'Callee' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 172, 174. AMDGPUInline.cpp 172
Puntero Llamado al principio se desreferencia en el momento en que se llama a la función obtenerTTI.
Y luego resulta que se debe verificar la igualdad de este puntero. punto nulo:
if (!Callee || Callee->isDeclaration())Pero es muy tarde…
Fragmento N13 - N...: Comprobando un puntero después de la desreferenciación
La situación analizada en el fragmento de código anterior no es única. Aparece aquí:
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()) { // <=
....
}Advertencia de PVS-Studio: V595 [CWE-476] El puntero 'CalleeFn' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 1079, 1081. SimplifyLibCalls.cpp 1079
Y aquí:
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()); // <=
....
}Advertencia de PVS-Studio: V595 [CWE-476] El puntero 'ND' se usó antes de verificarlo con nullptr. Líneas de verificación: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Y aquí:
- V595 [CWE-476] El puntero 'U' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] El puntero 'ND' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Y luego dejé de interesarme por estudiar las advertencias con el número V595. Así que no sé si hay más errores similares además de los que se enumeran aquí. Lo más probable es que lo haya.
Fragmento N17, N18: Cambio sospechoso
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Advertencia de PVS-Studio: [CWE-190] Considere inspeccionar la expresión '~(Tamaño - 1) << 1'. Desplazamiento de bits del valor de 32 bits con una posterior expansión al tipo de 64 bits. AArch64AddressingModes.h 260
Puede que no sea un error y que el código funcione exactamente como se esperaba. Pero este es claramente un lugar muy sospechoso y es necesario comprobarlo.
digamos la variable Tamaño es igual a 16, y luego el autor del código planeó obtenerlo en una variable NImms valor:
1111111111111111111111111111111111111111111111111111111111100000
Sin embargo, en realidad el resultado será:
0000000000000000000000000000000011111111111111111111111111100000
El hecho es que todos los cálculos se realizan utilizando el tipo sin signo de 32 bits. Y sólo entonces, este tipo sin firmar de 32 bits se expandirá implícitamente a uint64_t. En este caso, los bits más significativos serán cero.
Puedes arreglar la situación de esta manera:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Situación similar: V629 [CWE-190] Considere inspeccionar la expresión 'Immr << 6'. Desplazamiento de bits del valor de 32 bits con una posterior expansión al tipo de 64 bits. AArch64AddressingModes.h 269
Fragmento N19: Palabra clave faltante más?
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");
}
....
}Advertencia de PVS-Studio: [CWE-670] Considere inspeccionar la lógica de la aplicación. Es posible que falte la palabra clave "else". AMDGPUAsmParser.cpp 5655
No hay ningún error aquí. Desde el entonces bloque de la primera if termina con continue, entonces no importa, hay una palabra clave más O no. De cualquier manera, el código funcionará igual. todavía perdido más hace que el código sea más confuso y peligroso. Si en el futuro continue desaparece, el código comenzará a funcionar de manera completamente diferente. En mi opinión es mejor agregar más.
Fragmento N20: Cuatro errores tipográficos del mismo 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;
}Advertencias de PVS-Studio:
- V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + Nombre.str()'. Símbolo.cpp 32
- V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(Clase ObjC)" + Nombre.str()'. Símbolo.cpp 35
- V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(ObjC Class EH) " + Name.str()'. Símbolo.cpp 38
- V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(ObjC IVar)" + Nombre.str()'. Símbolo.cpp 41
Por accidente, se utiliza el operador + en lugar del operador +=. El resultado son diseños carentes de significado.
Fragmento N21: Comportamiento 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();
}
}
}Intente encontrar el código peligroso usted mismo. Y esta es una imagen para distraer la atención y no mirar inmediatamente la respuesta:
Advertencia de PVS-Studio: [CWE-758] Se utiliza una construcción peligrosa: 'FeaturesMap[Op] = FeaturesMap.size()', donde 'FeaturesMap' es de la clase 'mapa'. Esto puede conducir a un comportamiento indefinido. RISCVCompressInstEmitter.cpp 490
Línea de problema:
FeaturesMap[Op] = FeaturesMap.size();si elemento Op no se encuentra, entonces se crea un nuevo elemento en el mapa y el número de elementos en este mapa se escribe allí. Simplemente se desconoce si se llamará a la función. tamaño antes o después de agregar un nuevo elemento.
Fragmento N22-N24: Asignaciones 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;
}
....
}Advertencia de PVS-Studio: [CWE-563] A la variable 'NType' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 1663, 1664. MachOObjectFile.cpp 1664
No creo que haya un verdadero error aquí. Sólo una tarea repetida innecesaria. Pero sigue siendo un error garrafal.
Del mismo modo:
- V519 [CWE-563] A la variable 'B.NDesc' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] A la variable se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 59, 61. coff2yaml.cpp 61
Fragmento N25-N27: Más reasignaciones
Ahora veamos una versión ligeramente diferente de reasignación.
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;
....
}Advertencia de PVS-Studio: V519 [CWE-563] A la variable 'Alineación' se le asignan valores dos veces consecutivas. Quizás esto sea un error. Líneas de verificación: 1158, 1160. LoadStoreVectorizer.cpp 1160
Este es un código muy extraño que aparentemente contiene un error lógico. Al principio variable Alineación Se asigna un valor dependiendo de la condición. Y luego la asignación vuelve a ocurrir, pero ahora sin ningún control.
Situaciones similares se pueden ver aquí:
- V519 [CWE-563] A la variable 'Efectos' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] A la variable 'ExpectNoDerefChunk' se le asignan valores dos veces consecutivas. Quizás esto sea un error. Líneas de verificación: 4970, 4973. SemaType.cpp 4973
Fragmento N28: Condición siempre verdadera
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;
}
....
}Advertencia de PVS-Studio: [CWE-571] La expresión 'nextByte! = 0x90' siempre es verdadera. X86DisassemblerDecoder.cpp 379
Comprobar no tiene sentido. Variable byte siguiente siempre no es igual al valor 0x90, que se desprende del control anterior. Esto es una especie de error lógico.
Fragmento N29 - N...: Siempre condiciones verdaderas/falsas
El analizador emite muchas advertencias de que toda la condición () o parte del mismo () siempre es verdadero o falso. A menudo, estos no son errores reales, sino simplemente código descuidado, el resultado de una expansión macro y similares. Sin embargo, tiene sentido tener en cuenta todas estas advertencias, ya que de vez en cuando se producen errores lógicos genuinos. Por ejemplo, esta sección de código es sospechosa:
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;
....
}Advertencia de PVS-Studio: [CWE-570] Una parte de la expresión condicional siempre es falsa: RegNo == 0xe. ARMDisassembler.cpp 939
La constante 0xE es el valor 14 en decimal. Examen Número de registro == 0xe no tiene sentido porque si Número de registro > 13, entonces la función completará su ejecución.
Hubo muchas otras advertencias con los ID V547 y V560, pero al igual que con , No estaba interesado en estudiar estas advertencias. Ya estaba claro que tenía material suficiente para escribir un artículo :). Por lo tanto, se desconoce cuántos errores de este tipo se pueden identificar en LLVM utilizando PVS-Studio.
Te daré un ejemplo de por qué estudiar estos factores desencadenantes es aburrido. El analizador tiene toda la razón al emitir una advertencia para el siguiente código. Pero esto no es un error.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Advertencia de PVS-Studio: V547 [CWE-570] La expresión '!HasError' siempre es falsa. UnwrappedLineParser.cpp 1635
Fragmento N30: Devolución sospechosa
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();
}
....
}Advertencia de PVS-Studio: [CWE-670] Un 'retorno' incondicional dentro de un bucle. R600OptimizeVectorRegisters.cpp 63
Esto es un error o una técnica específica destinada a explicar algo a los programadores que leen el código. Este diseño no me explica nada y parece muy sospechoso. Es mejor no escribir así :).
¿Cansado? Entonces es hora de preparar té o café.
Defectos identificados por nuevos diagnósticos.
Creo que 30 activaciones de diagnósticos antiguos son suficientes. Veamos ahora qué cosas interesantes se pueden encontrar con los nuevos diagnósticos que aparecieron en el analizador después cheques. En total, durante este tiempo se agregaron 66 diagnósticos de uso general al analizador de C++.
Fragmento N31: Código inalcanzable
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();
}Advertencia de PVS-Studio: [CWE-561] Se detectó código inalcanzable. Es posible que haya un error. EjecuciónUtils.cpp 146
Como puede ver, ambas ramas del operador. if termina con una llamada al operador volvemos. En consecuencia, el contenedor DirectoresPorPrioridad nunca será aclarado.
Fragmento N32: Código inalcanzable
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;
}Advertencia de PVS-Studio: V779 [CWE-561] Se detectó un código inalcanzable. Es posible que haya un error. LLParser.cpp 835
Interesante situación. Veamos este lugar primero:
return ParseTypeIdEntry(SummaryID);
break;A primera vista, parece que aquí no hay ningún error. Parece el operador. romper Hay uno adicional aquí y simplemente puedes eliminarlo. Sin embargo, no todo es tan sencillo.
El analizador emite una advertencia en las líneas:
Lex.setIgnoreColonInIdentifiers(false);
return false;Y, de hecho, este código es inalcanzable. Todos los casos en cambiar termina con una llamada del operador volvemos. Y ahora sin sentido solo romper ¡No parece tan inofensivo! Quizás una de las ramas debería terminar con romperpero no en volvemos?
Fragmento N33: reinicio aleatorio 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);
....
}Advertencia de PVS-Studio: El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto provocará la pérdida de bits superiores. Tiempo de ejecuciónDyld.cpp 815
Tenga en cuenta que la función getStubAlignment tipo de devoluciones no firmado. Calculemos el valor de la expresión, asumiendo que la función devuelve el valor 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Ahora observe que la variable Tamaño de datos tiene un tipo sin firmar de 64 bits. Resulta que al realizar la operación DataSize & 0xFFFFFFF8u, los treinta y dos bits de orden superior se restablecerán a cero. Lo más probable es que esto no sea lo que quería el programador. Sospecho que quería calcular: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Para corregir el error, debes escribir esto:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);O así:
DataSize &= ~(getStubAlignment() - 1ULL);Fragmento N34: conversión de tipo explícita fallida
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);
....
}Advertencia de PVS-Studio: [CWE-190] Posible desbordamiento. Considere convertir los operandos del operador 'NumElts * Scale' al tipo 'size_t', no el resultado. X86ISelLowering.h 1577
La conversión de tipos explícita se utiliza para evitar el desbordamiento al multiplicar variables de tipo. int. Sin embargo, la conversión de tipos explícita aquí no protege contra el desbordamiento. Primero, las variables se multiplicarán, y solo entonces el resultado de la multiplicación de 32 bits se expandirá al tipo .
Fragmento N35: Copiar y pegar fallido
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] Se encontraron dos fragmentos de código similares. Quizás se trate de un error tipográfico y debería utilizarse la variable 'Op1' en lugar de 'Op0'. InstCombineCompares.cpp 5507
Este nuevo e interesante diagnóstico identifica situaciones en las que se ha copiado un fragmento de código y se han empezado a cambiar algunos nombres en él, pero en un lugar no lo han corregido.
Tenga en cuenta que en el segundo bloque cambiaron. Op0 en Op1. Pero en un lugar no lo arreglaron. Probablemente debería haberse escrito así:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragmento N36: Confusión variable
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Advertencia de PVS-Studio: [CWE-563] La variable 'Modo' se asigna pero no se utiliza al final de la función. SIModeRegister.cpp 48
Es muy peligroso dar a los argumentos de una función los mismos nombres que a los miembros de la clase. Es muy fácil confundirse. Tenemos ante nosotros un caso así. Esta expresión no tiene sentido:
Mode &= Mask;El argumento de la función cambia. Eso es todo. Este argumento ya no se utiliza. Lo más probable es que deberías haberlo escrito así:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragmento N37: Confusión variable
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;
}Advertencia PVS-Studio: V1001 [CWE-563] La variable 'Tamaño' está asignada pero no se utiliza al final de la función. Objeto.cpp 424
La situación es similar a la anterior. Debería escribirse:
this->Size += this->EntrySize;Fragmento N38-N47: Se olvidaron de revisar el índice
Anteriormente, analizamos ejemplos de desencadenantes de diagnóstico. . Su esencia es que se elimina la referencia al puntero al principio y solo luego se verifica. Diagnóstico joven Tiene el significado opuesto, pero también revela muchos errores. Identifica situaciones en las que se comprobó el puntero al principio y luego se olvidó de hacerlo. Veamos los casos que se encuentran dentro de 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()); // <=
....
}Advertencia de PVS-Studio: V1004 [CWE-476] El puntero 'Ptr' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 729, 738. TargetTransformInfoImpl.h 738
Variable ptr puede ser igual punto nulo, como lo demuestra el cheque:
if (Ptr != nullptr)Sin embargo, a continuación se elimina la referencia a este indicador sin verificación previa:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Consideremos otro caso similar.
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(); // <=
....
}Advertencia de PVS-Studio: V1004 [CWE-476] El puntero 'FD' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 3228, 3231. CGDebugInfo.cpp 3231
Presta atención a la señal. FD. Estoy seguro de que el problema es claramente visible y no se requiere ninguna explicación especial.
Y más:
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()); // <=
....
}Advertencia de PVS-Studio: V1004 [CWE-476] El puntero 'PtrTy' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 960, 965. InterleavedLoadCombinePass.cpp 965
¿Cómo protegerse de tales errores? Esté más atento a Code-Review y utilice el analizador estático PVS-Studio para comprobar periódicamente su código.
No tiene sentido citar otros fragmentos de código con errores de este tipo. Dejaré solo una lista de advertencias en el artículo:
- V1004 [CWE-476] El puntero 'Expr' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] El puntero 'PI' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] El puntero 'StatepointCall' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] El puntero 'RV' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] El puntero 'CalleeFn' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] El puntero 'TC' se usó de manera insegura después de verificarlo con nullptr. Líneas de verificación: 1819, 1824. Driver.cpp 1824
Fragmento N48-N60: no es crítico, pero es un defecto (posible pérdida de memoria)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Advertencia de PVS-Studio: [CWE-460] Se agrega un puntero sin propietario al contenedor 'Estrategias' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-isel-fuzzer.cpp 58
Para agregar un elemento al final de un contenedor como estándar::vector > no puedes simplemente escribir xxx.push_back(nueva X), ya que no hay ninguna conversión implícita de X* в std::único_ptr.
Una solución común es escribir xxx.emplace_back(nueva X)ya que compila: método emplace_back construye un elemento directamente a partir de sus argumentos y, por lo tanto, puede utilizar constructores explícitos.
No es seguro. Si el vector está lleno, se reasigna la memoria. La operación de reasignación de memoria puede fallar, lo que genera una excepción std :: bad_alloc. En este caso, el puntero se perderá y el objeto creado nunca se eliminará.
Una solución segura es crear único_ptrque será propietario del puntero antes de que el vector intente reasignar la memoria:
xxx.push_back(std::unique_ptr<X>(new X))Desde C++ 14, puedes usar 'std::make_unique':
xxx.push_back(std::make_unique<X>())Este tipo de defecto no es crítico para LLVM. Si no se puede asignar memoria, el compilador simplemente se detendrá. Sin embargo, para aplicaciones con largas , que no puede finalizar simplemente si falla la asignación de memoria, esto puede ser un error realmente desagradable.
Entonces, aunque este código no representa una amenaza práctica para LLVM, me resultó útil hablar sobre este patrón de error y que el analizador PVS-Studio ha aprendido a identificarlo.
Otras advertencias de este tipo:
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Pases' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. PassManager.h 546
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'AA' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. AliasAnalysis.h 324
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Entradas' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'AllEdges' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. CFGMST.h 268
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'VMaps' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Registros' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. FDRLogBuilder.h 30
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'PendingSubmodules' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. ModuleMap.cpp 810
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Objetos' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. DebugMap.cpp 88
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Estrategias' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 685
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 686
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 688
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 689
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 690
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 691
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 692
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 693
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Modificadores' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. llvm-estrés.cpp 694
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Operandos' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Stash' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Se agrega un puntero sin propietario al contenedor 'Matchers' mediante el método 'emplace_back'. En caso de una excepción, se producirá una pérdida de memoria. GlobalISelEmitter.cpp 2702
Conclusión
Emití 60 advertencias en total y luego paré. ¿Existen otros defectos que el analizador PVS-Studio detecta en LLVM? Sí tengo. Sin embargo, cuando estaba escribiendo fragmentos de código para el artículo, ya era tarde, o incluso de noche, y decidí que era hora de dar por terminado el día.
Espero que te haya resultado interesante y quieras probar el analizador PVS-Studio.
Puede descargar el analizador y obtener la clave del buscaminas en .
Lo más importante es utilizar el análisis estático con regularidad. Cheques únicos, realizados por nosotros para popularizar la metodología de análisis estático y PVS-Studio no son un escenario normal.
¡Buena suerte mejorando la calidad y confiabilidad de su código!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Andrey Karpov. .
Fuente: habr.com