👩‍👧‍👦 💷 🦃 QEMU mit PVS-Studio prüfen 🤚🏻 🏯 🎤

QEMU ist eine ziemlich bekannte Emulationsanwendung. Die statische Analyse kann Entwicklern komplexer Projekte wie QEMU helfen, Fehler frühzeitig zu erkennen und im Allgemeinen ihre Qualität und Zuverlässigkeit zu verbessern. In diesem Artikel wird der Quellcode der QEMU-Anwendung mithilfe des statischen Analysetools PVS-Studio auf potenzielle Schwachstellen und Fehler überprüft.

QEMU ist eine kostenlose Software zur plattformübergreifenden Emulation von Hardware. Sie können damit Anwendungen und Betriebssysteme auf Hardwareplattformen ausführen, die sich vom Ziel unterscheiden, z. B. eine Anwendung, die für MIPS geschrieben wurde, um für die x86-Architektur ausgeführt zu werden. QEMU unterstützt auch die Emulation einer Vielzahl von Peripheriegeräten wie Grafikkarten, USB usw. Das Projekt ist recht komplex und verdient Aufmerksamkeit. Es sind solche Projekte, die für die statische Analyse von Interesse sind. Daher wurde beschlossen, den Code mit PVS-Studio zu überprüfen.

Über die Analyse

Der Quellcode des Projekts kann von einem Spiegel auf Github bezogen werden . Das Projekt ist ziemlich groß und kann für verschiedene Plattformen kompiliert werden. Zur einfacheren Codeüberprüfung verwenden wir das PVS-Studio-Kompilierungsüberwachungssystem . Dieses System ist für die sehr einfache Integration der statischen Analyse in nahezu jede Build-Plattform ausgelegt. Das System basiert auf der Verfolgung von Compiler-Aufrufen während der Erstellung und ermöglicht es Ihnen, alle Informationen für die nachfolgende Analyse von Dateien zu sammeln. Mit anderen Worten, wir starten einfach den Build, PVS-Studio sammelt die erforderlichen Informationen und dann starten wir die Analyse - alles ist einfach. Details finden Sie unter dem obigen Link.

Nach der Überprüfung stellte der Analysator viele potenzielle Probleme fest. Für die Allzweckdiagnostik (Allgemeine Analyse) wurde erhalten: 1940 hoch, 1996 mittel, 9596 niedrig. Nachdem alle Warnungen überprüft worden waren, wurde beschlossen, sich auf die Diagnose für das erste Konfidenzniveau (Hoch) zu konzentrieren. Es wurden ziemlich viele solcher Warnungen gefunden (1940), aber die meisten Warnungen sind entweder vom gleichen Typ oder mit der wiederholten Verwendung eines verdächtigen Makros verbunden. Betrachten Sie beispielsweise das Makro 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;                                           \
  }))

Bei jeder Verwendung dieses Makros gibt der Analysator die Warnung V773 aus (der Sichtbarkeitsbereich des Zeigers '__p' wurde verlassen, ohne den Speicher freizugeben. Ein Speicherverlust ist möglich). Das g_new- Makro ist in der glib-Bibliothek definiert, es verwendet das _G_NEW- Makro und dieses Makro verwendet wiederum ein anderes Makro, G_GNUC_EXTENSION , um den GCC-Compiler anzuweisen , die Warnungen über nicht standardmäßigen Code zu überspringen. Es ist dieser nicht standardmäßige Code, der die Warnung des Analysators verursacht. Achten Sie auf die vorletzte Zeile. Im Allgemeinen funktioniert das Makro. Es gab 848 Warnungen dieses Typs, dh fast die Hälfte der Warnungen tritt an nur einer Stelle im Code auf.

Alle diese unnötigen Warnungen sind leicht zu entfernenVerwenden der Analysatoreinstellungen. Dieser spezielle Fall, auf den wir beim Schreiben dieses Artikels gestoßen sind, ist jedoch der Grund für unser Team, die Analysatorlogik für solche Situationen geringfügig zu ändern.

Eine große Anzahl von Warnungen bedeutet daher nicht immer eine schlechte Codequalität. Es gibt jedoch einige wirklich verdächtige Orte. Kommen wir zu den Warnungen.

Warnung N1

V517 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Überprüfen Sie die Zeilen: 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) {
    ....
  }
  ....
}

