Compile Warnings in vnsiserver

So wie es aussieht scheint der Move-Operator/Constructor nicht die Ursache zu sein.

Dazu wird er IMHO einfach zu oft aufgerufen, da müsste es auch anderswo krachen.

Uns auch die Selbstzuweisung ist inzwischen ausgeschlossen.

Auffällig ist, dass es vor dem Crash eine Änderung an der Kanalliste gab, auf die das Plugin gerade zugreift.

Ist das Immer der Fall?

Eigentlich sollte doch allein das korrekte setzen der Locks auf die Liste, die Zugriffe darauf threadsave machen?

Und damit da eigentlich auch jegliche Timing-Effekte ausschließen.

Auffällig ist aber halt, dass die Änderung der PIDs ziemlich direkt vor dem Crash kam.

Fehler sehe ich da auch weder im Plugin, noch im VDR.

Allenfalls diese Zeile in pat.c:

847 StateKey.Remove(ChannelsModified);

Was würde denn passieren, wenn ChannelsModified versehentlich false wäre, obwohl was geändert wurde?

Das müsste man doch eigentlich testweise einfach mal auf true setzen können, ohne dass es eine Katastrophe gibt?

Könnte es in diesem Fall was bringen die debug_locking zu aktivieren?

(Zeile 29 thread.c)

29 //#define DEBUG_LOCKING // uncomment this line to activate debug output for locking

Es wäre zumindest einen Versuch wert mal zu schauen ob etwas mit dem Locking falsch läuft. Wobei das schon komisch wäre wenn Plugins, die da bisher keine Probleme hatten, jetzt Locking Fehler werfen.

Und wenn das "writing 1 byte into a region of size 0 overflows the destination" ein Compilerfehler ist?

Mit g++ 7.5.0 gibts lt. Post #8 den Fehler ja nicht.

Aus Post #2:

In file included from /usr/include/c++/13.2.1/bits/shared_ptr_atomic.h:33,
                 from /usr/include/c++/13.2.1/memory:81,
                 from videoinput.h:27,
                 from videoinput.c:27:
In member function ‘std::__atomic_base<_IntTp>::__int_type std::__atomic_base<_IntTp>::load(std::memory_order) const [with _ITp = bool]’,

Er macht dort ja noch nichts außer #include <memory> (OK, config.h hat er schon eingelesen)

Kannst Du den kompletten VDR und Plugins mal mit g++ 7.5.0 und dessen Headerfiles compilieren und laufen lassen?

RHS kannst du den Vorschlag von SHF mal ausprobieren?

Post

RE: Compile Warnings in vnsiserver

So wie es aussieht scheint der Move-Operator/Constructor nicht die Ursache zu sein.
Dazu wird er IMHO einfach zu oft aufgerufen, da müsste es auch anderswo krachen.
Uns auch die Selbstzuweisung ist inzwischen ausgeschlossen.

Auffällig ist, dass es vor dem Crash eine Änderung an der Kanalliste gab, auf die das Plugin gerade zugreift.
Ist das Immer der Fall?

Eigentlich sollte doch allein das korrekte setzen der Locks auf die Liste, die Zugriffe darauf threadsave machen?
Und damit da eigentlich auch…

SHF

Feb 15th 2024

Wäre schon spannend zu wissen ob vor dem Crash auch Locking-Fehler geworfen werden.

Quote from FireFly

Und wenn das "writing 1 byte into a region of size 0 overflows the destination" ein Compilerfehler ist?

Da ist wohl wirklich ein Bug im GCC, aber im Sinne eines false positives:

108374 – [12/13/14 Regression] unexpected -Wstringop-overflow when using std::atomic and std::shared_ptr

In dem Fall geht es auch um weakPointer, sieht nahezu identisch aus.

Ich hoffe die haben mit ihrer Einschätzung recht .

Quote from M-Reimer

Wobei das schon komisch wäre wenn Plugins, die da bisher keine Probleme hatten, jetzt Locking Fehler werfen.

Wirkliche Fehler müssten jetzt schon im Log landen. Entsprechende Ausgaben sind jetzt schon aktiv und davon gibt es auch reichlich.

Wenn muss es da was subtileres sein. Deshalb auch die Frage mit dem StateKey.Remove.

Könnte es Probleme verursachen, wenn man was ändert und den state nicht inkrementiert?

Ich bin da momentan nicht sicher, ob das was macht, oder nicht. So weit habe ich mich noch nicht "durch gebuddelt".

Durch das DEBUG_LOCKING sollte man zumindest sehen, wann auf was Locks gesetzt werden, vielleicht sieht man dadurch irgendwelche Zusammenhänge.

Momentan sieht man aus dem Log eigentlich nur, dass sich zwei VNSI-Clinets verbinden und dann kommt schon die Crash-Sequenz.

Durch die Locks müsste man besser nachvollziehen können, worauf das Plugin zugreift.

Man könnte auch mal probieren das Aktualisieren der Kanäle in VDR ganz ab zu stellen (setup.conf UpdateChannels = 0).

