[ANNOUNCE] VDR Version 2.7.1 freigegeben

  • Der compiler checkt nur mehr als früher.

    Ein bug in gcc ist das kaum, es macht ja Sinn arrays auf Überlauf zu prüfen.

  • GetBits(3) kann nicht größer als 7 sein.

    Wenn der Compiler schon meint, so schlau zu sein, dann sollte er das auch erkennen ;-).

    Soll natürlich keine Kritik an wirbel sein. Es widerstrebt mir nur, eine unnötige Operation zu machen, nur um den Compiler zufriedenzustellen, wenn vollkommen klar ist, dass hier kein Fehler vorliegt. Die Aussage "Schreiben...an Offset 8 des Zielobjekts..." ist schlichtweg falsch.

  • Sag einfach nein und gut ist.

  • Woher soll denn ein noch so schlauer Compiler aus folgendem Code:

    … ableiten können, dass hier drei Bits gelesen und in die Bitpositionen [2:0] eingetragen werden. Uns ist die Semantik aufgrund des Namens der Funktion klar, aber wir nutzen dabei ja auch linguistische Kontextinformation.


    Damit der Compiler überhaupt eine Chance hat, müsste man ihm mindestens mitgeben, dass nur die untersten drei Bits signifikant sind:

    Code
    uint32_t cBitStream::GetBits(int n)
    {
      uint32_t r = 0;
      for (int i = n - 1; i >= 0; i--)
            r |= GetBit() << i;
      return r & ((1U << n) - 1);
    }

    Ein Compiler mit wirklich guter Datenflussanalyse könnte damit möglicherweise etwas anfangen.


    PS: Mist, n wird ja heruntergezählt, deshalb eine kleine Codeanpassung nachgetragen.


    Da mir unter Ubuntu 22.04.1 leider nur ein gcc v11.4 zur Verfügung steht, kann ich das nicht selbst ausprobieren. Aber vielleicht mag wirbel einmal checken, ob das bei seinem Compiler einen Unterschied macht?


    Viele Grüße

    Stefan

    Hardware: Antec NSK2480, Asus P8B75-M LX, Intel Core i5-3570T, 4 GB RAM, NVIDIA GT610, TT-Premium S2-6400, 128 GB SSD, 14 TB HDD, Pioneer BDR-207EBK
    Software: Ubuntu 22.04 LTS mit Kernel 6.8 und VDR 2.7.3 (mit offiziellen und eigenen Patches)
    Plugins: devstatus, dvbhddevice, dvd, dvdswitch, epgsearch, extrecmenu, recsearch, femon, live, markad, mlist, osdteletext, remote, screenshot, skinenigmang, skinnopacity, streamdev, systeminfo, undelete, xineliboutput
    Addons: VDR Convert 0.1.0 (angepasst)

    Edited 3 times, last by shofmann ().

  • Sag einfach nein und gut ist.

    Ich bin ja noch bei gcc 7.5.0, da kommt diese Warnung nicht.

    Gibt es vielleicht eine Möglichkeit, dem Compiler zu sagen "An dieser Stelle brauchst du nicht zu prüfen, ich *weiß*, dass das OK ist"?

    Die Warnung generell abzuschalten ist sicher nicht ratsam, denn es kann durchaus Fälle geben, wo sie Sinn macht...

  • Danke, ich habe nur mal schnell mit grep versucht, die Stelle zu finden. Die Funktion in remux.c hatte ich dabei ganz übersehen… Aber bei mir sieht sie wie die andere aus:

    Code
    uint32_t cH264Parser::GetBits(int Bits)
    {
      uint32_t b = 0;
      while (Bits--)
            b |= GetBit() << Bits;
      return b;
    }

    Das würde dann zu:

    Code
    uint32_t cH264Parser::GetBits(int Bits)
    {
      uint32_t b = 0;
      for (int i = Bits - 1; i >= 0; i--)
            b |= GetBit() << i;
      return b & ((1u << Bits) - 1);
    }

    Implizit unterstellen wir ja, dass n bzw. Bits nie negativ wird. Wenn man das absichern möchte, sollte man diesen Parameter entweder als unsigned übergeben (ich weiß, das übliche Drama mit den C-Datentypen) oder die letzte Zeile wie folgt ändern:

    Code
    return Bits >= 0 ? b & ((1u << Bits) - 1) : 0;

    Aber das wäre schon wieder ein Stolperstein mehr für die Datenflussanalyse moderner Compiler… sofern sie es überhaupt hin bekommen.

    Hardware: Antec NSK2480, Asus P8B75-M LX, Intel Core i5-3570T, 4 GB RAM, NVIDIA GT610, TT-Premium S2-6400, 128 GB SSD, 14 TB HDD, Pioneer BDR-207EBK
    Software: Ubuntu 22.04 LTS mit Kernel 6.8 und VDR 2.7.3 (mit offiziellen und eigenen Patches)
    Plugins: devstatus, dvbhddevice, dvd, dvdswitch, epgsearch, extrecmenu, recsearch, femon, live, markad, mlist, osdteletext, remote, screenshot, skinenigmang, skinnopacity, streamdev, systeminfo, undelete, xineliboutput
    Addons: VDR Convert 0.1.0 (angepasst)

    Edited once, last by shofmann ().

  • Das würde dann zu:

    Was aber auch eine völlig unnötige Operation wäre ;-).

    Könnte das hier vielleicht funktionieren: https://stackoverflow.com/ques…s-for-a-few-lines-of-code


    Also etwa:

    Code
      for (int i = 0; i < sps_max_sub_layers_minus1; i++ ) {
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wstringop-overflow="
         sub_layer_profile_present_flag[i] = GetBit(); // sub_layer_profile_present_flag[i]
         sub_layer_level_present_flag[i] = GetBit();   // sub_layer_level_present_flag[i]
    #pragma GCC diagnostic pop
         }
  • Ja und nein:

    • Ja, denn es schaltet die Prüfung an dieser Stelle ab. Kuriert also das Symptom und nicht die Ursache, dass nämlich die Variable einen deutlich größeren Wertebereich zulässt als der Wertebereich von drei Bits und der Code dem Compiler die Schlussfolgerung nicht auf hinreichend einfache Weise erlaubt.
    • Nein, denn ein solches Pragma ist wohl nicht portabel. Ich vermute, es gilt speziell für die GNU-Compilersuite.

    Aber die einfachste Lösung wäre wohl noch immer:

    Code
    #define SIZEOF(a) (sizeof(a) / sizeof(*a))
    for (int i = 0; i < sps_max_sub_layers_minus1 && i < SIZEOF(sub_layer_level_present_flag) ; i++ ) {

    Die zweite Bedingung muss aber halt an jeder betroffenen Stelle ergänzt werden.

    Hardware: Antec NSK2480, Asus P8B75-M LX, Intel Core i5-3570T, 4 GB RAM, NVIDIA GT610, TT-Premium S2-6400, 128 GB SSD, 14 TB HDD, Pioneer BDR-207EBK
    Software: Ubuntu 22.04 LTS mit Kernel 6.8 und VDR 2.7.3 (mit offiziellen und eigenen Patches)
    Plugins: devstatus, dvbhddevice, dvd, dvdswitch, epgsearch, extrecmenu, recsearch, femon, live, markad, mlist, osdteletext, remote, screenshot, skinenigmang, skinnopacity, streamdev, systeminfo, undelete, xineliboutput
    Addons: VDR Convert 0.1.0 (angepasst)

  • Mal sehen, ob es der Compiler ohne Warnungen verarbeitet. Aber das hier ist ja noch harmlos: in der Arbeit haben wir mit Coverity einen richtig kritischen Aufpasser…

    Hardware: Antec NSK2480, Asus P8B75-M LX, Intel Core i5-3570T, 4 GB RAM, NVIDIA GT610, TT-Premium S2-6400, 128 GB SSD, 14 TB HDD, Pioneer BDR-207EBK
    Software: Ubuntu 22.04 LTS mit Kernel 6.8 und VDR 2.7.3 (mit offiziellen und eigenen Patches)
    Plugins: devstatus, dvbhddevice, dvd, dvdswitch, epgsearch, extrecmenu, recsearch, femon, live, markad, mlist, osdteletext, remote, screenshot, skinenigmang, skinnopacity, streamdev, systeminfo, undelete, xineliboutput
    Addons: VDR Convert 0.1.0 (angepasst)

  • Plugins radio und tvguide funktionieren nicht, hat da jemand nen Patch?


    vdr-User-# 755 to_h264 chk_r vdr-transcode github

  • Ich bin ja noch bei gcc 7.5.0, da kommt diese Warnung nicht.

    Gibt es vielleicht eine Möglichkeit, dem Compiler zu sagen "An dieser Stelle brauchst du nicht zu prüfen, ich *weiß*, dass das OK ist"?

    Die Warnung generell abzuschalten ist sicher nicht ratsam, denn es kann durchaus Fälle geben, wo sie Sinn macht...

    Bestimmt.

    Mein Vorschlag bestand aus 9 Ascii Zeichen, ein '#pragma ' sind bereits sieben. [Edit: 8 Zeichen.]

  • Plugins radio und tvguide funktionieren nicht, hat da jemand nen Patch?

    Für tvguide sollte das genügen:

    yaVDR-Dokumentation (Ceterum censeo enchiridia esse lectitanda.)

  • compiliert, danke.


    vdr-User-# 755 to_h264 chk_r vdr-transcode github

  • Für das radio-Plugin ist der Fix auch recht übersichtlich (je nachdem ob man das Plugin von https://github.com/siricco/vdr-plugin-radio oder https://github.com/vdr-projects/vdr-plugin-radio hat, ändern sich die Zeilennummer ein bisschen):

    yaVDR-Dokumentation (Ceterum censeo enchiridia esse lectitanda.)

  • auch das passt, danke


    vdr-User-# 755 to_h264 chk_r vdr-transcode github

  • Mein Vorschlag bestand aus 9 Ascii Zeichen, ein '#pragma ' sind bereits sieben. [Edit: 8 Zeichen.]

    Darum geht's doch gar nicht. Die Frage ist: soll man funktionierenden Code ändern, wegen eines vermeintlichen Problems, das garantiert nie eintreten kann (sps_max_sub_layers_minus1kann nicht größer als 7 sein), oder soll man dem Compiler an der Stelle einfach sagen "es ist OK, du brauchst nichts zu melden". Sicher würde min(GetBits(3), 7U) den Compiler zufriedenstellen. Aber der Fall GetBits(3)>7 wird nie eintreten.

  • Einverstanden, sammeln wir compiler Warnungen.

  • wirbel Deinem Kommentar entnehme ich, dass wir an dieser Stelle wohl eher nicht zusammenkommen ;-). Ob das mit dem #pragma tatsächlich funktioniert kannst nur du testen, denn ich habe gcc 14.1.0 nicht im Einsatz.


    Aber schauen wir uns doch mal den Assembler-Code an (ohne -O3, denn damit ist die Struktur nur schwer wiederzuerkennen).

    Hier der ursprüngliche Aufruf:

    Code
    # remux.c:1746:   int sps_max_sub_layers_minus1 = GetBits(3); // sps_max_sub_layers_minus1
            movq    -264(%rbp), %rax        # this, _2
            movl    $3, %esi        #,
            movq    %rax, %rdi      # _2,
            call    _ZN11cH264Parser7GetBitsEi      #
            movl    %eax, -120(%rbp)        # _3, sps_max_sub_layers_minus1

    Hier dein Vorschlag:

    Einiges an Aufwand, nur um den Compiler zufriedenzustellen.


    Vorschlag zur Güte:

    Code
    # remux.c:1746:   int sps_max_sub_layers_minus1 = GetBits(3) & 7; // sps_max_sub_layers_minus1 - "& 7" to silence compiler warning in gcc 14.1.0
            movq    -264(%rbp), %rax        # this, _2
            movl    $3, %esi        #,
            movq    %rax, %rdi      # _2,
            call    _ZN11cH264Parser7GetBitsEi      #
            andl    $7, %eax        #, tmp306
            movl    %eax, -120(%rbp)        # tmp306, sps_max_sub_layers_minus1

    Ist minimalinvasiv und sollte den Compiler auch zufriedenstellen.

    Magst du das vielleicht testen?

    Hier als Diff:

Participate now!

Don’t have an account yet? Register yourself now and be a part of our community!