Jede Verwendung von "magischen" Zahlen im Code ist immer verdächtig. Hier gibt es zwei Bedingungen, und auf den ersten Blick scheinen sie unterschiedlich zu sein. Wenn Sie sich jedoch den Wert des Makros MEGASAS_MAX_SGE ansehen , stellt sich heraus, dass sich die Bedingungen gegenseitig duplizieren. Höchstwahrscheinlich gibt es hier einen Tippfehler und anstelle von 128 sollte es eine andere Nummer geben. Dies ist natürlich das Problem aller "magischen" Zahlen. Sie müssen sie nur versiegeln, wenn Sie sie verwenden. Die Verwendung von Makros und Konstanten hilft dem Entwickler in diesem Fall sehr.

Warnung N2

V523 Die Anweisung 'then' entspricht der Anweisung '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;
  }
  ....
}

In dem betrachteten Code sind die damaligen und sonstigen Körper des bedingten Operators identisch. Höchstwahrscheinlich hier kopieren und einfügen. Kopieren Sie einfach den Körper dann den Zweig , und fix vergessen. Es kann angenommen werden , dass env verwendet wurden , sollte anstelle des anderen Objekt . Das Update für diesen verdächtigen Ort könnte folgendermaßen aussehen:

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

Nur die Entwickler dieses Codes können eindeutig sagen, wie es eigentlich sein soll. Ein anderer ähnlicher Ort:

V523 Die Anweisung 'then' entspricht der Anweisung 'else'. translate.c 641

Warnung N3

V547 Der Ausdruck 'ret <0' ist immer falsch. 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
    ....
  }
}

Hier stellte der Analysator fest, dass die Bedingung (Kommentar 1) niemals erfüllt sein würde. Der Wert der Variablen ret wird mit dem Ergebnis der Ausführung der Funktion handle_dependencies initialisiert. Diese Funktion gibt nur 0 oder -EAGAIN zurück (Kommentare 3, 4, 5). Etwas höher haben wir in der ersten Bedingung den Wert von ret gegen -EAGAIN (Kommentar 2) geprüft , sodass das Ergebnis des Ausdrucks ret <0 immer falsch ist. Möglicherweise hat die Funktion handle_dependencies verwendet, um unterschiedliche Werte zurückzugeben, aber später hat sich das Verhalten beispielsweise aufgrund von Refactoring geändert. Hier müssen Sie nur das Refactoring abschließen. Ähnliche Auslöser:

V547 Ausdruck ist immer falsch. qcow2.c 1070
V547 Ausdruck 's-> state! = MIGRATION_STATUS_COLO' ist immer falsch. colo.c 595
V547 Der Ausdruck 's-> metadata_entries.present & 0x20' ist immer falsch. vhdx.c 769

Warnung N4

V557 Array-Überlauf ist möglich. Die Funktion 'dwc2_glbreg_read' verarbeitet den Wert '[0..63]'. Untersuchen Sie das dritte Argument. Überprüfen Sie die Zeilen: 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
    ....
  }
  ....
}

In diesem Code liegt möglicherweise ein Problem mit Array-Überläufen vor. In der Struktur DWC2State definiert Array glbreg , bestehend aus 28 Elementen (Kommentar 1). In der Funktion dwc2_glbreg_read verweisen wir nach Index auf unser Array (Kommentar 2). Beachten Sie nun, dass der Ausdruck ( addr - HSOTG_REG (0x000)) >> 2 (Kommentar 3) als Index an die Funktion dwc2_glbreg_read übergeben wird , die einen Wert im Bereich [0..63] annehmen kann. Um davon überzeugt zu sein, beachten Sie die Kommentare 4 und 5. Vielleicht ist es hier erforderlich, den Wertebereich von Kommentar 4 anzupassen. Weitere ähnliche Auslöser:

V557 Array-Überlauf ist möglich. Die Funktion 'dwc2_hreg0_read' verarbeitet den Wert '[0..63]'. Untersuchen Sie das dritte Argument. Überprüfen Sie die Zeilen: 814, 1050.hcd-dwc2.c 814
V557 Array-Überlauf ist möglich. Die Funktion 'dwc2_hreg1_read' verarbeitet den Wert '[0..191]'. Untersuchen Sie das dritte Argument. Überprüfen Sie die Zeilen: 927, 1053.hcd-dwc2.c 927
V557 Array-Überlauf ist möglich. Die Funktion 'dwc2_pcgreg_read' verarbeitet den Wert '[0..127]'. Untersuchen Sie das dritte Argument. Überprüfen Sie die Zeilen: 1012, 1060.hcd-dwc2.c 1012

Warnung N5

V575 Die Funktion 'strerror_s' verarbeitet '0'-Elemente. Untersuchen Sie das zweite Argument. Befehle-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);
    ....
  }
}

