Comprobaci贸n de Clang 11 con PVS-Studio

PVS-Studio: 隆A煤n merece la pena!


De vez en cuando tenemos que escribir art铆culos sobre c贸mo comprobar la pr贸xima versi贸n de un compilador. No es interesante. Sin embargo, como muestra la pr谩ctica, si esto no se hace durante mucho tiempo, la gente comienza a dudar de si el analizador PVS-Studio merece el t铆tulo de un buen receptor de errores y vulnerabilidades potenciales. 驴Quiz谩s el nuevo compilador ya sabe c贸mo hacer esto? S铆, los compiladores no se quedan quietos. Sin embargo, PVS-Studio tambi茅n se est谩 desarrollando, demostrando una y otra vez la capacidad de encontrar errores incluso en el c贸digo de proyectos de alta calidad como los compiladores.



Es hora de volver a comprobar el c贸digo Clang



Para ser honesto, tom茅 el art铆culo " Verificaci贸n del compilador GCC 10 usando PVS-Studio " como base para este texto . Entonces, si te parece que ya has le铆do algunos p谩rrafos en alguna parte, entonces no piensas :).



No es ning煤n secreto que los compiladores tienen sus propios analizadores de c贸digo est谩tico integrados y tambi茅n est谩n evolucionando. Por tanto, de vez en cuando escribimos art铆culos sobre c贸mo el analizador est谩tico PVS-Studio puede encontrar errores incluso dentro de los compiladores y que comemos pan por una raz贸n :)



De hecho, no se pueden comparar analizadores est谩ticos cl谩sicos con compiladores. Los analizadores est谩ticos no son solo una b煤squeda de errores en el c贸digo, sino tambi茅n una infraestructura desarrollada. Por ejemplo, esta es la integraci贸n con sistemas como SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins, Visual Studio. Estos son mecanismos avanzados para la supresi贸n masiva de advertencias, lo que le permite comenzar a usar PVS-Studio r谩pidamente incluso en un gran proyecto antiguo. Esta es una distribuci贸n de notificaci贸n. Y as铆 sucesivamente y as铆 sucesivamente. Sin embargo, sigue siendo la primera pregunta que se hace: "驴Puede PVS-Studio encontrar algo que los compiladores no puedan encontrar?" Esto significa que escribiremos una y otra vez art铆culos sobre la verificaci贸n de estos compiladores.



Volvamos al tema de comprobar el proyecto Clang. No es necesario detenerse en este proyecto y decir de qu茅 se trata. De hecho, no solo se prob贸 el c贸digo de Clang 11 en s铆, sino tambi茅n la biblioteca LLVM 11 en la que est谩 construido. Desde el punto de vista de este art铆culo, no importa si se encuentra un defecto en el c贸digo del compilador o de la biblioteca.



El c贸digo Clang / LLVM me pareci贸 mucho m谩s claro que el c贸digo GCC. Al menos faltan todas estas terribles macros, y las caracter铆sticas modernas del lenguaje C ++ se utilizan activamente.



A pesar de esto, el proyecto es grande y sin configuraciones preliminares del analizador, ver el informe sigue siendo muy tedioso. B谩sicamente, los medio positivos falsos interfieren. Por positivos "semifalsos" me refiero a situaciones en las que el analizador tiene formalmente raz贸n, pero no tiene sentido advertir. Por ejemplo, muchos de estos positivos se emiten para pruebas unitarias y c贸digo generado.



Ejemplo de pruebas:



Spaces.SpacesInParentheses = false;               // <=
Spaces.SpacesInCStyleCastParentheses = true;      // <=
verifyFormat("Type *A = ( Type * )P;", Spaces);
verifyFormat("Type *A = ( vector<Type *, int *> )P;", Spaces);
verifyFormat("x = ( int32 )y;", Spaces);
verifyFormat("int a = ( int )(2.0f);", Spaces);
verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
verifyFormat("#define x (( int )-1)", Spaces);