Das wäre durchaus interessant, ob und wann es dann zum Absturz kommt.

Wenn das Aktualisieren der PIDs die Ursache für den Absturz ist, sollte der dann nicht mehr auftreten.

Wenn es an was anderem liegt, dann wo anders.

Was passiert eigentlich mit den compiler warnings, wenn man einen check einfügt, ob der std::weak_ptr noch gültig ist?

Ansonsten könnte in videoinput.c Zeilen 467 und 468 recv auf ein ungültiges object zeigen.

videoinput.c:464 anstelle von

  // find an active receiver for the device
  for (auto p : s_Pool)
  {
    auto recv = p.lock();
    if (!recv->BeenDetached() && recv->Device() == device)
      return recv;
  }

eher so, und im loop mit Referenz arbeiten um Kopien zu vermeiden.

  // find an active receiver for the device
  for (auto& p : s_Pool)
  {
    if (not p.expired()) {
       auto recv = p.lock();
       if (!recv->BeenDetached() && recv->Device() == device)
          return recv;
       }
  }

RHS DEBUG_LOCKING schreibt nach stderr. Wenn du VDR normal von der Kommandozeile startest, kannst du das in eine Datei umleiten.

Quote from RHS

Könnte mir jemand bei der Menge der Testvorschläge die Varianten etwas strukturieren, ich blicke derzeit nicht genau wie was mit einander wechselwirkt.

So wirklich blickt hier wohl keiner durch was wie wechselwirkt, daher auch die vielen unterschiedlichen Testvorschläge um das Problem einzugrenzen.

Ich werde es aber mal versuchen mit dem Stukturieren.

Ausgangslage ist:

VDR 2.6.6, GCC 7.5.0: keine Fehlermeldungen und keine Abstürze.

VDR 2.6.6, GCC 13.2.1: Fehlermeldung und praktisch sofortiger Absturz.

VDR 2.6.6, GCC 13.2.1 ohne Move-Operator/Constructor: Fehlermeldung und Abstürze, aber seltener.

Richtig?

Der Move-Operator/Constructor hat großen Einfluss auf die Speicherbelegung.

Ohne entstehen durch die temporären Objekte, die gleich wieder gelöscht werden, viel mehr "Lücken".

Das dürfte erklären, warum es ohne besser wird. Wenn man so eine "Lücke" nach dem problematischen Bereich liegt, fällt der Fehler nicht unbedingt auf.

Zum weiteren Testen macht eigentlich nur die Version VDR 2.6.6, GCC 13.2.1 (mit Move-Operator/Constructor) Sinn, man will ja was sehen .

Es macht auch erstmal keinen Sinn die Tests zu kombinieren, das verwirrt einen nur, daher immer schön einzeln.

Quote from SHF

Man könnte auch mal probieren das Aktualisieren der Kanäle in VDR ganz ab zu stellen (setup.conf UpdateChannels = 0).

Der Test ist einfach, Änderungen am Code sind nicht nötig:

Sicherstellen, dass ein Crash eintritt.

VDR stoppen (wichtig!).

setup.conf ändern.

VDR starten und schauen, ob es erneut zum Crash kommt.

Quote from RHS

Habe deinen Vorschlag #70 mal eingebaut in videoinput.c insgesamt erhalte ich folgende Warnings in vnsiserver:

[...]

diese sehen, glaub ich, leicht anders aus als vorher

Der Test ist also erstmal schon erledigt.

Die ersten zwei Warnungen sind neu, ich verstehe die aber noch nicht so ganz.

Besonders die Zweite. Warum ist das erst jetzt ein Problem???

Die Dritte sieht leicht anders aus, scheint aber der Ursprünglichen weitgehend zu entsprechen.

Die Unterscheide können in den Compilereinstellungen liegen, die sind leicht unterschiedlich.

Apropos Compilereinstellungen, da könnte man mal -O1 und -march=x86-64 (ist doch eine x86 64Bit CPU?) anstelle von -O3[tt] und -march=native[/tt] versuchen.

Es könnte sein, dass der neue GCC es bei der Optimierung generell (-O_) etwas "übertreibt".

-march=native erlaubt spezifischere Optimierungen für die verwendete CPU, da besteht die Chance, dass da Bugs drin stecken. Distributionen verwenden "x86-64" oder was vergleichbares, damit die Pakete allgemein kompatibel sind, native für exakt Deine CPU ist also wenig getestet.

Auch wieder als frischer Test, ohne weitere Änderungen. Einfach schauen, ob es auch so auch crasht.

Quote from RHS

Ich habe das DEBUG_LOCKING versuchsweise aktiviert, leider schreibt vdr mir dann den syslog mit lock/unlock Meldungen voll. Der crash ist so nicht weiter einzugrenzen.

Dass da etwas mehr zusammen kommt, hatte ich schon erwartet. Das sind die Meldungen von allen Locks, Channels, Timers, EPG...

