Comprobación de QEMU con PVS-Studio

image1.png


QEMU es una aplicación de emulación bastante conocida. El análisis estático puede ayudar a los desarrolladores de proyectos complejos como QEMU a detectar errores en una etapa temprana y, en general, mejorar su calidad y confiabilidad. En este artículo, se verificará el código fuente de la aplicación QEMU en busca de posibles vulnerabilidades y errores utilizando la herramienta de análisis estático PVS-Studio.



QEMU es un software gratuito diseñado para emular hardware en todas las plataformas. Le permite ejecutar aplicaciones y sistemas operativos en plataformas de hardware distintas del objetivo, por ejemplo, una aplicación escrita para que MIPS se ejecute en la arquitectura x86. QEMU también admite la emulación de una variedad de periféricos como tarjetas de video, usb, etc. El proyecto es bastante complejo y digno de atención, son estos proyectos los que son de interés para el análisis estático, por lo que se decidió verificar su código usando PVS-Studio.



Sobre el análisis



El código fuente del proyecto se puede obtener de un espejo en github . El proyecto es bastante grande y se puede compilar para varias plataformas. Para una verificación de código más fácil, usaremos el sistema de monitoreo de compilación PVS-Studio . Este sistema está diseñado para una integración muy sencilla del análisis estático en casi cualquier plataforma de construcción. El sistema se basa en el seguimiento de las llamadas del compilador durante la compilación y le permite recopilar toda la información para el análisis posterior de los archivos. En otras palabras, simplemente comenzamos la compilación, PVS-Studio recopila la información necesaria y luego comenzamos el análisis: todo es simple. Los detalles se pueden encontrar en el enlace de arriba.



Después de verificar, el analizador encontró muchos problemas potenciales. Para el diagnóstico de propósito general (Análisis general) se obtuvo: 1940 Alto, 1996 Medio, 9596 Bajo. Después de revisar todas las advertencias, se decidió centrarse en los diagnósticos para el primer nivel de confianza (Alto). Se encontraron muchas de estas advertencias (1940), pero la mayoría de las advertencias son del mismo tipo o están asociadas con el uso repetido de una macro sospechosa. Por ejemplo, considere la macro g_new .



#define g_new(struct_type, n_structs)
                        _G_NEW (struct_type, n_structs, malloc)

#define _G_NEW(struct_type, n_structs, func)       \
  (struct_type *) (G_GNUC_EXTENSION ({             \
    gsize __n = (gsize) (n_structs);               \
    gsize __s = sizeof (struct_type);              \
    gpointer __p;                                  \
    if (__s == 1)                                  \
      __p = g_##func (__n);                        \
    else if (__builtin_constant_p (__n) &&         \
             (__s == 0 || __n <= G_MAXSIZE / __s)) \
      __p = g_##func (__n * __s);                  \
    else                                           \
      __p = g_##func##_n (__n, __s);               \
    __p;                                           \
  }))


Para cada uso de esta macro, el analizador emite una advertencia V773 (se salió del alcance de visibilidad del puntero '__p' sin liberar la memoria. Es posible que se produzca una pérdida de memoria). La macro g_new se define en la biblioteca glib, usa la macro _G_NEW , y esta macro a su vez usa otra macro, G_GNUC_EXTENSION , para decirle al compilador GCC que omita las advertencias sobre el código no estándar. Es este código no estándar el que provoca la advertencia del analizador, preste atención a la penúltima línea. En general, la macro está funcionando. Hubo 848 advertencias de este tipo, es decir, casi la mitad de las alertas ocurren en un solo lugar del código.



Todas estas advertencias innecesarias son fáciles de eliminarutilizando la configuración del analizador. Sin embargo, este caso en particular, que encontramos al escribir este artículo, es la razón por la que nuestro equipo modifica ligeramente la lógica del analizador para tales situaciones.



Por lo tanto, una gran cantidad de advertencias no siempre significa una mala calidad del código. Sin embargo, hay algunos lugares realmente sospechosos. Bueno, vayamos a las advertencias.



Advertencia N1



V517 Se detectó el uso del patrón 'if (A) {...} else if (A) {...}'. Existe una probabilidad de presencia de errores lógicos. Verifique las líneas: 2395, 2397. megasas.c 2395



#define MEGASAS_MAX_SGE 128             /* Firmware limit */
....
static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
{
  ....
  if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
    ....
  } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
    ....
  }
  ....
}