// Run the first set of tests again with:
Spaces.SpacesInParentheses = false;               // <=
Spaces.SpaceInEmptyParentheses = true;
Spaces.SpacesInCStyleCastParentheses = true;      // <=
verifyFormat("call(x, y, z);", Spaces);
verifyFormat("call( );", Spaces);


El analizador advierte que a las variables se les asignan los mismos valores que ya contienen:



  • V1048 A la variable 'Spaces.SpacesInParentheses' se le asign贸 el mismo valor. FormatTest.cpp 11554
  • V1048 A la variable 'Spaces.SpacesInCStyleCastParentheses' se le asign贸 el mismo valor. FormatTest.cpp 11556


Formalmente, el analizador devolvi贸 la respuesta correcta, y este es el fragmento de c贸digo que debe simplificarse o corregirse. Al mismo tiempo, est谩 claro que de hecho todo est谩 bien y que tampoco tiene sentido editar algo.



Otro ejemplo: el analizador emite una gran cantidad de advertencias al archivo Options.inc generado autom谩ticamente. Puede ver la "hoja de c贸digos" en el archivo:



El c贸digo


Y PVS-Studio env铆a advertencias a todo esto:



  • V501 Hay subexpresiones id茅nticas a la izquierda y a la derecha del operador '==': nullptr == nullptr Options.inc 26
  • V501 Hay sub-expresiones id茅nticas a la izquierda y a la derecha del operador '==': nullptr == nullptr Options.inc 27
  • V501 Hay subexpresiones id茅nticas a la izquierda y a la derecha del operador '==': nullptr == nullptr Options.inc 28
  • y as铆 sucesivamente para cada l铆nea ...


Todo esto no da miedo. Todo esto puede evitarse: deshabilite la comprobaci贸n de archivos innecesarios, marque algunas macros y funciones, suprima ciertos tipos de alarmas, etc. Es posible, pero hacerlo como parte de la tarea de escribir un art铆culo no es interesante. Por lo tanto, hice exactamente lo mismo que en el art铆culo sobre el compilador GCC. Estudi茅 el informe hasta que tuve 11 ejemplos de c贸digo interesantes para escribir un art铆culo. 驴Por qu茅 11? Pens茅 que dado que la versi贸n de Clang es 11, entonces deja que los fragmentos sean 11 :).



11 fragmentos de c贸digo sospechosos



Fragmento N1, m贸dulo divisi贸n por uno



% 1 - divisi贸n de m贸dulo por 1


隆Genial error! 隆Me encantan estos!



void Act() override {
  ....
  // If the value type is a vector, and we allow vector select, then in 50%
  // of the cases generate a vector select.
  if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
    unsigned NumElem =
        cast<FixedVectorType>(Val0->getType())->getNumElements();
    CondTy = FixedVectorType::get(CondTy, NumElem);
  }
  ....
}


Advertencia de PVS-Studio: V1063 La operaci贸n m贸dulo por 1 no tiene sentido. El resultado siempre ser谩 cero. llvm-stress.cpp 631 La



divisi贸n de m贸dulo se usa para obtener un valor aleatorio de 0 o 1. Pero, aparentemente, este valor de 1 confunde, y la gente hace el patr贸n cl谩sico de error, dividir por uno, aunque es necesario dividir por dos. La operaci贸n X% 1 no tiene sentido, ya que el resultado siempre es 0 . Variante de c贸digo correcta:



if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2)) {


Diagnostics V1063, que apareci贸 recientemente en PVS-Studio, es escandalosamente simple, pero, como puede ver, funciona.



Como sabemos, los desarrolladores de compiladores analizan lo que estamos haciendo y toman prestadas nuestras mejores pr谩cticas. No hay nada de malo en ello. Nos complace que PVS-Studio sea el motor del progreso . Establecemos el tiempo despu茅s de cu谩nto aparecer谩n los mismos diagn贸sticos en Clang y GCC :).



Fragmento N2, error tipogr谩fico en la condici贸n



class ReturnValueSlot {
  ....
  bool isNull() const { return !Addr.isValid(); }
  ....
};