Ich hatte nur die Hoffnung, dass es noch im Rahmen bleibt und der Syslog das bewältigt, das wäre übersichtlicher gewesen.

Eine getrennte Logfile geht aber auch, wie Du das bei SUSE umleitest, musst Du aber selber mal schauen.

Die (un)lock Meldungen sollten die pid/tid des verursachendem Prozesses enthalten.

Diese sind auch in den Syslog-Meldungen enthalten:

[...] vdr[1987]: [11772] VNSI Client[...] thread started (pid=1987, tid=11772, prio=high)

[...] vdr[1987]: [1995] changing pids of channel 4[...]

(Die 1987 ist die pid des VDR Hauptprozess.)

So sollte man die Meldungen den Prozessen zuordnen können.

(Die pids der channels haben damit übrigens gar nichts zu tun, heissen nur zufällig gleich.)

Relevant sind wahrscheinlich auch nur die letzten 10-20 (un)locks vor dem Crash.

Also etwa ab dem changing pids evtl noch ein paar davor.

VDR 2.6.6, GCC 11.1.0: keine Warnungen beim compilieren

VDR 2.6.6, GCC 13.2.0: Warnungen beim compilieren

Ich hab gestern mal gcc neu compiliert, spätestens ab 13.2.0 tauchen die Warnungen auf.

Damit kann das Problem weiter eingeengt werden

gcc > 11.1.0 && gcc <= 12.3.0

Quote from wirbel

Damit kann das Problem weiter eingeengt werden

gcc > 11.1.0 && gcc <= 12.3.0

Wenn man nach diesen Warnungen sucht, findet man auch immer, dass die mit dem 12er Zweig, also 12.0.0 anfingen. Von daher deckt sich das.

Im Changelog steht auch zu einigen der Warnungen, dass die "optimiert" wurden. Aber das sind ja letztendlich lediglich Hinweise, dass da was seien könnte und bislang hat es ja so funktioniert.

Einen echten Grund, warum es aber diese Probleme gibt, habe ich beim überfliegen nicht gesehen.

Vielleicht sieht ja jemand anderes mehr.

Quote from RHS

responsepacket.c, status.c,vnsisocket.c

und dann;

hash.c vnsitimer.c, vnsisocket.c

Letztendlich sind es aber immer die 3 gleichen Stellen, die die Warnung auslösen.

Das deutet stark darauf hin, dass die dem GCC nicht gefallen.

Warum das von unterschiedlichen Dateien getriggert wird, weiß der Geier.

Eventuell liegt es einfach an der Reihenfolge der Bearbeitung?

Interessant ist auch, dass die Änderung aus #70 anscheinend keinen Einfluss hatte.

streamer.c: In constructor ‘cLiveStreamer::cLiveStreamer(int, bool, int, uint8_t, uint32_t)’:
streamer.c:57:17: warning: member ‘cLiveStreamer::m_Event’ is used uninitialized [-Wuninitialized]
   57 |  , m_VideoInput(m_Event)
      |                 ^~~~~~~

Zu -Wuninitialized steht was im Changelog (s.o.).

An der Warnung könnte prinzipiell was dran sein, uninitialisierte Variablen/Objekte können derartige Abstürze auslösen.

Die Warnung warnt auch vor allen möglichen potenziellen Problemen, die aber nicht auftreten müssen, wenn man es richtig macht.

Und da es bislang ja funktioniert hat ...

Neu ist in GCC 12 übrigens dieses Stück in der Dokumentation dazu:

Quote

In C++, this warning also warns about using uninitialized objects in member-initializer-lists. For example, GCC warns about b being uninitialized in the following snippet:

Code

struct A {
  int a;
  int b;
  A() : a(b) { }
};

channelfilter.c: In member function ‘void cVNSIChannelFilter::StoreWhitelist(bool)’:
channelfilter.c:188:23: warning: loop variable ‘i’ creates a copy from type ‘const cVNSIProvider’ [-Wrange-loop-construct]
  188 |       for (const auto i : *whitelist)
      |                       ^
channelfilter.c:188:23: note: use reference type to prevent copying
  188 |       for (const auto i : *whitelist)
      |                       ^
      |                       &

Irgendwie sieht es mir so aus, als ob das ein beliebter Anfänger/Flüchtigkeitsfehler ist.

Die neue (ich habe noch C++2003 gelernt) Syntax mit dem "auto" ist zwar schön kompakt, was IMHO aber zu so Fehlern einlädt. Das fehlende "&" übersieht man einfach gerne mal.

Daher wohl die Warnung.

Die Kopie ist, von der Performance, Spericherverbrauch, ... zwar nicht ideal, ein wirkliches Problem dürfte das in den meisten Fällen aber wohl nicht auslösen, denke ich.

Und in unserem Fall bin ich momentan nicht mal sicher, ob das nicht Absicht so war.

Den letzten von den Fehlern hatten wir ja schon und da wird da wird behauptet, dass es ein "false positive" ist.

Probiert mal damit.