El invierno está fuera de la ventana, el año tiende a terminar, lo que significa que ha llegado el momento de considerar los errores más interesantes detectados por el analizador PVS-Studio en 2020.
Cabe señalar que el año pasado estuvo marcado por una gran cantidad de nuevas reglas de diagnóstico, cuya activación les permitió ingresar a esta cima. También seguimos mejorando el núcleo del analizador y agregando nuevos escenarios para su uso, puedes leer sobre todo esto en nuestro blog . Si está interesado en otros lenguajes compatibles con nuestro analizador (C, C # y Java), eche un vistazo a los artículos de mis colegas. Ahora vayamos directamente a los errores más memorables encontrados por PVS-Studio durante el año pasado.
Décimo lugar: Módulo división por uno
V1063 La operación módulo por 1 no tiene sentido. El resultado siempre será cero. llvm-stress.cpp 631
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);
}
....
}
El desarrollador quería obtener un valor aleatorio entre 0 y 1 mediante la división de módulo. Sin embargo, una operación como X% 1 siempre devolverá 0. En este caso, sería correcto reescribir la condición de la siguiente manera:
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))
Este error se incluyó en la parte superior del artículo: " Comprobación de Clang 11 con PVS-Studio ".
Noveno lugar: cuatro cheques
PVS-Studio emitió cuatro advertencias para la siguiente sección del código:
- V560 Una parte de la expresión condicional es siempre verdadera: x> = 0. editor.cpp 1137
- V560 Una parte de la expresión condicional siempre es verdadera: y> = 0. editor.cpp 1137
- V560 Una parte de la expresión condicional siempre es verdadera: x <40. Editor.cpp 1137
- V560 Una parte de la expresión condicional es siempre verdadera: y <30. Editor.cpp 1137
int editorclass::at( int x, int y )
{
if(x<0) return at(0,y);
if(y<0) return at(x,0);
if(x>=40) return at(39,y);
if(y>=30) return at(x,29);
if(x>=0 && y>=0 && x<40 && y<30)
{
return contents[x+(levx*40)+vmult[y+(levy*30)]];
}
return 0;
}
Todas las advertencias son para la última declaración if . El problema es que las cuatro comprobaciones que se realizan en él siempre devolverán verdadero . No diría que se trata de un error grave, pero resultó bastante divertido. En general, estas comprobaciones son redundantes y pueden eliminarse.
Este error entró en la parte superior del artículo: " VVVVVV ??? VVVVVV !!! ".
Octavo lugar: eliminar en lugar de eliminar []
V611 La memoria se asignó usando el operador 'new T []' pero se liberó usando el operador 'delete'. Considere inspeccionar este código. Probablemente sea mejor usar 'delete [] poke_data;'. CCDDE.CPP 410
BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
....
char *poke_data = new char [length + 2*sizeof(int)]; // <=
....
if(DDE_Class->Poke_Server( .... ) == FALSE) {
CCDebugString("C&C95 - POKE failed!\n");
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (FALSE);
}
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (TRUE);
}
El analizador ha detectado un error relacionado con el hecho de que la memoria se ha asignado y liberado de formas incompatibles. Para liberar la memoria asignada para una matriz, use el operador delete [] , no delete .
Este error apareció en la parte superior del artículo: " Código del juego Command & Conquer: errores de los años 90. Volumen dos ".
Séptimo lugar: búfer fuera de límites
Veamos la función net_hostname_get que se utilizará a continuación.
#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
#endif
En este caso, durante el preprocesamiento, se seleccionó la opción relacionada con la rama #else . Es decir, en el archivo preprocesado, la función se implementa de la siguiente manera:
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
La función devuelve un puntero a una matriz de 7 bytes (tenga en cuenta el cero terminal al final de la cadena).
Ahora veamos el código que conduce al desbordamiento de la matriz.
static int do_net_init(void)
{
....
(void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
....
}
Advertencia de PVS-Studio: V512 [CWE-119] Una llamada a la función 'memcpy' conducirá a que el búfer 'net_hostname_get ()' quede fuera de rango. log_backend_net.c 114
Después del preprocesamiento, MAX_HOSTNAME_LEN se expande de la siguiente manera:
(void)memcpy(hostname, net_hostname_get(),
sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));
En consecuencia, al copiar datos, se produce un desbordamiento del literal de cadena. Es difícil predecir cómo afectará esto a la ejecución del programa, ya que conduce a un comportamiento indefinido.
Este error llegó al principio del artículo: " Investigando la calidad del código del sistema operativo Zephyr ".
Sexto lugar: Algo muy extraño
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
memcpy(cpy_mntpt, mntpt, strlen(mntpt));
}
return cpy_mntpt;
}
Advertencia de PVS-Studio: V575 [CWE-628] La función 'memcpy' no copia toda la cadena. Utilice la función 'strcpy / strcpy_s' para preservar el nulo del terminal. shell.c 427
Alguien intentó hacer un análogo de la función strdup , pero falló.
Comencemos con la advertencia del analizador. Dice que la función memcpy copia la línea, pero no copiará el cero del terminal, lo cual es muy sospechoso.
Este terminal 0 parece estar copiado aquí:
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
¡Pero no! Aquí hay un error tipográfico que hace que la terminal cero se copie en sí misma. Tenga en cuenta que la escritura va a la matriz mntpt , no a cpy_mntpt . Como resultado, la función mntpt_prepare devuelve una cadena nula de terminal incompleta.
En realidad, el programador quería escribir así:
((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';
Sin embargo, todavía no está claro por qué se hizo tan difícil. Este código se puede simplificar a lo siguiente:
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
strcpy(cpy_mntpt, mntpt);
}
return cpy_mntpt;
}
Este error llegó al principio del artículo antes mencionado: " Examinar la calidad del código del sistema operativo Zephyr ".
Quinto lugar: protección contra desbordamiento incorrecta
V547 [CWE-570] La expresión 'rel_wait <0' siempre es falsa. El valor de tipo sin signo nunca es <0. Os_thread_windows.c 359
static DWORD
get_rel_wait(const struct timespec *abstime)
{
struct __timeb64 t;
_ftime64_s(&t);
time_t now_ms = t.time * 1000 + t.millitm;
time_t ms = (time_t)(abstime->tv_sec * 1000 +
abstime->tv_nsec / 1000000);
DWORD rel_wait = (DWORD)(ms - now_ms);
return rel_wait < 0 ? 0 : rel_wait;
}
En este caso, la variable rel_wait es del tipo DWORD sin firmar . Esto significa que la comparación rel_wait <0 no tiene sentido, ya que el resultado siempre es verdadero.
El error en sí no es muy interesante. Pero resultó interesante cómo intentaron solucionarlo. Resultó que los cambios no fueron arreglados, sino que solo simplificaron el código. Puede leer más sobre esta historia en el artículo de mi colega: " Por qué PVS-Studio no ofrece ediciones automáticas de código ".
El error entró en la parte superior del artículo: " Análisis estático del código de la colección de bibliotecas PMDK de Intel y errores que no son errores ".
Cuarto lugar: no escribas a std, hermano
V1061 La extensión del espacio de nombres 'std' puede resultar en un comportamiento indefinido. size_iterator.hh 210
// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
util::SizedIterator second)
{
util::swap(*first, *second);
}
} // namespace std
El artículo del que se toma el disparador: " Analizando el código del proyecto DeepSpeech o por qué no debería escribir en el espacio de nombres std " describe en detalle por qué no debería hacer esto.
Tercer lugar: barra de desplazamiento, que falló
V501 . Hay subexpresiones idénticas a la izquierda y a la derecha del operador '-': bufferHeight - bufferHeight TermControl.cpp 592
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(bufferHeight - bufferHeight);
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}
Esto es lo que se llama "activación con historia". En este caso, debido a un error, la barra de desplazamiento en la Terminal de Windows no funcionó. Se escribió un artículo completo basado en este error, en el que mi colega realizó una investigación y descubrió por qué sucedió esto. ¿Estás interesado? Aquí está: "La barra de desplazamiento que no pudo ".
Segundo lugar: radio y altura confusos
Y de nuevo hablaremos de varias advertencias del analizador:
- V764 Posible orden incorrecto de los argumentos pasados a la función 'CreateWheel': 'altura' y 'radio'. StandardJoints.cpp 791
- V764 Posible orden incorrecto de los argumentos pasados a la función 'CreateWheel': 'altura' y 'radio'. StandardJoints.cpp 833
- V764 Posible orden incorrecto de los argumentos pasados a la función 'CreateWheel': 'altura' y 'radio'. StandardJoints.cpp 884
Aquí están las llamadas a funciones:
NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);
Y así es como se ve su anuncio:
static NewtonBody* CreateWheel (DemoEntityManager* const scene,
const dVector& location, dFloat radius, dFloat height)
Los argumentos se invirtieron en las llamadas a funciones.
Este error apareció en la parte superior del artículo: " Volver a comprobar Newton Game Dynamics con el analizador estático PVS-Studio ".
Primer lugar: borrar el resultado
V519 A la variable 'color_name' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verifique las líneas: 621, 627. string.cpp 627
static bool parseNamedColorString(const std::string &value,
video::SColor &color)
{
std::string color_name;
std::string alpha_string;
size_t alpha_pos = value.find('#');
if (alpha_pos != std::string::npos) {
color_name = value.substr(0, alpha_pos);
alpha_string = value.substr(alpha_pos + 1);
} else {
color_name = value;
}
color_name = lowercase(value); // <=
std::map<const std::string, unsigned>::const_iterator it;
it = named_colors.colors.find(color_name);
if (it == named_colors.colors.end())
return false;
....
}
Esta función debe analizar el nombre del color con el parámetro de transparencia y devolver su código hexadecimal. Dependiendo del resultado de verificar la condición, el resultado de la división de línea o una copia del argumento de la función se pasa a la variable color_name .
Sin embargo, en la función minúscula () , no es la cadena resultante la que se convierte a minúscula, sino el argumento de la función original. Como resultado, simplemente perderemos el color que parseNamedColorString () debería haber devuelto .
color_name = lowercase(color_name);
Este error apareció en la parte superior del artículo: " PVS-Studio: Análisis de solicitudes de extracción en Azure DevOps utilizando agentes autohospedados ".
Conclusión
Durante el año pasado, hemos encontrado muchos errores en proyectos de código abierto. Estos fueron errores comunes de copiar y pegar, errores constantes, pérdidas de memoria y muchos otros problemas. Nuestro analizador no se detiene, y en la parte superior hay varios aspectos positivos de los nuevos diagnósticos escritos este año.
Espero que hayas disfrutado de los errores recopilados. Personalmente, los encontré bastante interesantes. Pero, por supuesto, su visión puede diferir de la mía, por lo que puede hacer su "Top 10 ..." leyendo artículos de nuestro blog o mirando la lista de errores encontrados por PVS-Studio en proyectos de código abierto.
También traigo a su atención artículos con los 10 errores principales de C ++ de los últimos años: 2016 , 2017 , 2018 , 2019 .
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Vladislav Stolyarov. Los 10 errores principales encontrados en proyectos C ++ en 2020 .