Geekly articles weekly

Encuentre errores en LLVM 8 utilizando el analizador PVS-Studio

PVS-Studio y LLVM 8.0.0

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 sigue siendo la herramienta líder para detectar errores y vulnerabilidades potenciales. Para hacer esto, verifique y encuentre nuevos errores en la versión LLVM 8.0.0.

Artículo a escribir


Para ser sincero, no quería escribir este artículo. No es interesante escribir sobre un proyecto que ya hemos probado repetidamente ( 1 , 2 , 3 ). Es mejor escribir sobre algo nuevo, pero no tengo otra opción.

Cada vez que se lanza una nueva versión de LLVM o se actualiza Clang Static Analyzer , aparecen las siguientes preguntas 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é opinas sobre esto?

A esto, siempre quiero responder algo en el espíritu:

¡Tampoco estamos sentados ociosos! Hemos mejorado significativamente las capacidades del analizador PVS-Studio. Así que no te preocupes, seguimos liderando, como antes.

Lamentablemente, 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 ha sido probado nuevamente y se han encontrado una variedad de errores. Los que me parecieron interesantes, ahora lo demostraré. El Analizador estático de Clang no puede encontrar estos errores (o es extremadamente inconveniente hacerlo). Y nosotros podemos. Y encontré y escribí todos estos errores en una noche.

Pero la redacción del artículo se prolongó durante varias semanas. No pude obligarme a organizar todo esto en forma de texto :).

Por cierto, si está interesado en qué tecnologías se utilizan en el analizador PVS-Studio para detectar errores y vulnerabilidades potenciales, le sugiero que se familiarice con esta nota .

Diagnósticos nuevos y viejos


Como ya se señaló, hace aproximadamente dos años, el proyecto LLVM se verificó una vez más y se corrigieron los errores encontrados. Ahora en este artículo se presentará una nueva porción de errores. ¿Por qué se encontraron nuevos errores? Hay 3 razones para esto:

  1. El proyecto LLVM se desarrolla, el código antiguo cambia y aparece uno nuevo. Naturalmente, hay nuevos errores en el código modificado y escrito. Esto demuestra bien que el análisis estático debe aplicarse regularmente, y no de un caso a otro. 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. ¡Use un analizador de código estático regularmente!
  2. Estamos finalizando y mejorando los diagnósticos existentes. Por lo tanto, el analizador puede detectar errores que no se notaron durante las comprobaciones anteriores.
  3. PVS-Studio introdujo nuevos diagnósticos, que no eran hace 2 años. Decidí separarlos en una sección separada para demostrar claramente el desarrollo de PVS-Studio.

Defectos identificados por diagnósticos que existían hace 2 años


Fragmento N1: Copiar-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: V501 [CWE-570] Hay subexpresiones idénticas 'Name.startswith ("avx512.mask.permvar.")' A la izquierda y a la derecha de '||' operador AutoUpgrade.cpp 73

Se verifica dos veces que el nombre comienza con la subcadena "avx512.mask.permvar". En la segunda prueba, claramente querían escribir algo más, pero olvidaron arreglar 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 de 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 nombre CXNameRange_WantQualifier se usa dos veces .

Fragmento N3: confusión sobre las prioridades del operador

 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: V502 [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, de acuerdo con las prioridades de los operadores , la expresión se calcula de la siguiente manera:

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

Desde 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 0/1 quisiera comparar con la variable Índice . Para arreglar el código, agregue corchetes 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 eso y no seas codicioso para poner paréntesis. Considere este tema con más detalle aquí en el capítulo "¿Temer al operador?: Y encerrarlo 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: V522 [CWE-476] Puede tener lugar la desreferenciación del puntero nulo 'LHS'. TGParser.cpp 2152

Si el puntero LHS es nulo, se debe emitir una advertencia. Sin embargo, en cambio, esto desreferenciará este puntero muy nulo: LHS-> getAsString () .

Esta es una situación muy típica cuando el error está oculto en el controlador de errores, ya que nadie los está probando. Los analizadores estáticos verifican todo el código accesible, sin importar con qué frecuencia se use. Este es un muy buen ejemplo de cómo el análisis estático complementa otras pruebas y técnicas de protección contra errores.

Se produce un error similar al procesar el puntero RHS en el siguiente código: V522 [CWE-476] Puede producirse una desreferenciación del puntero nulo 'RHS'. TGParser.cpp 2186

Fragmento N6: Usando el cursor después de 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); } .... }

