PVS-Studio für Java unter der Haube: Diagnoseentwicklung

Zur Abwechslung werden wir Ihnen heute ein wenig über den Prozess der Entwicklung und Fertigstellung von Diagnoseregeln für PVS-Studio Java erzählen. Mal sehen, warum die alten Analysator-Trigger von Release zu Release nicht zu stark schweben und die neuen nicht zu verrückt sind. Und wir werden ein bisschen mehr "was sind die Pläne der Javisten" verderben und ein paar schöne (und nicht so) Fehler zeigen, die mit Hilfe der Diagnose ab der nächsten Version gefunden wurden.

Entwicklungsprozess für Diagnose und SelfTester

Natürlich beginnt jede neue Diagnoseregel mit einer Idee. Und da der Java-Analysator die jüngste Richtung in der Entwicklung von PVS-Studio ist, stehlen wir diese Ideen im Wesentlichen den Abteilungen C / C ++ und C #. Aber nicht alles ist so schlecht: Wir fügen auch Regeln hinzu, die von uns selbst erfunden wurden (auch von Benutzern - danke!), Damit später dieselben Abteilungen sie uns stehlen. Der Zyklus, wie sie sagen.



Die Implementierung der Regeln im Code erweist sich in den meisten Fällen als eine ziemliche Pipeline-Aufgabe. Sie erstellen eine Datei mit mehreren synthetischen Beispielen, markieren mit Ihren Händen, wo Fehler sein sollten, und mit dem bereitstehenden Debugger durchlaufen Sie den Syntaxbaum, bis Sie sich langweilen und alle erfundenen Fälle abdecken. Manchmal erweisen sich die Regeln als absurd einfach (zum Beispiel V6063)besteht buchstäblich aus ein paar Zeilen), und manchmal muss man lange genug über die Logik nachdenken.



Dies ist jedoch nur der Anfang des Prozesses. Wie Sie wissen, mögen wir synthetische Beispiele nicht besonders, da sie die Art der Analysatorauslöser in realen Projekten sehr schlecht widerspiegeln. Übrigens stammen die meisten dieser Beispiele in unseren Unit-Tests aus realen Projekten - es ist fast unmöglich, alle möglichen Fälle selbst zu erfinden. Und Unit-Tests ermöglichen es uns auch, Auslöser für Beispiele aus der Dokumentation nicht zu verlieren. Es gab Präzedenzfälle, ja, nur shh.



Die positiven Aspekte in realen Projekten müssen also zuerst auf irgendeine Weise gefunden werden. Und Sie müssen das auch irgendwie überprüfen:

• Die Regel wird nicht auf Open-Source-Wahnsinn fallen, wo "interessante" Lösungen üblich sind;
• ( - , );
• data-flow ( ) - ;
• open-source ;
• over 9000%;
• "" , ;
• .

Im Allgemeinen tritt SelfTester hier wie ein Ritter auf einem Pferd (ein wenig hinkend, aber wir arbeiten daran) in den Vordergrund. Die Hauptaufgabe besteht darin, eine Reihe von Projekten automatisch zu überprüfen und anzuzeigen, welche Trigger in Bezug auf die "Referenz" im Versionskontrollsystem hinzugefügt, verschwunden oder geändert wurden. Geben Sie Unterschiede für den Analysatorbericht an und zeigen Sie kurz den entsprechenden Code in Projekten an. Derzeit testet SelfTester für Java 62 Open-Source-Projekte mit bärtigen Versionen, darunter beispielsweise DBeaver, Hibernate und Spring. Ein vollständiger Durchlauf aller Projekte dauert 2 bis 2,5 Stunden, was zweifellos schmerzhaft ist, aber nichts getan werden kann.





Im obigen Screenshot sind die "grünen" Projekte diejenigen, an denen sich nichts geändert hat. Jeder Unterschied in "roten" Projekten wird manuell überprüft und, falls korrekt, durch dieselbe Schaltfläche "Genehmigen" bestätigt. Das Analysator-Verteilungskit wird übrigens nur kompiliert, wenn SelfTester ein rein grünes Ergebnis liefert. Im Allgemeinen erhalten wir auf diese Weise die Konsistenz der Ergebnisse zwischen verschiedenen Versionen.



SelfTester gewährleistet nicht nur die Konsistenz der Ergebnisse, sondern ermöglicht es uns auch, eine große Anzahl von Fehlalarmen zu beseitigen, noch bevor die Diagnose veröffentlicht wird. Ein typisches Entwicklungsmuster sieht folgendermaßen aus:

• , . , " double-checked locking" ;
• SelfTester-, ;
• , -;
• SelfTester- , ;
• 3-4, ;
• , , ( , );
• , master.

