Dies ist ein klassischer Artikel darüber, wie unser Team ein offenes LibrePCB-Projekt mit dem statischen Code-Analysator PVS-Studio getestet hat. Der Artikel ist jedoch insofern interessant, als die Überprüfung im Docker-Container durchgeführt wurde. Wenn Sie Container verwenden, hoffen wir, dass der Artikel eine weitere einfache Möglichkeit zur Integration des Analysators in den Entwicklungsprozess zeigt.
LibrePCB
LibrePCB ist eine kostenlose Software zum Entwerfen elektronischer Schaltungen und Leiterplatten. Der Programmcode ist in C ++ geschrieben und Qt5 wird zum Erstellen der grafischen Oberfläche verwendet. Vor kurzem fand die erste offizielle Veröffentlichung dieser Anwendung statt, die die Stabilisierung des eigenen Dateiformats (* .lp, * .lplib) markierte. Binärpakete für Linux, MacOS und Windows.
LibrePCB ist ein kleines Projekt, das nur etwa 300.000 nicht leere Codezeilen in C und C ++ enthält. Gleichzeitig sind 25% der nicht leeren Zeilen Kommentare. Dies ist übrigens ein großer Prozentsatz für Kommentare. Dies ist höchstwahrscheinlich auf die Tatsache zurückzuführen, dass das Projekt viele kleine Dateien enthält, von denen ein erheblicher Teil mit Überschriftenkommentaren aus den Projektinformationen und der Lizenz belegt ist. GitHub-Site-Code:
LibrePCB .
Das Projekt schien uns interessant zu sein, und wir beschlossen, es auszuprobieren. Aber die Testergebnisse waren nicht mehr so interessant. Ja, es gab echte Fehler. Aber es gab nichts Besonderes, worüber wir den Lesern unserer Artikel sicherlich erzählen müssen. Vielleicht würden wir keinen Artikel schreiben und uns darauf beschränken, die Fehler an die Entwickler des Projekts zu senden. Der interessante Punkt war jedoch, dass das Projekt im Docker-Image getestet wurde und wir daher entschieden haben, dass es sich immer noch lohnt, einen Artikel zu schreiben.
Docker
Docker - Software zur Automatisierung der Bereitstellung und Verwaltung von Anwendungen in einer Virtualisierungsumgebung auf Betriebssystemebene. Sie können die Anwendung mit all ihren Umgebungen und Abhängigkeiten in einen Container "packen". Obwohl diese Technologie ungefähr fünf Jahre alt ist und viele Unternehmen Docker schon lange in die Infrastruktur ihrer Projekte implementiert haben, war dies in der Open-Source-Welt bis vor kurzem nicht sehr auffällig.
Unser Unternehmen arbeitet sehr eng mit Open Source-Projekten zusammen und überprüft den Quellcode mit unserem eigenen statischen PVS-Studio-Analysegerät. Derzeit wurden
mehr als 300 Projekte verifiziert. Das Schwierigste in dieser Aktivität war immer die Zusammenstellung von Projekten anderer Leute, aber die Einführung von Docker-Containern hat diesen Prozess erheblich vereinfacht.
Die erste Erfahrung beim Testen eines Open Source-Projekts in Docker machte
Azure Service Fabric . Dort haben die Entwickler die Installation des Quelldateiverzeichnisses im Container vorgenommen, und die Integration des Analysators beschränkte sich auf die Bearbeitung eines der im Container ausgeführten Skripte:
diff --git a/src/build.sh b/src/build.sh index 290c57d..2a286dc 100755 --- a/src/build.sh +++ b/src/build.sh @@ -193,6 +193,9 @@ BuildDir() cd ${ProjBinRoot}/build.${DirName} + pvs-studio-analyzer analyze --cfg /src/PVS-Studio.cfg \ + -o ./service-fabric-pvs.log -j4 + if [ "false" = ${SkipBuild} ]; then if (( $NumProc <= 0 )); then NumProc=$(($(getconf _NPROCESSORS_ONLN)+0))
Der Unterschied zwischen dem LibrePCB-Projekt besteht darin, dass sofort die
Docker-Datei zum
Erstellen des Images und des darin enthaltenen Projekts bereitgestellt wurde. Es stellte sich für uns als noch bequemer heraus. Hier ist der Teil der Docker-Datei, an dem wir interessiert sind:
FROM ubuntu:14.04 # install packages RUN DEBIAN_FRONTEND=noninteractive \ apt-get -q update \ && apt-get -qy upgrade \ && apt-get -qy install git g++ qt5-default qttools5-dev-tools qt5-doc \ qtcreator libglu1-mesa-dev dia \ && apt-get clean # checkout librepcb RUN git clone --recursive https:
Wir werden das Projekt beim Zusammenstellen des Bildes nicht kompilieren und installieren. So haben wir ein Bild gesammelt, in dem der Autor des Projekts die erfolgreiche Montage des Projekts garantiert.
Nach dem Starten des Containers wurde der Analysator installiert und die folgenden Befehle wurden ausgeführt, um das Projekt zu erstellen und zu analysieren:
cd /opt/LibrePCB mkdir build && cd build qmake -r ../librepcb.pro pvs-studio-analyzer trace -- make -j2 pvs-studio-analyzer analyze -l /mnt/Share/PVS-Studio.lic -r /opt/LibrePCB \ -o /opt/LibrePCB/LibrePCB.log -v -j4 cp -R -L -a /opt/LibrePCB /mnt/Share
Übrigens wurden alle Aktionen in Windows 10 ausgeführt. Es ist sehr erfreulich, dass sich auch die Entwickler aller gängigen Betriebssysteme in diese Richtung entwickeln. Leider sind Container mit Windows nicht so bequem. Vor allem wegen der Unmöglichkeit, auch Software einfach zu installieren.
Bugs gefunden
Nun ein klassischer Abschnitt mit einer Beschreibung der Fehler, die wir mit dem statischen Code-Analysator PVS-Studio gefunden haben. Bei dieser Gelegenheit möchte ich Sie übrigens daran erinnern, dass wir kürzlich einen Analysator entwickeln, der die Codeanalyse für eingebettete Systeme unterstützt. Hier sind einige Artikel, die einige unserer Leser möglicherweise übersprungen haben:
- PVS-Studio bietet Unterstützung für die GNU Arm Embedded Toolchain .
- PVS-Studio: Unterstützung für Codierungsstandards MISRA C und MISRA C ++ .
Tippfehler
SymbolPreviewGraphicsItem::SymbolPreviewGraphicsItem( const IF_GraphicsLayerProvider& layerProvider, const QStringList& localeOrder, const Symbol& symbol, const Component* cmp, const tl::optional<Uuid>& symbVarUuid, const tl::optional<Uuid>& symbVarItemUuid) noexcept { if (mComponent && symbVarUuid && symbVarItemUuid) .... if (mComponent && symbVarItemUuid && symbVarItemUuid)
PVS-Studio Warnung:
V501 CWE-571 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'symbVarItemUuid'. symbolpreviewgraphicsitem.cpp 74
Klassischer Tippfehler: Die Variable
symbVarItemUuid wird zweimal hintereinander überprüft. Es gibt oben eine ähnliche Prüfung, und wenn man sie betrachtet, wird klar, dass die zweite zu
prüfende Variable
symbVarUuid sein
sollte .
Das folgende Tippfehler-Code-Snippet:
void Clipper::DoMaxima(TEdge *e) { .... if (e->OutIdx >= 0) { AddOutPt(e, e->Top); e->OutIdx = Unassigned; } DeleteFromAEL(e); if (eMaxPair->OutIdx >= 0) { AddOutPt(eMaxPair, e->Top);
PVS-Studio Warnung:
V778 CWE-682 Es wurden zwei ähnliche Codefragmente gefunden. Möglicherweise ist dies ein Tippfehler und die Variable 'eMaxPair' sollte anstelle von 'e' verwendet werden. clipper.cpp 2999
Höchstwahrscheinlich wurde der Code mit Copy-Paste geschrieben. Aufgrund des Versehens im zweiten Textblock haben sie vergessen,
e-> Top durch
eMaxPair-> Top zu ersetzen.
Zusätzliche Schecks
static int rndr_emphasis(hoedown_buffer *ob, const hoedown_buffer *content, const hoedown_renderer_data *data) { if (!content || !content->size) return 0; HOEDOWN_BUFPUTSL(ob, "<em>"); if (content) hoedown_buffer_put(ob, content->data, content->size); HOEDOWN_BUFPUTSL(ob, "</em>"); return 1; }
PVS-Studio Warnung:
V547 CWE-571 Der Ausdruck 'Inhalt' ist immer wahr. html.c 162
Dies ist wahrscheinlich immer noch kein Fehler, sondern einfach redundanter Code. Der Inhaltszeiger muss nicht erneut überprüft
werden . Wenn es Null ist, beendet die Funktion sofort ihre Arbeit.
Eine ähnliche Situation:
void Clipper::DoMaxima(TEdge *e) { .... else if( e->OutIdx >= 0 && eMaxPair->OutIdx >= 0 ) { if (e->OutIdx >= 0) AddLocalMaxPoly(e, eMaxPair, e->Top); DeleteFromAEL(e); DeleteFromAEL(eMaxPair); } .... }
PVS-Studio-
Warnung :
V547 CWE-571 Ausdruck 'e-> OutIdx> = 0' ist immer wahr. clipper.cpp 2983
Eine erneute Überprüfung (e-> OutIdx> = 0) ist nicht sinnvoll. Vielleicht ist dies jedoch ein Fehler. Zum Beispiel können wir davon ausgehen, dass die Variable
e-> Top aktiviert sein sollte. Dies ist jedoch nur eine Vermutung. Wir sind mit dem Projektcode nicht vertraut und können Fehler nicht von redundantem Code unterscheiden :).
Und noch ein Fall:
QString SExpression::toString(int indent) const { .... if (child.isLineBreak() && nextChildIsLineBreak) { if (child.isLineBreak() && (i > 0) && mChildren.at(i - 1).isLineBreak()) {
PVS-Studio Warnung:
V571 CWE-571 Wiederkehrende Prüfung. Die Bedingung 'child.isLineBreak ()' wurde bereits in Zeile 208 überprüft. Sexpression.cpp 209
Fehler in der Logik
void FootprintPreviewGraphicsItem::paint(....) noexcept { .... for (const Circle& circle : mFootprint.getCircles()) { layer = mLayerProvider.getLayer(*circle.getLayerName()); if (!layer) continue;
PVS-Studio Warnung:
V547 CWE-571 Ausdruck 'Ebene' ist immer wahr. footprintpreviewgraphicsitem.cpp 177
Da die Bedingung in der zweiten
if-Anweisung immer wahr ist, ist der
else- Zweig niemals erfüllt.
Zeigerprüfung vergessen
extern int ZEXPORT unzGetGlobalComment ( unzFile file, char * szComment, uLong uSizeBuf) { .... if (uReadThis>0) { *szComment='\0'; if (ZREAD64(s->z_filefunc,s->filestream,szComment,uReadThis)!=uReadThis) return UNZ_ERRNO; } if ((szComment != NULL) && (uSizeBuf > s->gi.size_comment)) *(szComment+s->gi.size_comment)='\0'; .... }
PVS-Studio Warnung:
V595 CWE-476 Der Zeiger 'szComment' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 2068, 2073. unzip.c 2068
Wenn
uReadThis> 0 ist , wird der
szComment- Zeiger
dereferenziert . Dies ist gefährlich, da dieser Zeiger möglicherweise null ist. Der Analysator zieht eine solche Schlussfolgerung auf der Grundlage der Tatsache, dass dieser Zeiger weiter auf Gleichheit
NULL geprüft wird.
Nicht initialisiertes Klassenmitglied
template <class T> class Edge { public: using VertexType = Vector2<T>; Edge(const VertexType &p1, const VertexType &p2, T w=-1) : p1(p1), p2(p2), weight(w) {};
PVS-Studio Warnung:
V730 CWE-457 Nicht alle Mitglieder einer Klasse werden im Konstruktor initialisiert. Betrachten Sie Folgendes: isBad. edge.h 14
Alle Konstruktoren außer dem ersten initialisieren das Feld der
isBad- Klasse. Höchstwahrscheinlich hat der erste Konstruktor diese Initialisierung einfach versehentlich vergessen. Infolgedessen erstellt der erste Konstruktor ein unvollständig initialisiertes Objekt, was zu undefiniertem Programmverhalten führen kann.
Es gibt 11 weitere
V730- Diagnoseauslöser. Da wir mit dem Code jedoch nicht vertraut sind, ist es schwierig zu sagen, ob diese Warnungen auf echte Probleme hinweisen. Ich denke, diese Warnungen werden am besten von den Autoren des Projekts untersucht.
Speicherverlust
template <typename ElementType> void ProjectLibrary::loadElements(....) { .... ElementType* element = new ElementType(elementDir, false);
PVS-Studio Warnung:
V773 CWE-401 Die Ausnahme wurde ausgelöst, ohne den Zeiger 'Element' freizugeben. Ein Speicherverlust ist möglich. projectlibrary.cpp 245
Wenn ein Element bereits in der Liste enthalten ist, wird eine Ausnahme ausgelöst. Dies zerstört jedoch nicht das zuvor erstellte Objekt, dessen Zeiger in der Elementvariablen gespeichert ist.
Ungültiger Ausnahmetyp
bool CmdRemoveSelectedSchematicItems::performExecute() { .... throw new LogicError(__FILE__, __LINE__); .... }
PVS-Studio Warnung:
V1022 CWE-755 Eine Ausnahme wurde vom Zeiger ausgelöst. Ziehen Sie es stattdessen nach Wert. cmdremoveselectedschematicitems.cpp 143
Der Analysator hat eine vom Zeiger ausgelöste Ausnahme festgestellt. Meistens ist es üblich, Ausnahmen nach Wert auszulösen und nach Referenz abzufangen. Das Werfen eines Zeigers kann dazu führen, dass die Ausnahme nicht abgefangen wird, da sie als Referenz abgefangen wird. Die Verwendung eines Zeigers zwingt den Interceptor außerdem, den
Löschoperator aufzurufen, um das erstellte Objekt zu zerstören, damit keine Speicherverluste auftreten.
Im Allgemeinen wird der
neue Operator hier versehentlich geschrieben und sollte gelöscht werden. Dass dies ein Fehler ist, wird durch die Tatsache bestätigt, dass an allen anderen Stellen steht:
throw LogicError(__FILE__, __LINE__);
Gefährliche Verwendung von dynamic_cast
void GraphicsView::handleMouseWheelEvent( QGraphicsSceneWheelEvent* event) noexcept { if (event->modifiers().testFlag(Qt::ShiftModifier)) .... } bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... handleMouseWheelEvent(dynamic_cast<QGraphicsSceneWheelEvent*>(event)); .... }
PVS-Studio Warnung:
V522 CWE-628 Es kann zu einer Dereferenzierung des Nullzeiger-Ereignisses kommen. Der potenzielle Nullzeiger wird an die Funktion 'handleMouseWheelEvent' übergeben. Überprüfen Sie das erste Argument. Überprüfen Sie die Zeilen: 143, 252. graphicsview.cpp 143
Der aus dem Operator
dynamic_cast resultierende Zeiger wird an die Funktion
handleMouseWheelEvent übergeben . Darin wird dieser Zeiger ohne vorherige Überprüfung dereferenziert.
Dies ist gefährlich, da der Operator
dynamic_cast möglicherweise
nullptr zurückgibt . Es stellt sich heraus, dass dieser Code nicht besser ist als nur die Verwendung des schnelleren
static_cast .
Vor der Verwendung sollte diesem Code eine explizite Zeigerprüfung hinzugefügt werden.
Auch Code dieser Art ist sehr verbreitet:
bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... QGraphicsSceneMouseEvent* e = dynamic_cast<QGraphicsSceneMouseEvent*>(event); Q_ASSERT(e); if (e->button() == Qt::MiddleButton) .... }
PVS-Studio Warnung:
V522 CWE-690 Möglicherweise wird ein potenzieller Nullzeiger 'e' dereferenziert. graphicsview.cpp 206
Der Zeiger wird mit dem Makro
Q_ASSERT überprüft. Hier ist seine Beschreibung:
Druckt eine Warnmeldung mit dem Namen der Quellcodedatei und der Zeilennummer, wenn der Test falsch ist.
Q_ASSERT () ist nützlich, um Vor- und Nachbedingungen während der Entwicklung zu testen. Es tut nichts, wenn QT_NO_DEBUG während der Kompilierung definiert wurde.
|
Q_ASSERT ist eine schlechte Methode, um Zeiger vor der Verwendung zu überprüfen. In der Release-Version ist
QT_NO_DEBUG in der
Regel nicht definiert. Ich weiß nicht, wie es im LibrePCB-Projekt läuft. Wenn jedoch
QT_NO_DEBUG in Release definiert ist, ist dies eine seltsame und nicht standardmäßige Lösung.
Wenn sich das Makro in die Leere ausdehnt, stellt sich heraus, dass keine Überprüfung erfolgt. Und dann ist nicht klar, warum Sie
dynamic_cast überhaupt verwenden. Warum dann nicht
static_cast verwenden ?
Im Allgemeinen ist dieser Code geruchlos und eine Überprüfung aller ähnlichen Codefragmente wert. Und es gibt übrigens viele von ihnen.
Ich habe 82 ähnliche Fälle gezählt!Fazit
Im Allgemeinen schien uns das LibrePCB-Projekt von hoher Qualität zu sein. Wir empfehlen jedoch, dass sich die Autoren des Projekts mit dem PVS-Studio-Tool ausrüsten und unabhängig eine Codeüberprüfung der vom Analysator angegebenen Codeabschnitte durchführen. Wir sind bereit, ihnen einen Monat lang eine kostenlose Lizenz für eine vollständige Analyse des Projekts zur Verfügung zu stellen. Darüber hinaus können sie die kostenlose Lizenzierungsoption für Analysegeräte verwenden, da der Projektcode geöffnet und auf der GitHub-Website veröffentlicht ist. Wir werden in Kürze über diese Lizenzoption schreiben.
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Andrey Karpov, Svyatoslav Razmyslov.
Überprüfen von LibrePCB mit PVS-Studio in einem Docker-Container .