Advertencia de PVS-Studio: V522 [CWE-476] Puede tener lugar la desreferenciación del puntero nulo 'ProgClone'. Miscompilation.cpp 601

Al principio, el puntero inteligente ProgClone deja de ser dueño del objeto:

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

De hecho, ahora ProgClone es un puntero nulo. Por lo tanto, una desreferenciación del puntero nulo debería ocurrir justo debajo:

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

Pero, de hecho, ¡esto no sucederá! Tenga en cuenta que el bucle no se está ejecutando realmente.

Al principio, se borra el contenedor MiscompiledFunctions :

 MiscompiledFunctions.clear();

A continuación, el tamaño de este contenedor se usa en la condición de bucle:

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

Es fácil ver que el ciclo no se inicia. Creo que esto también es un error, y el código debe escribirse de manera diferente.

¡Parece que conocimos esa famosa paridad de errores! Un error disfraza a otro :).

Fragmento N7: Usando el cursor después de mover

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

Advertencia de PVS-Studio: V522 [CWE-476] Puede tener lugar la referencia del puntero nulo 'Prueba'. Miscompilation.cpp 709

De nuevo la misma situación. Al principio, el contenido del objeto se mueve y luego se usa como si nada hubiera pasado. Cada vez más veo esta situación en el código del programa, después de que la semántica del movimiento apareció en C ++. ¡Por esto me encanta el lenguaje C ++! Hay más y más nuevas formas de dispararle a su propia 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] Puede tener lugar la desreferenciación del puntero nulo 'Tipo'. PrettyFunctionDumper.cpp 233

Además de los controladores de errores, las funciones para depurar impresiones generalmente no se prueban. Ante nosotros es un caso así. La función está esperando a un usuario que, en lugar de resolver sus problemas, se verá obligado a solucionarlo.

Correctamente:

 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] Puede tener lugar la desreferenciación del puntero nulo 'Ty'. SearchableTableEmitter.cpp 614

Creo, y 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; }

PVS-Studio Advertencia: V570 La variable 'Identificador-> Tipo' se asigna a sí misma. FormatTokenLexer.cpp 249

No tiene sentido asignar una variable a sí mismo. Lo más probable es que quisieran escribir:

 Identifier->Type = Question->Type;

Fragmento N11: ruptura 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: V622 [CWE-478] Considere inspeccionar la declaración 'switch'. Es posible que falte el primer operador de 'caso'. SystemZAsmParser.cpp 652

Al principio hay una declaración de ruptura muy sospechosa. ¿Has olvidado escribir algo más aquí?

Fragmento N12: Verificación del puntero después de 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"); .... }

Advertencia de PVS-Studio: V595 [CWE-476] El puntero 'Callee' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 172, 174. AMDGPUInline.cpp 172

El puntero de Callee al principio se desreferencia cuando se llama a la función getTTI .

Y luego resulta que este puntero debe verificarse para la igualdad nullptr :

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

Pero es muy tarde ...

Fragmento N13 - N ...: Verificación del puntero después de desreferenciar

La situación discutida en el fragmento de código anterior no es única. Ella se encuentra 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 que se verificara contra nullptr. Líneas de verificación: 1079, 1081. SimplifyLibCalls.cpp 1079

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

Advertencia de PVS-Studio: V595 [CWE-476] El puntero 'ND' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 532, 534. SemaTemplateInstantiateDecl.cpp 532

Y aqui:

  • V595 [CWE-476] El puntero 'U' se utilizó antes de que se verificara contra nullptr. Verifique las líneas: 404, 407. DWARFFormValue.cpp 404
  • V595 [CWE-476] El puntero 'ND' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 2149, 2151. SemaTemplateInstantiate.cpp 2149

