Han pasado tres meses desde que terminó 2018. Para muchos, simplemente pasó volando, pero para nosotros, los desarrolladores de PVS-Studio, fue un año bastante agitado. Estábamos trabajando arduamente, compitiendo sin miedo por correr la voz sobre el análisis estático y estábamos buscando errores en proyectos de código abierto, escritos en lenguaje C, C ++, C # y Java. ¡En este artículo, reunimos los 10 mejores más interesantes para usted!
Para encontrar los lugares más interesantes, utilizamos el analizador de código estático
PVS-Studio . Puede detectar errores y vulnerabilidades potenciales en el código, escrito en los idiomas enumerados anteriormente.
Si está entusiasmado con la búsqueda de errores usted mismo, siempre puede descargar y probar nuestro analizador. Ofrecemos la
versión de analizador gratuita para estudiantes y desarrolladores entusiastas, la
licencia gratuita para desarrolladores de proyectos de código abierto y también la
versión de prueba para todo el mundo y su perro. Quién sabe, ¿quizás para el próximo año podrás crear tu propio top 10? :)
Nota: lo invito a que se revise a sí mismo y, antes de mirar la advertencia del analizador, intente revelar los defectos usted mismo. ¿Cuántos errores podrás encontrar?
Décimo lugar
Fuente:
Into Space Again: cómo el Unicornio visitó StellariumEste error se detectó al verificar un planetario virtual llamado Stellarium.
El fragmento de código anterior, aunque pequeño, está plagado de un error bastante complicado:
Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }
¿Lo encontraste?
Advertencia de PVS-Studio :
V603 El objeto se creó pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> Plane :: Plane (....)'. Plane.cpp 29
El autor del código tenía la intención de inicializar los campos de algunos objetos, utilizando otro constructor, anidado en el principal. Bueno, en lugar de eso, solo logró crear un objeto temporal destruido al salir de su alcance. Al hacerlo, los campos de varios objetos permanecerán sin inicializar.
El autor debería haber utilizado un constructor delegado, introducido en C ++ 11, en lugar de una llamada de constructor anidada. Por ejemplo, podría haber escrito así:
Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }
De esta manera, todos los campos necesarios se habrían inicializado correctamente. ¿No es maravilloso?
Noveno lugar
Fuente:
Perl 5: Cómo ocultar errores en macrosUna macro muy notable se destaca en toda su belleza en el noveno lugar.
Al recopilar errores para escribir un artículo, mi colega Svyatoslav encontró una advertencia, emitida por el analizador, que se relacionaba con el uso de macros. Aquí esta:
PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... }
Para descubrir qué estaba mal, Svyatoslav cavó más profundo. Abrió la definición de macro y vio que contenía varias macros anidadas, algunas de las cuales a su vez también tenían macros anidadas. Era muy difícil de entender, así que tuvo que usar un archivo preprocesado. Lamentablemente, no ayudó. Esto es lo que Svyatoslav encontró en la línea de código anterior:
(((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40));
Advertencia de PVS-Studio :
V502 Quizás el operador '?:' Funciona de una manera diferente a la esperada. El operador '?:' Tiene una prioridad menor que el operador '&&'. pp_hot.c 3036
Creo que sería un desafío simplemente notar ese error. Hemos estado pensando en este código durante mucho tiempo, pero, francamente, no hemos encontrado ningún error. De todos modos, es un ejemplo bastante divertido de código mal legible.
Dicen que las macros son malvadas. Claro, hay casos en que las macros son indispensables, pero si puede reemplazar una macro con una función, definitivamente debería hacerlo.
Las macros anidadas están especialmente llenas de trampas. No solo porque es difícil darles sentido, sino también porque pueden dar resultados impredecibles. Si un programador comete un error en tal macro, será mucho más difícil encontrarlo en una macro que en una función.
Octavo lugar
Fuente:
cromo: otros erroresEl siguiente ejemplo fue tomado de la serie de artículos sobre el análisis del proyecto Chromium. El error se estaba ocultando en la biblioteca WebRTC.
std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; }
Advertencia de PVS-Studio: V789 CWE-672 Los iteradores para el contenedor de 'formatos', utilizados en el bucle basado en rango, se vuelven inválidos cuando se llama a la función 'push_back'. stereocodecfactory.cc 89
El error es que el tamaño del vector de
formatos varía dentro del rango para el bucle. Los bucles basados en el rango se basan en iteradores, es por eso que cambiar el tamaño del contenedor dentro de dichos bucles podría resultar en la invalidación de estos iteradores.
Este error persiste si reescribe el ciclo con un uso explícito de iteradores. Para mayor claridad, puedo citar el siguiente código:
for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } }
Por ejemplo, cuando se usa el método
push_back , puede ocurrir una reasignación de vectores; de esta manera, los iteradores abordarán una ubicación de memoria no válida.
Para evitar tales errores, siga la regla: nunca cambie el tamaño de un contenedor dentro de un bucle con condiciones vinculadas a este contenedor. También se relaciona con bucles basados en rango y bucles que utilizan iteradores. Le invitamos a leer esta
discusión en StackOverflow que cubre el tema de las operaciones que causan la invalidación de los iteradores.
Séptimo lugar
Fuente:
Godot: sobre el uso regular de analizadores estáticosEl primer ejemplo de la industria del juego será un fragmento de código que encontramos en el motor del juego Godot. Probablemente, tomará un poco de trabajo notar el error, pero estoy seguro de que nuestros lectores expertos se encargarán de eso.
void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }
Advertencia de PVS-Studio: V621 CWE-835 Considere inspeccionar el operador 'para'. Es posible que el bucle se ejecute incorrectamente o no se ejecute en absoluto. animation_blend_space_1d.cpp 113
Echemos un vistazo de cerca a la condición del bucle. La variable de contador se inicializa con el valor
blend_points_used - 1 . Además, a juzgar por dos comprobaciones anteriores (en
ERR_FAIL_COND y en
if ), queda claro que, en el momento de la ejecución del bucle
blend_points_used ,
blend_points_used siempre será mayor que
p_at_index . Por lo tanto, la condición del bucle siempre es verdadera o el bucle no se ejecuta en absoluto.
Si
blend_points_used - 1 == p_at_index , el bucle no se ejecuta.
En todos los demás casos, la comprobación
i> p_at_index siempre será verdadera, ya que el contador
i sube en cada iteración del bucle.
Parece que el ciclo es eterno, pero no es así.
En primer lugar, se producirá un desbordamiento de enteros de la variable
i (que es un comportamiento indefinido). Esto significa que no debemos confiar en ello.
Si
no estaba
firmado int , entonces, después de que el contador alcanza el mayor valor posible, el operador
i ++ lo convertiría en
0 . Tal comportamiento está definido por el estándar y se denomina "ajuste sin firmar". Sin embargo, debe tener en cuenta que el uso de dicho mecanismo tampoco
es una buena idea .
¡Era el primer punto, pero todavía tenemos el segundo! El caso es que ni siquiera llegaremos a un desbordamiento de enteros. El índice de la matriz se saldrá de los límites mucho antes. Esto significa que se intentará acceder a la memoria fuera del bloque asignado para la matriz. Lo cual es un comportamiento indefinido también. Un ejemplo clásico :)
Puedo darle un par de recomendaciones para que sea más fácil evitar errores similares:
- Escribir código simple y comprensible
- Revise el código más a fondo y escriba más pruebas para el código recién escrito
- Use analizadores estáticos;)
Sexto lugar
Fuente:
Amazon Lumberyard: A Scream of AnguishAquí hay otro ejemplo de la industria de gamedev, es decir, del código fuente del motor AAA de Amazon Lumberyard.
void TranslateVariableNameByOperandType(....) {
Advertencia de PVS-Studio :
V523 La declaración 'then' es equivalente a la declaración 'else'. toglsloperand.c 700
Amazon Lumberyard se desarrolla como un motor multiplataforma. Por esta razón, los desarrolladores intentan admitir tantos compiladores como sea posible. Como podemos ver en los comentarios, un programador Igor vino en contra del compilador Qualcomm.
No sabemos si logró llevar a cabo su tarea y vadear las comprobaciones de compilación "paranoicas", pero dejó un código muy extraño. Lo extraño es que tanto las ramas de la instrucción
if contienen código absolutamente idéntico. Lo más probable es que dicho error se haya producido al usar un método descuidado de copiar y pegar.
Ni siquiera sé qué aconsejar aquí. ¡Así que solo deseo a los desarrolladores de Amazon Lumberyard todo lo mejor para corregir errores y buena suerte para el desarrollador Igor!
Quinto lugar
Fuente: una
vez más, el analizador PVS-Studio ha demostrado ser más atento que una personaUna historia interesante sucedió con el siguiente ejemplo. Mi colega Andrey Karpov estaba preparando un artículo sobre otra verificación del marco Qt. Al escribir algunos errores notables, se topó con la advertencia del analizador, que consideró falsa. Aquí está ese fragmento de código y la advertencia:
QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing)
Advertencia de PVS-Studio: V616 CWE-480 La constante con nombre 'CursorShowing' con el valor de 0 se utiliza en la operación bit a bit. qwindowscursor.cpp 669
Lo que significa que PVS-Studio se quejó en el lugar, lo que obviamente no tuvo ningún error. Es imposible que la constante
CursorShowing sea
0 , ya que solo un par de líneas arriba se inicializan en
1 .
Como Andrey estaba usando una versión de analizador inestable, cuestionó la exactitud de la advertencia. Miró cuidadosamente a través de ese código y aún no encontró un error. Finalmente le dio un falso positivo en el rastreador de errores para que otros colegas pudieran remediar la situación.
Solo un análisis detallado mostró que PVS-Studio resultó ser más cuidadoso que una persona nuevamente. El valor
0x1 se asigna a una constante con nombre llamada
cursorShowing, mientras que
CursorShowing participa en una operación "y" bit a bit. Estas son dos constantes totalmente diferentes, la primera comienza con una letra minúscula, la segunda, con mayúscula.
El código se compila correctamente, porque la clase
QWindowsCursor realmente contiene una constante con este nombre. Aquí está su definición:
class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... }
Si no asigna un valor a una constante enum explícitamente, se inicializará de manera predeterminada. Como
CursorShowing es el primer elemento en la enumeración, se le asignará
0 .
Para evitar tales errores, no debe dar a las entidades nombres demasiado similares. Debería seguir especialmente esta regla si las entidades son del mismo tipo o se pueden lanzar implícitamente entre sí. Como en tales casos, será casi imposible notar el error, pero el código incorrecto aún se compilará y vivirá en una calle fácil dentro de su proyecto.
Cuarto lugar
Fuente:
dispararte en el pie cuando manejes datos de entradaNos estamos acercando a los tres primeros finalistas y el siguiente en la línea es el error del proyecto FreeSWITCH.
static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; break; } .... }
Advertencia de PVS-Studio: V1010 CWE-20 Los datos contaminados no
verificados se usan en el índice: 'strlen (command_buf)'.
El analizador le advierte que algunos datos no verificados se utilizan en la expresión
strlen (command_buf) - 1 . De hecho: si
command_buf es una cadena vacía en términos del lenguaje C (que contiene el único carácter - '\ 0'),
strlen (command_buf) devolverá
0 . En tal caso, se
accederá a command_buf [-1] , que es un comportamiento indefinido. Eso es malo!
El entusiasmo real de este error no es
por qué ocurre, sino
cómo . Este error es uno de esos ejemplos más bonitos, que "tocas" tú mismo, reproduces. Puede ejecutar FreeSwitch, realizar algunas acciones que conducirán a la ejecución del fragmento de código mencionado anteriormente y pasar una cadena vacía a la entrada del programa.
Como resultado, con un movimiento sutil de la mano, un programa de trabajo se convierte en un no trabajo. Puede encontrar los detalles sobre cómo reproducir este error en el artículo de origen en el enlace que figura más arriba. Mientras tanto, déjame darte un resultado revelador:
Tenga en cuenta que los datos de salida pueden ser cualquier cosa, por lo que siempre debe verificarlos. De esta manera, el analizador no se quejará y el programa se volverá más confiable.
Ahora es el momento de buscar a nuestro ganador: ¡estamos en el final del juego ahora! Por cierto, los finalistas de errores ya han esperado una larga espera, luego se aburrieron e incluso comenzaron a ser quisquillosos. ¡Solo mira lo que representaron mientras estábamos fuera!
Tercer lugar
Fuente:
NCBI Genome Workbench: Investigación científica bajo amenazaUn fragmento de código del proyecto NCBI Genome Workbench, que es un conjunto de herramientas para estudiar y analizar datos genéticos, abre los 3 ganadores principales. Aunque no es necesario ser un superhumano modificado genéticamente para encontrar este error, solo unas pocas personas conocen la contingencia de cometer un error aquí.
void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } }
Advertencias de PVS-Studio:- V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'hash'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 365
- V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'passwd_buf'. La función memset_s () debe usarse para borrar los datos privados. challenge.c 366
¿Encontraste un error? Si es así, ¡eres un ataboy! ... o un superhumano genéticamente modificado.
El hecho es que los compiladores de optimización modernos pueden hacer mucho para permitir que un programa construido funcione más rápido. Incluyendo el hecho de que los compiladores ahora pueden rastrear que un búfer, pasado a
memset , no se usa en ningún otro lugar.
En este caso, pueden eliminar la llamada "innecesaria" de
memset , teniendo todos los derechos para eso. Entonces el búfer que almacena datos importantes puede permanecer en la memoria para el deleite de los atacantes.
En este contexto, este comentario geek "con seguridad es mejor ser pedante" suena aún más divertido. A juzgar por una pequeña cantidad de advertencias, dadas para este proyecto, sus desarrolladores hicieron todo lo posible para ser precisos y escribir código seguro. Sin embargo, como podemos ver, uno puede fácilmente pasar por alto tal defecto de seguridad. De acuerdo con la Enumeración de Debilidad Común, este defecto se clasifica como
CWE-14 : Eliminación del Código del Compilador para Borrar Buffers.
Debe usar la función
memset_s () para que la desasignación de memoria sea segura. La función es más segura que
memset () y no puede ser ignorada por un compilador.
Segundo lugar
Fuente:
Cómo PVS-Studio demostró ser más atento que tres programadores y medioUn medallista de plata nos fue enviado amablemente por uno de nuestros clientes. Estaba seguro de que el analizador emitía algunos falsos positivos.
Evgeniy recibió el correo electrónico, lo revisó y lo envió a Svyatoslav. Svyatoslav observó detenidamente el código enviado por el cliente y pensó: "¿cómo es posible que el analizador haya cometido un error tan grande?". Entonces fue a pedir consejo a Andrey. También verificó ese lugar y determinó: de hecho, el analizador generó falsos positivos.
Así sigue, eso necesitaba ser reparado. Solo después de que Svyatoslav comenzó a hacer ejemplos sintéticos para crear la tarea en nuestro rastreador de errores, obtuvo lo que estaba mal.
Ninguno de los programadores pudo encontrar los errores, pero realmente estaban en el código. Hablando francamente, el autor de este artículo tampoco logró encontrarlos a pesar del hecho de que el analizador claramente emitió advertencias sobre lugares erróneos.
¿Encontrarás un error tan astuto? Ponte a prueba con la vigilancia y la atención.
Advertencia de PVS-Studio:- V560 Una parte de la expresión condicional siempre es falsa: (ch> = 0x0FF21). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es verdadera: (ch <= 0x0FF3A). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es falsa: (ch> = 0x0FF41). decodew.cpp 525
- V560 Una parte de la expresión condicional siempre es verdadera: (ch <= 0x0FF5A). decodew.cpp 525
Si lo hiciste, ¡felicitaciones a ti!
El error radica en el hecho de que el operador de negación lógica (!) No se aplica a toda la condición, sino solo a su primera subexpresión:
!((ch >= 0x0FF10) && (ch <= 0x0FF19))
Si esta condición es verdadera, el valor de la variable
ch se encuentra en el rango [0x0FF10 ... 0x0FF19]. Por lo tanto, otras cuatro comparaciones ya no tienen sentido: siempre serán verdaderas o falsas.
Para evitar tales errores, vale la pena seguir algunas reglas. En primer lugar, es muy conveniente e informativo alinear el código como una tabla. En segundo lugar, no debe sobrecargar las expresiones con paréntesis. Por ejemplo, este código podría reescribirse así:
const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19)
De esta manera, habrá menos paréntesis y, por otro lado, lo más probable es que notes un error ocasional.
Aquí viene la guinda del pastel: ¡pasemos al primer lugar!
Primer lugar
Fuente:
Sistema conmocionado: errores interesantes en el código fuente del choque del sistema legendario¡El mejor finalista de hoy es un error del legendario System Shock! Es un juego lanzado hace bastante tiempo en 1994, que se convirtió en un predecesor e inspiración para juegos tan icónicos, como Dead Space, BioShock y Deus Ex.
Pero primero tengo algo que confesar. Lo que voy a mostrar ahora no contiene ningún error. En realidad, ni siquiera es un código, ¡pero no pude evitar compartirlo contigo!
La cuestión es que mientras analizaba el código fuente del juego, mi colega Victoria descubrió muchos comentarios fascinantes. En diferentes fragmentos encontró algunos comentarios ingeniosos e irónicos, e incluso poesía.
Así es como se ven los comentarios que los desarrolladores dejaron en los juegos en los últimos años 90 ... Por cierto, Doug Church, un diseñador jefe de System Shock, también estaba ocupado escribiendo código. Quién sabe, ¿quizás algunos de estos comentarios fueron escritos por él? Espero que las cosas de los hombres en toallas no sean su obra :)
Conclusión
En conclusión, me gustaría agradecer a
mis colegas por buscar nuevos errores y escribir sobre ellos en artículos. Gracias chicos Sin ti, este artículo no sería tan interesante.
También me gustaría contar un poco sobre nuestros logros, ya que durante todo el año no hemos estado ocupados solo buscando errores. También hemos estado desarrollando y mejorando el analizador, lo que resultó en cambios significativos.
Por ejemplo, hemos agregado soporte para varios compiladores nuevos y ampliado la lista de reglas de diagnóstico. También hemos implementado el soporte inicial de los estándares
MISRA C y MISRA C ++ . La nueva característica más importante y lenta fue la compatibilidad con un nuevo idioma. ¡Sí, ahora podemos analizar el código en
Java ! Y lo que es más, tenemos un
ícono renovado :)
También quiero agradecer a nuestros lectores. ¡Gracias por leer nuestros artículos y escribirnos! ¡Eres muy receptivo y eres muy importante para nosotros!
Nuestros 10 errores principales de C ++ de 2018 han llegado a su fin. ¿Qué fragmentos te gustaron más y por qué? ¿Encontraste algunos ejemplos interesantes en 2018?
Todo lo mejor, ¡hasta la próxima!