Cualquier uso de números "mágicos" en el código siempre es sospechoso. Aquí hay dos condiciones, y a primera vista parecen ser diferentes, pero si observa el valor de la macro MEGASAS_MAX_SGE , resulta que las condiciones se duplican entre sí. Lo más probable es que haya un error tipográfico aquí y en lugar de 128 debería haber otro número. Por supuesto, este es el problema de todos los números "mágicos", solo necesita sellarlos cuando los use. El uso de macros y constantes ayuda mucho al desarrollador en este caso.



Advertencia N2



V523 La instrucción 'then' es equivalente a la instrucción 'else'. cp0_helper.c 383



target_ulong helper_mftc0_cause(CPUMIPSState *env)
{
  ....
  CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);

  if (other_tc == other->current_tc) {
    tccause = other->CP0_Cause;
  } else {
    tccause = other->CP0_Cause;
  }
  ....
}


En el código considerado, los cuerpos then y else del operador condicional son idénticos. Lo más probable es que copie y pegue aquí. Sólo tienes que copiar el cuerpo entonces la rama , y la solución olvidado. Se puede suponer que se debería haber utilizado env en lugar de otro . La solución para este lugar sospechoso podría verse así:



if (other_tc == other->current_tc) {
  tccause = other->CP0_Cause;
} else {
  tccause = env->CP0_Cause;
}


Solo los desarrolladores de este código pueden decir de manera inequívoca cómo debería ser realmente. Otro lugar similar:



  • V523 La instrucción 'entonces' es equivalente a la instrucción 'else'. translate.c 641


Advertencia N3



V547 La expresión 'ret <0' siempre es falsa. qcow2-cluster.c 1557



static int handle_dependencies(....)
{
  ....
  if (end <= old_start || start >= old_end) {
    ....
  } else {

    if (bytes == 0 && *m) {
      ....
      return 0;           // <= 3
    }

    if (bytes == 0) {
      ....
      return -EAGAIN;     // <= 4
    }
  ....
  }
  return 0;               // <= 5
}

int qcow2_alloc_cluster_offset(BlockDriverState *bs, ....)
{
  ....
  ret = handle_dependencies(bs, start, &cur_bytes, m);
  if (ret == -EAGAIN) {   // <= 2
    ....
  } else if (ret < 0) {   // <= 1
    ....
  }
}


Aquí, el analizador encontró que la condición (comentario 1) nunca se cumpliría. El valor de la variable ret se inicializa con el resultado de ejecutar la función handle_dependencies , esta función solo devuelve 0 o -EAGAIN (comentarios 3, 4, 5). Un poco más arriba, en la primera condición, verificamos el valor de ret con -EAGAIN (comentario 2), por lo que el resultado de la expresión ret <0 siempre será falso. Quizás la función handle_dependencies solía devolver diferentes valores, pero más tarde, como resultado de, por ejemplo, la refactorización, el comportamiento cambió. Aquí solo necesitas completar la refactorización. Activadores similares:



  • V547 La expresión siempre es falsa. qcow2.c 1070
  • V547 La expresión 's-> state! = MIGRATION_STATUS_COLO' siempre es falsa. colo.c 595
  • V547 La expresión 's-> metadata_entries.present & 0x20' siempre es falsa. vhdx.c 769


Advertencia N4



V557 Array overrun is possible. La función 'dwc2_glbreg_read' procesa el valor '[0..63]'. Examine el tercer argumento. Verifique las líneas: 667, 1040.hcd-dwc2.c 667



#define HSOTG_REG(x) (x)                                             // <= 5
....
struct DWC2State {
  ....
#define DWC2_GLBREG_SIZE    0x70
  uint32_t glbreg[DWC2_GLBREG_SIZE / sizeof(uint32_t)];              // <= 1
  ....
}
....
static uint64_t dwc2_glbreg_read(void *ptr, hwaddr addr, int index,
                                 unsigned size)
{
  ....
  val = s->glbreg[index];                                            // <= 2
  ....
}
static uint64_t dwc2_hsotg_read(void *ptr, hwaddr addr, unsigned size)
{
  ....
  switch (addr) {
    case HSOTG_REG(0x000) ... HSOTG_REG(0x0fc):                      // <= 4
        val = dwc2_glbreg_read(ptr, addr,
                              (addr - HSOTG_REG(0x000)) >> 2, size); // <= 3
    ....
  }
  ....
}


Existe un problema potencial con los desbordamientos de matrices en este código. En la estructura DWC2State define la matriz glbreg , que consta de 28 elementos (comentario 1). En la función dwc2_glbreg_read , nos referimos a nuestra matriz por índice (comentario 2). Ahora, observe que la expresión ( addr - HSOTG_REG (0x000)) >> 2 (comentario 3) se pasa como un índice a la función dwc2_glbreg_read , que puede tomar un valor en el rango [0..63]. Para estar convencido de esto, preste atención a los comentarios 4 y 5. Quizás, aquí sea necesario ajustar el rango de valores del comentario 4. Más disparadores similares:







  • V557 Array overrun es posible. La función 'dwc2_hreg0_read' procesa el valor '[0..63]'. Examine el tercer argumento. Verifique las líneas: 814, 1050.hcd-dwc2.c 814
  • V557 Array overrun es posible. La función 'dwc2_hreg1_read' procesa el valor '[0..191]'. Examine el tercer argumento. Verifique las líneas: 927, 1053.hcd-dwc2.c 927
  • V557 Array overrun es posible. La función 'dwc2_pcgreg_read' procesa el valor '[0..127]'. Examine el tercer argumento. Verifique las líneas: 1012, 1060.hcd-dwc2.c 1012


Advertencia N5



V575 La función 'strerror_s' procesa elementos '0'. Examine el segundo argumento. comandos-win32.c 1642



void qmp_guest_set_time(bool has_time, int64_t time_ns, 
                        Error **errp)
{
  ....
  if (GetLastError() != 0) {
    strerror_s((LPTSTR) & msg_buffer, 0, errno);
    ....
  }
}


La función strerror_s devuelve una descripción textual del código de error del sistema. Su firma se ve así:



errno_t strerror_s( char *buf, rsize_t bufsz, errno_t errnum );


El primer parámetro es un puntero al búfer donde se copiará la descripción del texto, el segundo parámetro es el tamaño del búfer y el tercero es el código de error. En el código, se pasa 0 como el tamaño del búfer, esto es claramente un valor erróneo. Por cierto, es posible saber de antemano cuántos bytes deben asignarse: solo necesita llamar a strerrorlen_s , que devuelve la longitud del texto de descripción del error. Este valor se puede utilizar para asignar un búfer de tamaño suficiente.



Advertencia N6



V595 El puntero 'blen2p' se utilizó antes de que se verificara con nullptr. Verifique las líneas: 103, 106.dsound_template.h 103



static int glue (
    ....
    DWORD *blen1p,
    DWORD *blen2p,
    int entire,
    dsound *s
    )
{
  ....
  dolog("DirectSound returned misaligned buffer %ld %ld\n",
        *blen1p, *blen2p);                         // <= 1
  glue(.... p2p ? *p2p : NULL, *blen1p,
                            blen2p ? *blen2p : 0); // <= 2
....
}


En este código, primero se usa el valor del argumento blen2p (comentario 1) y luego se verifica si hay nullptr (comentario 2). Este lugar altamente sospechoso parece que se olvidó de insertar el cheque antes del primer uso (comentario 1). Como solución, solo agregue un cheque:



dolog("DirectSound returned misaligned buffer %ld %ld\n",
      *blen1p, blen2p ? *blen2p : 0);


Todavía hay una pregunta sobre el argumento blen1p . Probablemente, también puede ser un puntero nulo, y aquí también necesitará agregar un cheque. Varios aspectos positivos más similares:



  • V595 El puntero 'ref' se utilizó antes de que se verificara con nullptr. Líneas de control: 2191, 2193.uri.c 2191
  • V595 El puntero 'cmdline' se utilizó antes de que se verificara con nullptr. Líneas de control: 420, 425.qemu-io.c 420
  • V595 El puntero 'dp' se utilizó antes de que se verificara con nullptr. Verifique las líneas: 288, 294. onenand.c 288
  • V595 El puntero 'omap_lcd' se utilizó antes de que se verificara con nullptr. Verifique las líneas: 81, 87. omap_lcdc.c 81


Advertencia N7



V597 El compilador podría eliminar la llamada a la función 'memset', que se usa para vaciar el objeto 'op_info'. La función RtlSecureZeroMemory () debe usarse para borrar los datos privados. virtio-crypto.c 354



static void virtio_crypto_free_request(VirtIOCryptoReq *req)
{
  if (req) {
    if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
      ....
      /* Zeroize and free request data structure */
      memset(op_info, 0, sizeof(*op_info) + max_len); // <= 1
      g_free(op_info);
    }
    g_free(req);
  }
}


En este fragmento de código, se llama a la función memset para el objeto op_info (comentario 1), después de lo cual op_info se elimina inmediatamente, es decir, después de la limpieza, este objeto no se modifica en ningún otro lugar. Este es exactamente el caso cuando el compilador puede eliminar la llamada a memset durante el proceso de optimización . Para eliminar este comportamiento potencial, puede utilizar funciones especiales que el compilador nunca elimina. Consulte también el artículo " Eliminación segura de datos privados ".



Advertencia N8



V610 Comportamiento no especificado. Verifique el operador de turno '>>'. El operando izquierdo es negativo ('número' = [-32768..2147483647]). cris.c 2111



static void
print_with_operands (const struct cris_opcode *opcodep,
         unsigned int insn,
         unsigned char *buffer,
         bfd_vma addr,
         disassemble_info *info,
         const struct cris_opcode *prefix_opcodep,
         unsigned int prefix_insn,
         unsigned char *prefix_buffer,
         bfd_boolean with_reg_prefix)
{
  ....
  int32_t number;
  ....
  if (signedp && number > 127)
    number -= 256;            // <= 1
  ....
  if (signedp && number > 32767)
    number -= 65536;          // <= 2
  ....
  unsigned int highbyte = (number >> 24) & 0xff;
  ....
}


Dado que el número de variable puede ser negativo, el desplazamiento a la derecha bit a bit es un comportamiento no especificado. Para asegurarse de que la variable en cuestión pueda tomar un valor negativo, consulte los comentarios 1 y 2. Para eliminar las diferencias en el comportamiento de su código en diferentes plataformas, estos casos deben evitarse.



Más advertencias:



  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo es negativo ('(hclk_div - 1)' = [-1..15]). aspeed_smc.c 1041
  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo '(target_long) - 1' es negativo. exec-vary.c 99
  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo es negativo ('hex2nib (palabras [3] [i * 2 + 2])' = [-1..15]). qtest.c 561


También hay varias advertencias del mismo tipo, solo se usa -1 como operando izquierdo .



V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo '-1' es negativo. hppa.c 2702



int print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
{
  ....
  disp = (-1 << 10) | imm10;
  ....
}


Otras advertencias similares:



  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo '-1' es negativo. hppa.c 2718
  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo '-0x8000' es negativo. fmopl.c 1022
  • V610 Comportamiento indefinido. Compruebe el operador de turno '<<'. El operando izquierdo '(intptr_t) - 1' es negativo. sve_helper.c 889


Advertencia N9



V616 La constante denominada 'TIMER_NONE' con el valor 0 se utiliza en la operación bit a bit. sys_helper.c 179



#define HELPER(name) ....

enum {
  TIMER_NONE = (0 << 30),        // <= 1
  ....
}

void HELPER(mtspr)(CPUOpenRISCState *env, ....)
{
  ....
  if (env->ttmr & TIMER_NONE) {  // <= 2
    ....
  }
}


Puede verificar fácilmente que el valor de la macro TIMER_NONE es cero (comentario 1). Además, esta macro se usa en una operación bit a bit, cuyo resultado siempre será 0. Como resultado, el cuerpo de la declaración condicional if (env-> ttmr & TIMER_NONE) nunca se ejecutará.



Advertencia N10



V629 Considere inspeccionar la expresión 'n << 9'. Desplazamiento de bits del valor de 32 bits con una posterior expansión al tipo de 64 bits. qemu-img.c 1839



#define BDRV_SECTOR_BITS   9
static int coroutine_fn convert_co_read(ImgConvertState *s, 
                  int64_t sector_num, int nb_sectors, uint8_t *buf)
{
  uint64_t single_read_until = 0;
  int n;
  ....
  while (nb_sectors > 0) {
    ....
    uint64_t offset;
    ....
    single_read_until = offset + (n << BDRV_SECTOR_BITS);
    ....
  }
  ....
}


En este fragmento de código , se realiza una operación de desplazamiento en la variable n , que tiene un tipo con signo de 32 bits, luego este resultado con signo de 32 bits se expande a un tipo con signo de 64 bits y luego, como un tipo sin signo, se agrega al desplazamiento de la variable de 64 bits sin signo . Suponga que en el momento en que se ejecuta la expresión, la variable n tiene algunos de los 9 bits más significativos. Estamos realizando una operación de desplazamiento de 9 bits ( BDRV_SECTOR_BITS), y esto, a su vez, es un comportamiento indefinido, entonces, como resultado, podemos obtener el bit establecido en el bit más significativo. Recuerde que este bit en el tipo firmado es responsable del signo, es decir, el resultado puede volverse negativo. Dado que n es una variable con signo, el signo se tendrá en cuenta al expandir. Luego, el resultado se agrega a la variable de compensación . A partir de estas consideraciones, es fácil ver que el resultado de ejecutar la expresión puede diferir del deseado. Una de las posibles soluciones es reemplazar el tipo de la variable n con un tipo sin signo de 64 bits, es decir, con uint64_t .



Aquí hay algunos desencadenantes más similares:



  • V629 Consider inspecting the '1 << refcount_order' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2.c 3204
  • V629 Consider inspecting the 's->cluster_size << 3' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2-bitmap.c 283
  • V629 Consider inspecting the 'i << s->cluster_bits' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. qcow2-cluster.c 983
  • V629 Consider inspecting the expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. vhdx.c 1145
  • V629 Consider inspecting the 'delta << 2' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mips.c 4341


Advertencia N11



V634 La prioridad de la operación '*' es mayor que la de la operación '<<'. Es posible que se deban usar paréntesis en la expresión. nand.c 310



static void nand_command(NANDFlashState *s)
{
  ....
  s->addr &= (1ull << s->addrlen * 8) - 1;
  ....
}


Es solo un lugar sospechoso. No está claro qué quería hacer el programador al principio: cambio o multiplicación. Incluso si no hay ningún error aquí, debe volver a mirar el código y colocar los corchetes correctamente. Este es solo uno de esos lugares en los que los desarrolladores deberían mirar para asegurarse de que su algoritmo sea correcto. Otros lugares similares:



  • V634 La prioridad de la operación '*' es mayor que la de la operación '<<'. Es posible que se deban usar paréntesis en la expresión. exynos4210_mct.c 449
  • V634 La prioridad de la operación '*' es mayor que la de la operación '<<'. Es posible que se deban usar paréntesis en la expresión. exynos4210_mct.c 1235
  • V634 La prioridad de la operación '*' es mayor que la de la operación '<<'. Es posible que se deban usar paréntesis en la expresión. exynos4210_mct.c 1264


Advertencia N12



V646 Considere inspeccionar la lógica de la aplicación. Es posible que falte la palabra clave "más". pl181.c 400



static void pl181_write(void *opaque, hwaddr offset,
                        uint64_t value, unsigned size)
{
  ....
  if (s->cmd & PL181_CMD_ENABLE) {
    if (s->cmd & PL181_CMD_INTERRUPT) {
      ....
    } if (s->cmd & PL181_CMD_PENDING) { // <= else if
      ....
    } else {
      ....
    }
    ....
  }
  ....
}


En este código, a juzgar por el formato, el uso de else if en lugar de if se sugiere directamente . Quizás se olvidaron de agregar más aquí . Entonces la opción de corrección podría ser así:



} else if (s->cmd & PL181_CMD_PENDING) { // <= else if


Sin embargo, existe la posibilidad de que todo esté en orden con este código y haya un formato incorrecto del texto del programa, lo cual es confuso. Entonces, el código podría verse así:



if (s->cmd & PL181_CMD_INTERRUPT) {
  ....
}
if (s->cmd & PL181_CMD_PENDING) { // <= if
  ....
} else {
  ....
}


Advertencia N13



V773 Se salió de la función sin soltar el puntero de 'regla'. Es posible una pérdida de memoria. blkdebug.c 218



static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
{
  ....
  struct BlkdebugRule *rule;
  ....
  rule = g_malloc0(sizeof(*rule));                   // <= 1
  ....
  if (local_error) {
    error_propagate(errp, local_error);
    return -1;                                       // <= 2
  }
  ....
  /* Add the rule */
  QLIST_INSERT_HEAD(&s->rules[event], rule, next);   // <= 3
  ....
}


En este código, el objeto de regla (comentario 1) se selecciona y se agrega a la lista para su uso posterior (comentario 3), pero en caso de error, la función regresa sin eliminar el objeto de regla creado previamente (comentario 2). Aquí solo necesita manejar correctamente el error: elimine el objeto creado anteriormente, de lo contrario habrá una pérdida de memoria.



Advertencia N14



V781 El valor del índice 'ix' se comprueba después de su uso. Quizás haya un error en la lógica del programa. uri.c 2110



char *uri_resolve_relative(const char *uri, const char *base)
{
  ....
  ix = pos;
  if ((ref->path[ix] == '/') && (ix > 0)) {
  ....
}


Aquí, el analizador ha detectado una matriz potencial fuera de límites. Primero, se lee el elemento de la matriz ref-> path en el índice ix , y luego se verifica que ix sea correcto ( ix> 0 ). La solución correcta aquí es revertir estas acciones:



if ((ix > 0) && (ref->path[ix] == '/')) {


Había varios de esos lugares:



  • V781 El valor del índice 'ix' se comprueba después de su uso. Quizás haya un error en la lógica del programa. uri.c 2112
  • V781 El valor del índice de 'compensación' se comprueba después de su uso. Quizás haya un error en la lógica del programa. keymaps.c 125
  • V781 El valor de la variable 'calidad' se comprueba después de su uso. Quizás haya un error en la lógica del programa. Verifique las líneas: 326, 335.vnc-enc-tight.c 326
  • V781 El valor del índice 'i' se verifica después de su uso. Quizás haya un error en la lógica del programa. mem_helper.c 1929


Advertencia N15



V784 El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto provocará la pérdida de bits más altos. cadence_gem.c 1486



typedef struct CadenceGEMState {
  ....
  uint32_t regs_ro[CADENCE_GEM_MAXREG];
}
....
static void gem_write(void *opaque, hwaddr offset, uint64_t val,
        unsigned size)
{
  ....
  val &= ~(s->regs_ro[offset]);
  ....
}


Este código realiza una operación bit a bit en objetos de diferentes tipos. El operando izquierdo es el argumento val , que es un tipo sin signo de 64 bits. El valor recibido del elemento de matriz s-> regs_ro en el índice de desplazamiento , que tiene un tipo sin signo de 32 bits, se utiliza como operando derecho . El resultado de la operación en el lado derecho (~ (s-> regs_ro [offset])) es un tipo sin signo de 32 bits, y antes de la multiplicación bit a bit se expandirá al tipo de 64 bits con ceros, es decir, después de evaluar la expresión completa, todos los bits de orden superior de la variable val se pondrán a cero . Estos lugares siempre parecen sospechosos. Aquí solo podemos recomendar a los desarrolladores que vuelvan a revisar este código. Más similar:



  • V784 El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto provocará la pérdida de bits más altos. xlnx-zynq-devcfg.c 199
  • V784 El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto provocará la pérdida de bits más altos. soc_dma.c 214
  • V784 El tamaño de la máscara de bits es menor que el tamaño del primer operando. Esto provocará la pérdida de bits más altos. fpu_helper.c 418


Advertencia N16



V1046 Uso inseguro de los tipos 'bool' e 'unsigned int' juntos en la operación '& ='. helper.c 10821



static inline uint32_t extract32(uint32_t value, int start, int length);
....
static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
                                          ARMMMUIdx mmu_idx)
{
  ....
  bool epd, hpd;
  ....
  hpd &= extract32(tcr, 6, 1);
}


En este fragmento de código, se realiza una operación AND bit a bit sobre la variable hpd de tipo bool y el resultado de ejecutar la función extract32 , que tiene el tipo uint32_t . Dado que el valor de bit de una variable booleana solo puede ser 0 o 1, el resultado de la expresión siempre será falso si el bit menos significativo devuelto por la función extract32 es cero. Veamos esto con un ejemplo. Suponga que hpd es verdadero y la función devuelve 2, es decir, en representación binaria, la operación se verá como 01 y 10 = 0, y el resultado de la expresión será falso . Lo más probable es que el programador quisiera establecer el valor en verdaderosi la función devuelve algo distinto de cero. Aparentemente, el código debe corregirse para que el resultado de la función se transmita al tipo bool , por ejemplo, así:



hpd = hpd && (bool)extract32(tcr, 6, 1);


Conclusión



Como puede ver, el analizador encontró muchos lugares sospechosos. Quizás los problemas potenciales encontrados aún no se hayan manifestado de ninguna manera, pero su presencia no puede dejar de ser alarmante, ya que son capaces de disparar en el momento más inesperado. Ver todos los lugares sospechosos de antemano y corregirlos es mejor que corregir un flujo interminable de errores. Obviamente, para proyectos complejos como este, el análisis estático puede traer beneficios tangibles, especialmente si organiza una revisión regular del proyecto. Si desea probar PVS-Studio para su proyecto, puede descargar el analizador y obtener una clave de prueba gratuita en esta página.





Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Evgeniy Ovsannikov. Comprobando QEMU usando PVS-Studio .



All Articles