Y luego no me interesó estudiar las advertencias con el número V595. Por lo tanto, no sé si hay errores similares además de los enumerados 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: V629 [CWE-190] Considere inspeccionar la expresión '~ (Tamaño - 1) << 1'. Desplazamiento de bits del valor de 32 bits con una expansión posterior al tipo de 64 bits. AArch64AddressingModes.h 260

Quizás esto no sea un error, y el código funciona exactamente como se esperaba. Pero este es claramente un lugar muy sospechoso, y necesita ser revisado.

Suponga que la variable Tamaño es 16, y luego el autor del código planeó obtener el valor en la variable NImms :

11111111111111111111111111111111111111111111111111111111111111100000000

Sin embargo, de hecho, el resultado es:

0000000000000000000000000000000000001111111111111111111111111111100000

El hecho es que todos los cálculos se realizan utilizando el tipo sin signo de 32 bits. Y solo entonces, este tipo sin signo 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 así:

 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 expansión posterior al tipo de 64 bits. AArch64AddressingModes.h 269

Fragmento N19: ¿Falta otra palabra clave ?

 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: V646 [CWE-670] Considere inspeccionar la lógica de la aplicación. Es posible que falte la palabra clave 'else'. AMDGPUAsmParser.cpp 5655

No hay error aquí. Dado que el bloque del primer if termina con continuar , no importa si hay una palabra clave más o no. En cualquier caso, el código funcionará igual. Sin embargo, la falta de otra cosa hace que el código sea más oscuro y peligroso. Si en el futuro continúa desapareciendo, entonces el código comenzará a funcionar de una 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 'Result + Name.str ()'. Symbol.cpp 32
  • V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(Clase ObjC)" + Nombre.str ()'. Symbol.cpp 35
  • V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(ObjC Clase EH)" + Nombre.str ()'. Symbol.cpp 38
  • V655 [CWE-480] Las cadenas se concatenaron pero no se utilizan. Considere inspeccionar la expresión 'Resultado + "(ObjC IVar)" + Nombre.str ()'. Symbol.cpp 41

Por casualidad, se usa el operador + en lugar del operador + =. El resultado son diseños sin sentido.

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

Intenta encontrar el código peligroso tú mismo. Y esta es una imagen de distracción, para no mirar de inmediato la respuesta:

Hmmm ...


Advertencia de PVS-Studio: V708 [CWE-758] Se utiliza una construcción peligrosa: 'FeaturesMap [Op] = FeaturesMap.size ()', donde 'FeaturesMap' es de la clase 'map'. Esto puede conducir a un comportamiento indefinido. RISCVCompressInstEmitter.cpp 490

Línea del problema:

 FeaturesMap[Op] = FeaturesMap.size();

Si no se encuentra el elemento Op , se crea un nuevo elemento en el mapa y allí se escribe el número de elementos en este mapa. Simplemente no se sabe si la función de tamaño se llamará antes o después de agregar un nuevo elemento.

Fragmento N22-N24: reasignaciones

 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: V519 [CWE-563] A la variable 'NType' se le asignan valores dos veces sucesivamente. Quizás esto sea un error. Líneas de verificación: 1663, 1664. MachOObjectFile.cpp 1664

Creo que no hay ningún error real aquí. Solo tarea repetida superflua. Pero sigue siendo un error.

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. Verifique las líneas: 59, 61. coff2yaml.cpp 61

Fragmento N25-N27: más reasignaciones

Ahora considere una opción de reasignación ligeramente diferente.

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

Advertencia de PVS-Studio: V519 [CWE-563] La variable 'Alineación' recibe valores asignados dos veces sucesivamente. 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 parece contener un error lógico. Al principio, a la variable Alineación se le asigna un valor según la condición. Y luego la asignación ocurre nuevamente, pero ahora sin ninguna verificación.

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] La variable 'ExpectNoDerefChunk' recibe valores asignados dos veces sucesivamente. Quizás esto sea un error. Líneas de verificación: 4970, 4973. SemaType.cpp 4973