Glücklicherweise sind vollständige SelfTester-Läufe selten genug und Sie müssen nicht sehr oft "2-2,5 Stunden" warten. Von Zeit zu Zeit umgehen Glück und Auslöser große Projekte wie Sakai und Apache Hive - es ist Zeit, Kaffee zu trinken, Kaffee zu trinken und Kaffee zu trinken. Sie können auch die Dokumentation studieren, dies ist jedoch nicht jedermanns Sache.



"Warum brauchen wir Unit-Tests, da es so ein magisches Werkzeug gibt?"



Und dann sind diese Tests deutlich schneller. Ein paar Minuten - und es gibt bereits ein Ergebnis. Sie können auch genau sehen, welcher Teil der Regel abgefallen ist. Außerdem werden in SelfTester-Projekten nicht immer alle zulässigen Auslösungen einer Regel erfasst, sondern auch ihre Funktionsfähigkeit überprüft.

Neue Probleme bei alten Bekannten

Anfänglich begann dieser Abschnitt des Artikels mit den Worten "Die Versionen von Projekten in SelfTester sind ziemlich alt, sodass die meisten der vorgestellten Fehler höchstwahrscheinlich bereits behoben wurden." Als ich mich jedoch entschied, dies sicherzustellen, war ich überrascht. Jeder einzelne Fehler blieb bestehen. Alles. Dies bedeutet zwei Dinge:

• Diese Fehler sind für die Funktion der Anwendung nicht besonders wichtig. Viele von ihnen befinden sich übrigens im Testcode, und falsche Tests können kaum als konsistent bezeichnet werden.
• Diese Fehler finden sich in selten verwendeten Dateien großer Projekte, zu denen Entwickler kaum gehen. Aus diesem Grund ist der falsche Code dazu verdammt, sehr lange dort zu liegen: höchstwahrscheinlich, bis ein kritischer Fehler auftritt.

Für diejenigen, die tiefer graben möchten, gibt es Links zu bestimmten Versionen, die wir überprüfen.



PS Das oben Gesagte bedeutet nicht, dass die statische Analyse nur harmlose Fehler in nicht verwendetem Code abfängt. Wir überprüfen die Release-Versionen (und fast die Release-Versionen) von Projekten, bei denen Entwickler und Tester (und manchmal leider auch Benutzer) die relevantesten Fehler von Hand gefunden haben. Dies ist lang, teuer und schmerzhaft. Weitere Informationen hierzu finden Sie in unserem Artikel " Fehler, die bei der statischen Code-Analyse nicht gefunden werden können, weil sie nicht verwendet werden ".

Apache Dubbo und leeres Menü

GitHub



Diagnostics " V6080 Erwägen Sie, nach Druckfehlern zu suchen. Es ist möglich, dass eine zugewiesene Variable in der nächsten Bedingung überprüft wird " wurde bereits in Version 7.08 veröffentlicht, ist jedoch noch nicht in unseren Artikeln enthalten. Daher ist es an der Zeit, dies zu beheben.



Menu.java:40

public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.get(menu);
    if (item == null)                      // <=
    {
      items = new ArrayList<String>();
      menus.put(menu, items);
    }
    items.add(item);
  }
  ....
}

Ein klassisches Beispiel für ein "Key-Collection" -Wörterbuch und einen ebenso klassischen Tippfehler. Der Entwickler wollte eine Sammlung erstellen, die dem Schlüssel entspricht, falls dieser noch nicht vorhanden ist. Er hat jedoch den Namen der Variablen verwechselt und in der letzten Zeile nicht nur eine falsche Operation der Methode, sondern auch eine NullPointerException erhalten . Für Java 8 und höher sollten Sie zum Implementieren solcher Wörterbücher die Methode computeIfAbsent verwenden :

public class Menu
{
  private Map<String, List<String>> menus = new HashMap<String, List<String>>();

  public void putMenuItem(String menu, String item)
  {
    List<String> items = menus.computeIfAbsent(menu, key -> new ArrayList<>());
    items.add(item);
  }
  ....
}

Glasfisch und doppelt geprüftes Schloss

GitHub



Eine der Diagnosen, die in der nächsten Version enthalten sein werden, ist die Überprüfung der korrekten Implementierung des Musters "Double Checked Locking". Glassfish erwies sich als Rekordhalter für Erkennungen aus SelfTester-Projekten: Insgesamt hat PVS-Studio nach dieser Regel 10 Problembereiche im Projekt gefunden. Ich lade den Leser ein, Spaß zu haben und im Code-Snippet unten nach zwei davon zu suchen. Hilfe finden Sie in der Dokumentation: " V6082 Unsichere doppelt überprüfte Verriegelung ". Nun, oder, wenn Sie überhaupt nicht wollen, am Ende des Artikels.



EjbComponentAnnotationScanner.java

public class EjbComponentAnnotationScanner
{
  private Set<String> annotations = null;

  public boolean isAnnotation(String value)
  {
    if (annotations == null)
    {
      synchronized (EjbComponentAnnotationScanner.class)
      {
        if (annotations == null)
        {
          init();
        }
      }
    }
    return annotations.contains(value);
  }

  private void init()
  {
    annotations = new HashSet();
    annotations.add("Ljavax/ejb/Stateless;");
    annotations.add("Ljavax/ejb/Stateful;");
    annotations.add("Ljavax/ejb/MessageDriven;");
    annotations.add("Ljavax/ejb/Singleton;");
  }

  ....
}

SonarQube und Datenfluss

GitHub Bei der



Verbesserung der Diagnose geht es nicht nur darum, den Code direkt zu ändern, um verdächtigere Stellen zu erkennen oder Fehlalarme zu entfernen. Die manuelle Kennzeichnung von Methoden für den Datenfluss spielt auch eine wichtige Rolle bei der Entwicklung des Analysators. Sie können beispielsweise schreiben, dass eine solche und eine solche Bibliotheksmethode immer ungleich Null zurückgibt. Beim Schreiben einer neuen Diagnose haben wir versehentlich festgestellt, dass die Map # clear () -Methode nicht markiert war . Abgesehen von dem offensichtlich dummen Code, dass die " V6009-Sammlung leer ist. Der Aufruf der Funktion" Löschen "ist sinnlos " zu fangen begann, konnten wir einen großartigen Tippfehler finden.



MetricRepositoryRule.java:90

protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}

Auf den ersten Blick ist das erneute Löschen des Wörterbuchs kein Fehler. Und wir würden sogar denken, dass dies eine zufällig duplizierte Linie ist, wenn unser Blick nicht ein wenig tiefer gefallen wäre - buchstäblich zur nächsten Methode.

protected void after()
{
  this.metricsById.clear();
  this.metricsById.clear();
}
public Metric getByKey(String key)
{
  Metric res = metricsByKey.get(key);
  ....
}

Genau. Die Klasse hat zwei Felder mit ähnlichen Namen MetricsById und MetricsByKey . Ich bin sicher, dass der Entwickler in der After- Methode beide Wörterbücher löschen wollte, aber entweder schlug die automatische Vervollständigung fehl oder er gab träge denselben Namen ein. Daher sind die beiden Wörterbücher, die verwandte Daten enthalten, nach dem Aufruf von after nicht mehr synchron .

Sakai und leere Sammlungen

GitHub



Eine weitere neue Diagnose, die in der nächsten Version enthalten sein wird, ist " V6084 Verdächtige Rückgabe einer immer leeren Sammlung ". Es ist leicht zu vergessen, Elemente zur Sammlung hinzuzufügen, insbesondere wenn jedes Element zuerst initialisiert werden muss. Aus persönlicher Erfahrung führen solche Fehler meist nicht zu einem Absturz der Anwendung, sondern zu seltsamem Verhalten oder dem Fehlen jeglicher Funktionalität.



DateModel.java:361

public List getDaySelectItems()
{
  List selectDays = new ArrayList();
  Integer[] d = this.getDays();
  for (int i = 0; i < d.length; i++)
  {
    SelectItem selectDay = new SelectItem(d[i], d[i].toString());
  }
  return selectDays;
}

Übrigens enthält dieselbe Klasse sehr ähnliche Methoden ohne denselben Fehler. Zum Beispiel:

public List getMonthSelectItems()
{
  List selectMonths = new ArrayList();
  Integer[] m = this.getMonths();
  for (int i = 0; i < m.length; i++)
  {
    SelectItem selectMonth = new SelectItem(m[i], m[i].toString());
    selectMonths.add(selectMonth);
  }
  return selectMonths;
}

Pläne für die Zukunft

Abgesehen von verschiedenen nicht sehr interessanten internen Dingen denken wir darüber nach, dem Java-Analysator Diagnosen für das Spring Framework hinzuzufügen. Es ist nicht nur das Hauptbrot für Javisten, sondern enthält auch viele nicht offensichtliche Momente, über die man stolpern kann. Wir sind uns noch nicht ganz sicher, in welcher Form diese Diagnose irgendwann erscheinen wird, wann sie stattfinden wird und ob sie überhaupt stattfinden wird. Wir sind jedoch sicher, dass wir Ideen für sie und Open-Source-Projekte mit Spring for SelfTester benötigen. Wenn Sie also etwas im Sinn haben, schlagen Sie es vor (in Kommentaren oder privaten Nachrichten können Sie dies auch)! Und je mehr von dieser Güte wir sammeln, desto mehr Priorität wird sich darauf verlagern.



Und schließlich gibt es Fehler in der doppelt überprüften Sperrimplementierung von Glassfish:

• Das Feld wird nicht als "flüchtig" deklariert.
• Das Objekt wird zuerst veröffentlicht und dann initialisiert.

Warum das alles schlecht ist - wieder können Sie in der Dokumentation sehen .

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Nikita Lazeba. Unter der Haube von PVS-Studio für Java: Wie wir die Diagnose entwickeln .