Die Funktion strerror_s gibt eine Textbeschreibung des Systemfehlercodes zurück. Die Unterschrift sieht folgendermaßen aus:

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

Der erste Parameter ist ein Zeiger auf den Puffer, in den die Textbeschreibung kopiert wird, der zweite Parameter ist die Puffergröße, der dritte ist der Fehlercode. Im Code wird 0 als Größe des Puffers übergeben, dies ist eindeutig ein fehlerhafter Wert. Übrigens ist es möglich, im Voraus zu wissen, wie viele Bytes zugewiesen werden sollen: Sie müssen nur strerrorlen_s aufrufen , das die Länge der Textbeschreibung des Fehlers zurückgibt. Dieser Wert kann verwendet werden, um einen Puffer mit ausreichender Größe zuzuweisen.

Warnung N6

V595 Der Zeiger 'blen2p' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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
....
}

In diesem Code wird zuerst der Wert des Arguments blen2p verwendet (Kommentar 1) und dann auf nullptr überprüft (Kommentar 2). Dieser höchst verdächtige Ort sieht so aus, als hätten Sie vergessen, den Scheck vor der ersten Verwendung einzufügen (Kommentar 1). Fügen Sie zur Behebung einfach einen Scheck hinzu:

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

Es gibt noch eine Frage zum Argument blen1p . Wahrscheinlich kann es auch ein Nullzeiger sein, und hier müssen Sie auch einen Scheck hinzufügen. Mehrere ähnliche Positive:

V595 Der 'ref'-Zeiger wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 2191, 2193.uri.c 2191
V595 Der Zeiger 'cmdline' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 420, 425.qemu-io.c 420
V595 Der Zeiger 'dp' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 288, 294. onenand.c 288
V595 Der Zeiger 'omap_lcd' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 81, 87. omap_lcdc.c 81

Warnung N7

V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'op_info' gelöscht wird. Die Funktion RtlSecureZeroMemory () sollte verwendet werden, um die privaten Daten zu löschen. 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);
  }
}

In diesem Codefragment wird die Memset- Funktion für das op_info- Objekt aufgerufen (Kommentar 1). Danach wird die op_info sofort gelöscht, dh nach dem Bereinigen wird dieses Objekt nirgendwo anders geändert. Dies ist genau dann der Fall, wenn der Compiler den Memset- Aufruf während des Optimierungsprozesses entfernen kann . Um dieses potenzielle Verhalten zu beseitigen, können Sie spezielle Funktionen verwenden, die der Compiler niemals entfernt. Siehe auch den Artikel " Private Daten sicher löschen ".

N8

V610 Nicht spezifizierte Verhaltenswarnung. Überprüfen Sie den Schaltoperator '>>'. Der linke Operand ist negativ ('number' = [-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;
  ....
}

Da die variable Zahl negativ sein kann, die bitweise Verschiebung nach rechts ist nicht spezifiziert Verhalten. Um sicherzustellen, dass die betreffende Variable einen negativen Wert annehmen kann, lesen Sie die Kommentare 1 und 2. Um Unterschiede im Verhalten Ihres Codes auf verschiedenen Plattformen zu vermeiden, sollten solche Fälle vermieden werden.

Weitere Warnungen:

V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtoperator '<<'. Der linke Operand ist negativ ('(hclk_div - 1)' = [-1..15]). aspeed_smc.c 1041
V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtoperator '<<'. Der linke Operand '(target_long) - 1' ist negativ. exec-vari.c 99
V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtführer '<<'. Der linke Operand ist negativ ('hex2nib (Wörter [3] [i * 2 + 2])' = [-1..15]). qtest.c 561

Es gibt auch mehrere Warnungen desselben Typs, nur -1 wird als linker Operand verwendet .

V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtoperator '<<'. Der linke Operand '-1' ist negativ. hppa.c 2702

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

Andere ähnliche Warnungen:

V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtführer '<<'. Der linke Operand '-1' ist negativ. hppa.c 2718
V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtoperator '<<'. Der linke Operand '-0x8000' ist negativ. fmopl.c 1022
V610 Undefiniertes Verhalten. Überprüfen Sie den Schichtführer '<<'. Der linke Operand '(intptr_t) - 1' ist negativ. sve_helper.c 889

Warnung N9

V616 Bei der bitweisen Operation wird die Konstante 'TIMER_NONE' mit dem Wert 0 verwendet. sys_helper.c 179

#define HELPER(name) ....

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

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

Sie können leicht überprüfen, ob der Wert des Makros TIMER_NONE Null ist (Kommentar 1). Ferner wird dieses Makro in einer bitweisen Operation verwendet, deren Ergebnis immer 0 ist. Infolgedessen wird der Hauptteil der bedingten Anweisung if (env-> ttmr & TIMER_NONE) niemals ausgeführt.

Warnung N10

V629 Überprüfen Sie den Ausdruck 'n << 9'. Bitverschiebung des 32-Bit-Werts mit anschließender Erweiterung auf den 64-Bit-Typ. 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);
    ....
  }
  ....
}