Fragmento N28: siempre una condición 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: V547 [CWE-571] La expresión 'nextByte! = 0x90' siempre es verdadera. X86DisassemblerDecoder.cpp 379

La verificación no tiene sentido. La variable nextByte no siempre es igual a 0x90 , que se deduce de la verificación anterior. Este es algún tipo de error lógico.

Fragmento N29 - N ...: Siempre condiciones verdaderas / falsas

El analizador da muchas advertencias de que toda la condición ( V547 ) o parte de ella ( V560 ) siempre es verdadera o falsa. A menudo, estos no son errores reales, sino simplemente código inexacto, el resultado del despliegue de macros y similares. Sin embargo, tiene sentido observar todas estas advertencias, ya que de vez en cuando hay errores lógicos reales. Por ejemplo, este fragmento de código es sospechoso:

 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: V560 [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 el sistema decimal. Comprobar RegNo == 0xe no tiene sentido, porque si RegNo> 13 , entonces la función completará su ejecución.

Hubo muchas otras advertencias con los identificadores V547 y V560, pero, como en el caso de V595 , no estaba interesado en estudiar estas advertencias. Ya estaba claro que tenía suficiente material para escribir un artículo :). Por lo tanto, no se sabe cuántos errores de este tipo se pueden detectar en LLVM utilizando PVS-Studio.

Daré un ejemplo de por qué es aburrido estudiar estas respuestas. 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: ​​regreso sospechoso

 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: V612 [CWE-670] Un 'retorno' incondicional dentro de un bucle. R600OptimizeVectorRegisters.cpp 63

Esto es un error o un truco específico que pretende 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í :).

Estas cansado Luego es hora de hacer té o café.

Cafe


Defectos detectados por nuevos diagnósticos


Creo que 30 disparos de diagnósticos antiguos es suficiente. Veamos ahora qué es interesante se puede encontrar con los nuevos diagnósticos que aparecieron en el analizador después de una verificación previa . En total, se agregaron 66 diagnósticos de uso general al analizador C ++ durante este tiempo.

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: V779 [CWE-561] Código inalcanzable detectado. Es posible que haya un error presente. ExecutionUtils.cpp 146

Como puede ver, ambas ramas de la declaración if terminan con una llamada a la declaración return . En consecuencia, el contenedor CtorDtorsByPriority nunca se vaciará .

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] Código inalcanzable detectado. Es posible que haya un error presente. LLParser.cpp 835

Una situación interesante Veamos el comienzo de este lugar:

 return ParseTypeIdEntry(SummaryID); break;

A primera vista, parece que no hay error. Parece que la declaración de ruptura es superflua aquí, y simplemente puede eliminarla. Sin embargo, no todo es tan simple.

El analizador genera una advertencia en las líneas:

 Lex.setIgnoreColonInIdentifiers(false); return false;

De hecho, este código es inalcanzable. Todos los casos en switch finalizan con una llamada a la declaración de devolución . ¡Y ahora el inútil descanso solitario no parece tan inofensivo! ¿Quizás una de las ramas debería terminar con descanso y no con retorno ?

Fragmento N33: Accidentalización 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: V784 El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto causará la pérdida de bits más altos. RuntimeDyld.cpp 815

Tenga en cuenta que la función getStubAlignment devuelve un tipo sin signo . Calculamos el valor de la expresión si suponemos que la función devuelve el valor 8:

~ (getStubAlignment () - 1)

~ (8u-1)

0xFFFFFFF8u

Ahora observe que la variable DataSize tiene un tipo sin signo de 64 bits. Resulta que durante la operación DataSize & 0xFFFFFFF88, se restablecerán los treinta y dos bits de orden superior. Lo más probable es que esto no sea lo que el programador quería. Sospecho que quería calcular: DataSize & 0xFFFFFFFFFFFFFFFFF8u.

Para corregir el error, debe escribir así:

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

Más o menos:

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

Fragmento N34: lanzamiento explícito fallido

 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: V1028 [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 explícita de tipos se utiliza para evitar el desbordamiento al multiplicar variables de tipo int . Sin embargo, la transmisión explícita aquí no protege contra el desbordamiento.Al principio, las variables se multiplicarán, y solo entonces el resultado de la multiplicación de 32 bits se expandirá al tipo size_t .

Fragmento N35: Copiar y pegar sin éxito

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

V778 [CWE-682] Se encontraron dos fragmentos de código similares. Quizás, este es un error tipográfico y se debe usar la variable 'Op1' en lugar de 'Op0'. InstCombineCompares.cpp 5507

Este nuevo diagnóstico interesante revela situaciones en las que se copió un fragmento de código y algunos nombres comenzaron a cambiar, pero no se corrigieron en un solo lugar.

Tenga en cuenta que en el segundo bloque cambiaron Op0 a Op1 . Pero en un lugar no lo arreglaron. Lo más probable es que 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 en variables

 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: V1001 [CWE-563] La variable 'Modo' se asigna pero no se usa al final de la función. SIModeRegister.cpp 48

Es muy peligroso dar a los argumentos de función los mismos nombres que los miembros de la clase. Muy fácil confundirse. Ante nosotros es un caso así. Esta expresión no tiene sentido:

 Mode &= Mask;

El argumento de la función cambia. Y eso es todo.Este argumento ya no se usa. Lo más probable es que fuera necesario escribir así:

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

Fragmento N37: Confusión en variables

 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 de PVS-Studio: V1001 [CWE-563] La variable 'Tamaño' se asigna pero no se usa al final de la función. Object.cpp 424

La situación es similar a la anterior. Debería estar escrito:

 this->Size += this->EntrySize;

Fragmento N38-N47: el puntero se olvidó de verificar.

Anteriormente, examinamos ejemplos de activación de diagnóstico V595 . Su esencia es que el puntero se desreferencia al principio, y solo luego se verifica. El joven diagnóstico del V1004 es lo contrario de su significado, pero también detecta muchos errores. Identifica situaciones en las que el puntero se verificó al principio y luego se olvidó de hacerlo. Considere tales casos encontrados 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 que se verificó contra nullptr. Líneas de verificación: 729, 738. TargetTransformInfoImpl.h 738

La variable Ptr puede ser nullptr , como lo demuestra la verificación:

 if (Ptr != nullptr)

Sin embargo, debajo de este puntero se desreferencia sin verificación previa:

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

Considere 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 que se verificó contra nullptr. Verifique las líneas: 3228, 3231. CGDebugInfo.cpp 3231

Preste atención al puntero FD . Estoy seguro de que el problema es claramente visible y no se requiere ninguna explicación especial.

Y también:

 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 que se verificó contra nullptr. Verifique las líneas: 960, 965. InterleavedLoadCombinePass.cpp 965

¿Cómo protegerse de tales errores? Tenga cuidado con la revisión de código y use el analizador estático PVS-Studio para verificar regularmente su código.

No tiene sentido traer 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 forma insegura después de que se verificó contra nullptr. Líneas de verificación: 1049, 1078. DebugInfoMetadata.cpp 1078
  • V1004 [CWE-476] El puntero 'PI' se usó de forma insegura después de que se verificó contra nullptr. Líneas de verificación: 733, 753. LegacyPassManager.cpp 753
  • V1004 [CWE-476] The 'StatepointCall' pointer was used unsafely after it was verified against nullptr. Check lines: 4371, 4379. Verifier.cpp 4379
  • V1004 [CWE-476] The 'RV' pointer was used unsafely after it was verified against nullptr. Check lines: 2263, 2268. TGParser.cpp 2268
  • V1004 [CWE-476] The 'CalleeFn' pointer was used unsafely after it was verified against nullptr. Check lines: 1081, 1096. SimplifyLibCalls.cpp 1096
  • V1004 [CWE-476] The 'TC' pointer was used unsafely after it was verified against nullptr. Check lines: 1819, 1824. Driver.cpp 1824

N48-N60: , ( )

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

Advertencia de PVS-Studio: V1023 [CWE-460] El método 'emplace_back' agrega un puntero sin propietario al contenedor 'Estrategias'. Se producirá una pérdida de memoria en caso de una excepción. llvm-isel-fuzzer.cpp 58

Para agregar un elemento al final de un contenedor como std :: vector <std :: unique_ptr <X>> no puede simplemente escribir xxx.push_back (nueva X) , ya que no hay una conversión implícita de X * a std: : unique_ptr <X> .

Una solución común es escribir xxx.emplace_back (nueva X) , porque compila: el método emplace_back construye un elemento directamente de los argumentos y, por lo tanto, puede usar constructores explícitos.

Esto no es seguro. Si el vector está lleno, se asigna memoria. Una operación de reasignación de memoria puede fallar, dando como resultado 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 unique_ptr , que será el propietario del puntero antes de que el vector intente reasignar la memoria:

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

Comenzando con C ++ 14, puede 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, entonces el compilador simplemente dejará de funcionar. Sin embargo, para aplicaciones con un tiempo de actividad prolongado que no puede terminar si falla la asignación de memoria, esto puede ser un error realmente desagradable.

Entonces, aunque este código no representa un peligro práctico para LLVM, me pareció útil hablar sobre este patrón de error y que el analizador PVS-Studio aprendió a detectarlo.

Otras advertencias de este tipo:

  • V1023 [CWE-460] El método 'emplace_back' agrega un puntero sin propietario al contenedor 'Passes'. Se producirá una pérdida de memoria en caso de una excepción. PassManager.h 546
  • V1023 [CWE-460] A pointer without owner is added to the 'AAs' container by the 'emplace_back' method. A memory leak will occur in case of an exception. AliasAnalysis.h 324
  • V1023 [CWE-460] A pointer without owner is added to the 'Entries' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DWARFDebugFrame.cpp 519
  • V1023 [CWE-460] A pointer without owner is added to the 'AllEdges' container by the 'emplace_back' method. A memory leak will occur in case of an exception. CFGMST.h 268
  • V1023 [CWE-460] A pointer without owner is added to the 'VMaps' container by the 'emplace_back' method. A memory leak will occur in case of an exception. SimpleLoopUnswitch.cpp 2012
  • V1023 [CWE-460] A pointer without owner is added to the 'Records' container by the 'emplace_back' method. A memory leak will occur in case of an exception. FDRLogBuilder.h 30
  • V1023 [CWE-460] A pointer without owner is added to the 'PendingSubmodules' container by the 'emplace_back' method. A memory leak will occur in case of an exception. ModuleMap.cpp 810
  • V1023 [CWE-460] A pointer without owner is added to the 'Objects' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DebugMap.cpp 88
  • V1023 [CWE-460] A pointer without owner is added to the 'Strategies' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-isel-fuzzer.cpp 60
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 685
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 686
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 688
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 689
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 690
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 691
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 692
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 693
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 694
  • V1023 [CWE-460] A pointer without owner is added to the 'Operands' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 1911
  • V1023 [CWE-460] A pointer without owner is added to the 'Stash' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2100
  • V1023 [CWE-460] A pointer without owner is added to the 'Matchers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2702

Conclusión


En total, escribí 60 advertencias, después de lo cual paré. ¿Hay otros defectos que el analizador PVS-Studio detecta en LLVM? Si lo hay Sin embargo, cuando escribí los fragmentos de código para el artículo, ya era tarde, o más bien, incluso de noche, y decidí que era hora de redondear.

Espero que haya estado interesado y desee probar el analizador PVS-Studio.

Puede descargar el analizador y obtener una clave de prueba en esta página .

Lo más importante, utilice análisis estático regularmente. Las verificaciones únicas realizadas por nosotros para popularizar la metodología del análisis estático y PVS-Studio no son un escenario normal.

¡Buena suerte en mejorar la calidad y confiabilidad del código!



Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Andrey Karpov. Encontrar errores en LLVM 8 con PVS-Studio .

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

More articles:


All Articles