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.
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:
int cBitStream::GetBit(void)
{
if (index >= length)
return 1;
int r = (data[index >> 3] >> (7 - (index & 7))) & 1;
++index;
return r;
}
uint32_t cBitStream::GetBits(int n)
{
uint32_t r = 0;
while (n--)
r |= GetBit() << n;
return r;
}
Display More
… 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:
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
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...
Woher soll denn ein noch so schlauer Compiler aus folgendem Code:
Soweit ich sehe geht es nicht um cBitStream, sondern um cH264Parser::GetBits(int Bits). Und das sieht deinem zweiten Code sehr ähnlich.
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:
uint32_t cH264Parser::GetBits(int Bits)
{
uint32_t b = 0;
while (Bits--)
b |= GetBit() << Bits;
return b;
}
Das würde dann zu:
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:
Aber das wäre schon wieder ein Stolperstein mehr für die Datenflussanalyse moderner Compiler… sofern sie es überhaupt hin bekommen.
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:
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:
Aber die einfachste Lösung wäre wohl noch immer:
#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.
Das gefällt mir ja noch weniger als der ursprüngliche Vorschlag von wirbel ;-).
Nein, denn ein solches Pragma ist wohl nicht portabel. Ich vermute, es gilt speziell für die GNU-Compilersuite.
Es ist ja auch gcc, der die Warnung bringt ;-).
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…
Plugins radio und tvguide funktionieren nicht, hat da jemand nen Patch?
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:
Index: vdr-plugin-tvguide-1.3.8/recmanager.c
===================================================================
--- vdr-plugin-tvguide-1.3.8.orig/recmanager.c 2023-11-21 18:44:59.000000000 +0100
+++ vdr-plugin-tvguide-1.3.8/recmanager.c 2024-09-14 18:03:54.338112708 +0200
@@ -470,7 +470,7 @@
if (channel) {
const cSchedule *Schedule = NULL;
Schedule = schedules->GetSchedule(channel);
- event = Schedule->GetEvent(eventID);
+ event = Schedule->GetEventById(eventID);
if (event) {
searchResults[index] = event;
} else
Display More
compiliert, danke.
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):
Index: vdr-plugin-radio-ng-10/radiotextosd.c
===================================================================
--- vdr-plugin-radio-ng-10.orig/radiotextosd.c 2024-09-14 18:32:18.644732674 +0200
+++ vdr-plugin-radio-ng-10/radiotextosd.c 2024-09-14 18:32:18.639732740 +0200
@@ -660,7 +660,14 @@
// check end @ replay
if (RT_Replay) {
int rplayCur, rplayTot;
+#if APIVERSNUM > 20402
+ {
+ cMutexLock ControlMutexLock;
+ cControl::Control(ControlMutexLock)->GetIndex(rplayCur, rplayTot, false);
+ }
+#else
cControl::Control()->GetIndex(rplayCur, rplayTot, false);
+#endif
if (rplayCur >= rplayTot - 1) {
Hide();
return osEnd;
Display More
auch das passt, danke
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:
# 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:
# remux.c:1746: int sps_max_sub_layers_minus1 = min(GetBits(3), 7U); // sps_max_sub_layers_minus1
movl $7, -228(%rbp) #, D.45483
movq -280(%rbp), %rax # this, _2
movl $3, %esi #,
movq %rax, %rdi # _2,
call _ZN11cH264Parser7GetBitsEi #
movl %eax, -224(%rbp) # _3, D.45482
leaq -228(%rbp), %rdx #, tmp307
leaq -224(%rbp), %rax #, tmp308
movq %rdx, %rsi # tmp307,
movq %rax, %rdi # tmp308,
call _ZSt3minIjERKT_S2_S2_ #
movl (%rax), %eax # *_4, _5
movl %eax, -120(%rbp) # _5, sps_max_sub_layers_minus1
Display More
Einiges an Aufwand, nur um den Compiler zufriedenzustellen.
Vorschlag zur Güte:
# 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:
--- remux.c 2024/01/23 19:33:45 5.7
+++ remux.c 2024/09/14 21:24:49
@@ -1743,7 +1743,7 @@
uint8_t sub_layer_profile_present_flag[8];
uint8_t sub_layer_level_present_flag[8];
GetBits(4); // sps_video_parameter_set_id
- int sps_max_sub_layers_minus1 = GetBits(3); // sps_max_sub_layers_minus1
+ int sps_max_sub_layers_minus1 = GetBits(3) & 7; // sps_max_sub_layers_minus1 - "& 7" to silence compiler warning in gcc 14.1.0
GetBit(); // sps_temporal_id_nesting_flag
// begin profile_tier_level(1, sps_max_sub_layers_minus1)
GetByte();
Display More
Don’t have an account yet? Register yourself now and be a part of our community!