In diesem Codefragment , ein Schaltvorgang durchgeführt wird , auf der Variable n , die einen 32-Bit - signierten Typen hat, dann ist dieses 32-bit mit Vorzeichen versehene Ergebnis wird auf eine 64-Bit - signierte Art erweitert, und dann, als unsigned - Typen, ist mit den unsigned 64-Bit - Variable addiert versetzt . Angenommen, zum Zeitpunkt der Ausführung des Ausdrucks hat die Variable n einige signifikante höchstwertige 9 Bits. Wir führen eine 9-Bit-Verschiebungsoperation durch ( BDRV_SECTOR_BITS)), und dies ist wiederum ein undefiniertes Verhalten, und als Ergebnis können wir das gesetzte Bit im höchstwertigen Bit erhalten. Denken Sie daran, dass dieses Bit im vorzeichenbehafteten Typ für das Vorzeichen verantwortlich ist, dh das Ergebnis kann negativ werden. Da n eine vorzeichenbehaftete Variable ist, wird das Vorzeichen beim Erweitern berücksichtigt. Das Ergebnis wird dann zur Versatzvariablen hinzugefügt . Aus diesen Überlegungen ist leicht ersichtlich, dass das Ergebnis der Ausführung des Ausdrucks vom beabsichtigten abweichen kann. Eine der möglichen Lösungen besteht darin, den Typ der Variablen n durch einen 64-Bit-Typ ohne Vorzeichen zu ersetzen , dh durch uint64_t .

Hier sind einige ähnliche Auslöser:

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

Warnung N11

V634 Die Priorität der Operation '*' ist höher als die der Operation '<<'. Es ist möglich, dass im Ausdruck Klammern verwendet werden. nand.c 310

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

Es ist nur ein verdächtiger Ort. Es ist nicht klar, was der Programmierer am Anfang tun wollte: Verschiebung oder Multiplikation. Auch wenn hier kein Fehler vorliegt, müssen Sie den Code erneut überprüfen und die Klammern richtig platzieren. Dies ist nur einer der Orte, an denen Entwickler nachsehen sollten, um sicherzustellen, dass ihr Algorithmus korrekt ist. Andere solche Orte:

V634 Die Priorität der Operation '*' ist höher als die der Operation '<<'. Es ist möglich, dass im Ausdruck Klammern verwendet werden. exynos4210_mct.c 449
V634 Die Priorität der Operation '*' ist höher als die der Operation '<<'. Es ist möglich, dass im Ausdruck Klammern verwendet werden. exynos4210_mct.c 1235
V634 Die Priorität der Operation '*' ist höher als die der Operation '<<'. Es ist möglich, dass im Ausdruck Klammern verwendet werden. exynos4210_mct.c 1264

Warnung N12

V646 Überprüfen Sie die Logik der Anwendung. Möglicherweise fehlt das Schlüsselwort "else". 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 {
      ....
    }
    ....
  }
  ....
}

In diesem Code bietet sich nach der Formatierung die Verwendung von else if anstelle von if direkt an . Vielleicht haben sie vergessen, hier noch etwas hinzuzufügen . Dann könnte die Korrekturoption folgendermaßen aussehen:

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

Es besteht jedoch die Möglichkeit, dass mit diesem Code alles in Ordnung ist und der Programmtext falsch formatiert ist, was verwirrend ist. Dann könnte der Code folgendermaßen aussehen:

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

Warnung N13

V773 Die Funktion wurde beendet, ohne den ' Regel' -Zeiger loszulassen . Ein Speicherverlust ist möglich. 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
  ....
}

In diesem Code der Regel Objekt (Kommentar 1) ausgewählt und für die spätere Verwendung (Kommentar 3) in die Liste aufgenommen, aber im Falle eines Fehlers die Funktion zurück , ohne das zuvor erstellte Löschen Regel Objekt (Kommentar 2). Hier müssen Sie nur den Fehler richtig behandeln: Löschen Sie das zuvor erstellte Objekt, da sonst ein Speicherverlust auftritt.

Warnung N14

V781 Der Wert des 'ix'-Index wird nach seiner Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. uri.c 2110

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