static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
                                   const FunctionDecl *F2, unsigned NumParams) {
  ....
  unsigned I1 = 0, I2 = 0;
  for (unsigned I = 0; I != NumParams; ++I) {
    QualType T1 = NextParam(F1, I1, I == 0);
    QualType T2 = NextParam(F2, I2, I == 0);
    if (!T1.isNull() && !T1.isNull() && !Context.hasSameUnqualifiedType(T1, T2))
      return false;
  }
  return true;
}


Advertencia de PVS-Studio: V501 Hay subexpresiones id茅nticas a la izquierda y a la derecha del operador '&&':! T1.isNull () &&! T1.isNull () SemaOverload.cpp 9493 Comprobando



dos veces ! T1.isNull () . Este es un error tipogr谩fico obvio, y la segunda parte de la condici贸n deber铆a verificar la variable T2 .



Fragmento N3, potencial fuera de los l铆mites de la matriz



std::vector<Decl *> DeclsLoaded;

SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
  ....
  unsigned Index = ID - NUM_PREDEF_DECL_IDS;

  if (Index > DeclsLoaded.size()) {
    Error("declaration ID out-of-range for AST file");
    return SourceLocation();
  }

  if (Decl *D = DeclsLoaded[Index])
    return D->getLocation();
  ....
}


Advertencia de PVS-Studio: Es posible que la matriz V557 se desborde. El 铆ndice 'Index' apunta m谩s all谩 del l铆mite de la matriz. ASTReader.cpp 7318



Suponga que la matriz contiene un elemento y la variable de 铆ndice tambi茅n es uno. La condici贸n (1> 1) es falsa y, como resultado, la matriz se invadir谩. Verificaci贸n correcta:



if (Index >= DeclsLoaded.size()) {


Fragmento N4, el orden de evaluaci贸n de los argumentos



void IHexELFBuilder::addDataSections() {
  ....
  uint32_t SecNo = 1;
  ....
  Section = &Obj->addSection<OwnedDataSection>(
      ".sec" + std::to_string(SecNo++), RecAddr,
      ELF::SHF_ALLOC | ELF::SHF_WRITE, SecNo);
  ....
}


Advertencia de PVS-Studio: V567 Comportamiento no especificado. El orden de evaluaci贸n de los argumentos no est谩 definido para la funci贸n 'addSection'. Considere inspeccionar la variable 'SecNo'. Object.cpp 1223



Tenga en cuenta que el argumento SecNo se utiliza dos veces y se incrementa a lo largo del camino. Es imposible decir en qu茅 orden se evaluar谩n los argumentos. Por lo tanto, el resultado variar谩 seg煤n la versi贸n del compilador o la configuraci贸n de la compilaci贸n.



D茅jame explicarte esto con un ejemplo sint茅tico:



#include <cstdio>
int main()
{
  int i = 1;
  printf("%d, %d\n", i, i++);
  return 0;
}


Dependiendo del compilador, se pueden imprimir tanto "1, 2" como "2, 1". Usando el Explorador del compilador, obtengo los siguientes resultados:



  • un programa compilado con Clang 11.0.0 produce : 1, 1.
  • un programa compilado con GCC 10.2 produce : 2, 1.


Curiosamente, para este simple caso, el compilador de Clang emite una advertencia:



<source>:6:26: warning:
unsequenced modification and access to 'i' [-Wunsequenced]
printf("%d, %d\n", i, i++);


Al parecer, en condiciones reales, esta advertencia no ayud贸. El diagn贸stico est谩 deshabilitado, ya que es inconveniente para el uso pr谩ctico, o el compilador no pudo advertir sobre un caso m谩s complejo.



Fragmento N5, extra帽a nueva prueba



template <class ELFT>
void GNUStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
                                               const Elf_Shdr *Sec) {

  ....
  Expected<StringRef> NameOrErr =
      this->dumper()->getSymbolVersionByIndex(Ndx, IsDefault);
  if (!NameOrErr) {
    if (!NameOrErr) {
      unsigned SecNdx = Sec - &cantFail(Obj->sections()).front();
      this->reportUniqueWarning(createError(
          "unable to get a version for entry " + Twine(I) +
          " of SHT_GNU_versym section with index " + Twine(SecNdx) + ": " +
          toString(NameOrErr.takeError())));
    }
    Versions.emplace_back("<corrupt>");
    continue;
  }
  ....
}


Advertencia de PVS-Studio: V571 Comprobaci贸n recurrente. La condici贸n 'if (! NameOrErr)' ya se verific贸 en la l铆nea 4666. ELFDumper.cpp 4667



La segunda verificaci贸n duplica la primera y es redundante. Quiz谩s el segundo cheque pueda simplemente eliminarse. Pero lo m谩s probable es que se trate de un error tipogr谩fico y se debe utilizar una variable diferente en la segunda condici贸n.



Snippet N6, eliminando la referencia a un puntero potencialmente nulo



void RewriteObjCFragileABI::RewriteObjCClassMetaData(
  ObjCImplementationDecl *IDecl, std::string &Result)
{
  ObjCInterfaceDecl *CDecl = IDecl->getClassInterface();

  if (CDecl->isImplicitInterfaceDecl()) {
    RewriteObjCInternalStruct(CDecl, Result);
  }

  unsigned NumIvars = !IDecl->ivar_empty()
  ? IDecl->ivar_size()
  : (CDecl ? CDecl->ivar_size() : 0);
  ....
}


Advertencia de PVS-Studio: V595 El puntero 'CDecl' se utiliz贸 antes de que se verificara con nullptr. L铆neas de verificaci贸n: 5275, 5284. RewriteObjC.cpp 5275



Durante la primera verificaci贸n, el puntero CDecl siempre se desreferencia en negrita :



if (CDecl->isImplicitInterfaceDecl())


Y solo a partir del c贸digo escrito a continuaci贸n, queda claro que este puntero puede ser nulo:



(CDecl ? CDecl->ivar_size() : 0)


Lo m谩s probable es que la primera comprobaci贸n tenga este aspecto:



if (CDecl && CDecl->isImplicitInterfaceDecl())


Snippet N7, eliminando la referencia a un puntero potencialmente nulo



bool
Sema::InstantiateClass(....)
{
  ....
  NamedDecl *ND = dyn_cast<NamedDecl>(I->NewDecl);
  CXXRecordDecl *ThisContext =
      dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext());
  CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
                              ND && ND->isCXXInstanceMember());
  ....
}


Advertencia de PVS-Studio: V595 El puntero 'ND' se utiliz贸 antes de que se verificara con nullptr. Verifique las l铆neas: 2803, 2805. SemaTemplateInstantiate.cpp 2803



Una variaci贸n del error anterior. Es peligroso eliminar la referencia de un puntero sin comprobarlo primero si su valor se obtiene mediante una conversi贸n din谩mica. Adem谩s, a partir del c贸digo siguiente, queda claro que se necesita tal verificaci贸n.



Fragmento N8, la funci贸n contin煤a ejecut谩ndose a pesar de un estado de error



bool VerifyObject(llvm::yaml::Node &N,
                  std::map<std::string, std::string> Expected) {
  ....
  auto *V = llvm::dyn_cast_or_null<llvm::yaml::ScalarNode>(Prop.getValue());
  if (!V) {
    ADD_FAILURE() << KS << " is not a string";
    Match = false;
  }
  std::string VS = V->getValue(Tmp).str();
  ....
}


Advertencia de PVS-Studio: V1004 El puntero 'V' se utiliz贸 de forma insegura despu茅s de que se verificara con nullptr. Verifique las l铆neas: 61, 65. TraceTests.cpp 65 El



puntero V puede ser nulo. Esta es claramente una condici贸n err贸nea, que incluso se informa. Sin embargo, despu茅s de eso, la funci贸n contin煤a ejecut谩ndose como si nada hubiera pasado, lo que conducir谩 a la eliminaci贸n de referencias de este puntero nulo. Quiz谩s se olvidaron de interrumpir la funci贸n y la opci贸n correcta deber铆a verse as铆:



auto *V = llvm::dyn_cast_or_null<llvm::yaml::ScalarNode>(Prop.getValue());
if (!V) {
  ADD_FAILURE() << KS << " is not a string";
  Match = false;
  return false;
}
std::string VS = V->getValue(Tmp).str();


Fragmento N9, error tipogr谩fico



const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
                                  const InputInfo &Output) {
  if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
    if (StringRef(A->getValue()) == "single")
      return Args.MakeArgString(Output.getFilename());

  Arg *FinalOutput = Args.getLastArg(options::OPT_o);
  if (FinalOutput && Args.hasArg(options::OPT_c)) {
    SmallString<128> T(FinalOutput->getValue());
    llvm::sys::path::replace_extension(T, "dwo");
    return Args.MakeArgString(T);
  } else {
    // Use the compilation dir.
    SmallString<128> T(
        Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
    SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
    llvm::sys::path::replace_extension(F, "dwo");
    T += F;
    return Args.MakeArgString(F);       // <=
  }
}


Advertencia de PVS-Studio: V1001 La variable 'T' est谩 asignada pero no se utiliza al final de la funci贸n. CommonArgs.cpp 873



Observe el final de la funci贸n. La variable local T se cambia pero no se usa de ninguna manera. Lo m谩s probable es que se trate de un error tipogr谩fico y la funci贸n deber铆a terminar con las siguientes l铆neas de c贸digo:



T += F;
return Args.MakeArgString(T);


Fragmento N10, el divisor es cero



typedef int32_t si_int;
typedef uint32_t su_int;

typedef union {
  du_int all;
  struct {
#if _YUGA_LITTLE_ENDIAN
    su_int low;
    su_int high;
#else
    su_int high;
    su_int low;
#endif // _YUGA_LITTLE_ENDIAN
  } s;
} udwords;

COMPILER_RT_ABI du_int __udivmoddi4(du_int a, du_int b, du_int *rem) {
  ....
  if (d.s.low == 0) {
    if (d.s.high == 0) {
      // K X
      // ---
      // 0 0
      if (rem)
        *rem = n.s.high % d.s.low;
      return n.s.high / d.s.low;
    }
  ....
}


Advertencias de PVS-Studio:



  • V609 Mod por cero. Denominador 'dslow' == 0.udivmoddi4.c 61
  • V609 Dividir por cero. Denominador 'dslow' == 0.udivmoddi4.c 62


No s茅 si esto es un error o una idea enga帽osa, pero el c贸digo es muy extra帽o. Hay dos variables enteras ordinarias y una es divisible por la otra. Curiosamente, esto solo sucede si ambas variables son cero. 驴Qu茅 significa todo esto?



Fragmento N11, copiar y pegar



bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(....)
{
  ....
  StringRef FName = II->getName();
  ....
  if (FName == "postEvent" &&
      FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
    return true;
  }

  if (FName == "postEvent" &&
      FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
    return true;
  }
  ....
}


Advertencia de PVS-Studio: V581 Las expresiones condicionales de las declaraciones 'if' situadas una junto a la otra son id茅nticas. Verifique las l铆neas: 3108, 3113. MallocChecker.cpp 3113



El fragmento de c贸digo se copi贸, pero no se modific贸 de ninguna manera. El segundo fragmento debe eliminarse o modificarse para comenzar a realizar una verificaci贸n 煤til.



Conclusi贸n





Perm铆tame recordarle que puede usar esta opci贸n de licencia gratuita para verificar proyectos de c贸digo abierto . Por cierto, existen otras opciones para la licencia gratuita de PVS-Studio, incluso para proyectos cerrados. Se enumeran aqu铆: " Opciones de licencia gratuitas de PVS-Studio ". Gracias por su atenci贸n.



Nuestros otros art铆culos sobre la comprobaci贸n del compilador









Si desea compartir este art铆culo con una audiencia de habla inglesa, utilice el enlace de traducci贸n: Andrey Karpov. Comprobando Clang 11 con PVS-Studio .



All Articles