Hier hat der Analysator ein Potential außerhalb der Grenzen des Arrays erkannt. Zuerst wird das Element des ref-> path- Arrays am Index ix gelesen und dann wird ix auf Richtigkeit überprüft ( ix> 0 ). Die richtige Lösung besteht darin, diese Aktionen umzukehren:

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

Es gab mehrere solcher Orte:

V781 Der Wert des 'ix'-Index wird nach seiner Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. uri.c 2112
V781 Der Wert des 'Offset'-Index wird nach seiner Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. keymaps.c 125
V781 Der Wert der Variablen 'Qualität' wird nach ihrer Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. Überprüfen Sie die Zeilen: 326, 335.vnc-enc-tight.c 326
V781 Der Wert des 'i'-Index wird nach seiner Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. mem_helper.c 1929

Warnung N15

V784 Die Größe der Bitmaske ist kleiner als die Größe des ersten Operanden. Dies führt zum Verlust höherer Bits. 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]);
  ....
}

Dieser Code führt eine bitweise Operation für Objekte unterschiedlichen Typs aus. Der linke Operand ist das Argument val , ein 64-Bit-Typ ohne Vorzeichen. Der empfangene Wert des Array-Elements s-> regs_ro am Offset- Index , der einen vorzeichenlosen 32-Bit-Typ hat, wird als rechter Operand verwendet . Das Ergebnis der Operation auf der rechten Seite (~ (s-> regs_ro [offset])) ist ein vorzeichenloser 32-Bit-Typ, der vor der bitweisen Multiplikation auf den 64-Bit-Typ mit Nullen erweitert wird, dh nach Auswertung des gesamten Ausdrucks werden alle höherwertigen Bits der val- Variablen auf Null gesetzt . Solche Orte sehen immer verdächtig aus. Hier können wir den Entwicklern nur empfehlen, diesen Code erneut zu überarbeiten. Mehr ähnlich:

V784 Die Größe der Bitmaske ist kleiner als die Größe des ersten Operanden. Dies führt zum Verlust höherer Bits. xlnx-zynq-devcfg.c 199
V784 Die Größe der Bitmaske ist kleiner als die Größe des ersten Operanden. Dies führt zum Verlust höherer Bits. soc_dma.c 214
V784 Die Größe der Bitmaske ist kleiner als die Größe des ersten Operanden. Dies führt zum Verlust höherer Bits. fpu_helper.c 418

Warnung N16

V1046 Unsichere Verwendung der Typen 'bool' und 'unsigned int' zusammen in der Operation '& ='. 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);
}

In diesem Codefragment wird eine bitweise UND-Verknüpfung für die hpd- Variable vom Typ bool und das Ergebnis der Ausführung der Funktion extract32 mit dem Typ uint32_t ausgeführt . Da der Bitwert einer booleschen Variablen nur 0 oder 1 sein kann, ist das Ergebnis des Ausdrucks immer falsch, wenn das von der Funktion extract32 zurückgegebene niedrigstwertige Bit Null ist. Schauen wir uns das anhand eines Beispiels an. Angenommen, hpd ist wahr und die Funktion gibt 2 zurück, dh in der binären Darstellung sieht die Operation wie 01 & 10 = 0 aus und das Ergebnis des Ausdrucks ist falsch . Höchstwahrscheinlich wollte der Programmierer den Wert auf true setzenwenn die Funktion etwas ungleich Null zurückgibt. Anscheinend muss der Code korrigiert werden, damit das Ergebnis der Funktion in den Bool- Typ umgewandelt wird , zum Beispiel wie folgt :

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

Fazit

Wie Sie sehen können, hat der Analysator viele verdächtige Stellen gefunden. Vielleicht haben sich die potenziellen Probleme noch nicht in irgendeiner Weise manifestiert, aber ihre Anwesenheit kann nur alarmierend sein, da sie im unerwartetsten Moment schießen können. Es ist besser, alle verdächtigen Stellen im Voraus anzuzeigen und zu beheben, als einen endlosen Strom von Fehlern zu beheben. Bei komplexen Projekten wie diesem kann die statische Analyse natürlich greifbare Vorteile bringen, insbesondere wenn Sie eine regelmäßige Projektüberprüfung organisieren. Wenn Sie PVS-Studio für Ihr Projekt ausprobieren möchten, können Sie den Analysator herunterladen und auf dieser Seite einen kostenlosen Testschlüssel erhalten .

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Evgeniy Ovsannikov. Überprüfen der QEMU mit PVS-Studio .

QEMU mit PVS-Studio prüfen

Über die Analyse

Fazit

More articles: