Lösungen für das Auffinden von Fehlern, die das PVS-Studio-Team auf Konferenzen in den Jahren 2018-2019 anbietet

Bild 2


Hi! Obwohl die Konferenzsaison 2019 noch nicht vorbei ist, möchten wir über die Herausforderungen sprechen, die wir den Besuchern an unserem Stand während der vergangenen Konferenzen bei der Fehlersuche angeboten haben. Ab Herbst 2019 bringen wir eine Reihe neuer Herausforderungen mit, sodass wir nun die Lösungen für die vorherigen Aufgaben von 2018 und das erste Halbjahr 2019 aufzeigen können - schließlich stammten viele von ihnen aus zuvor veröffentlichten Artikeln. und wir hatten einen Link oder QR-Code mit Informationen zu den jeweiligen Artikeln auf unseren Herausforderungsbroschüren.

Wenn Sie an Veranstaltungen teilgenommen haben, an denen wir mit einem Stand teilgenommen haben, haben Sie wahrscheinlich einige unserer Herausforderungen gesehen oder sogar versucht, sie zu lösen. Dies sind Codeausschnitte aus echten Open-Source-Projekten, die in C, C ++, C # oder Java geschrieben wurden. Jedes Snippet enthält einen Fehler, und die Gäste müssen versuchen, ihn zu finden. Eine erfolgreiche Lösung (oder einfach die Teilnahme an der Diskussion des Fehlers) wird mit einem Preis belohnt: einem spiralgebundenen Desktop-Status, einem Schlüsselbund und dergleichen:

Bild 4

Willst du auch welche? Dann schauen Sie doch einfach mal auf unserem Stand vorbei.

Übrigens, in den Artikeln " Conference Time! Summing 2018 " und " Conferences. Zwischensummen für das erste Halbjahr 2019 " teilen wir unsere Erfahrungen mit der Teilnahme an den Veranstaltungen, die Anfang dieses Jahres und im Jahr 2018 stattfanden.

Okay, lass uns unser "Find the Bug" Spiel spielen. Zunächst werfen wir einen Blick auf die früheren Herausforderungen des Jahres 2018, gegliedert nach Sprachen.

2018


C ++


Chrom Bug

static const int kDaysInMonth[13] = { 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }; bool ValidateDateTime(const DateTime& time) { if (time.year < 1 || time.year > 9999 || time.month < 1 || time.month > 12 || time.day < 1 || time.day > 31 || time.hour < 0 || time.hour > 23 || time.minute < 0 || time.minute > 59 || time.second < 0 || time.second > 59) { return false; } if (time.month == 2 && IsLeapYear(time.year)) { return time.month <= kDaysInMonth[time.month] + 1; } else { return time.month <= kDaysInMonth[time.month]; } } 

Lösung
Dieser in Chromium gefundene Fehler war wahrscheinlich die "langlebigste" Herausforderung. Wir haben es bis 2018 angeboten und es auch in mehrere Präsentationen aufgenommen.

 if (time.month == 2 && IsLeapYear(time.year)) { return time.month <= kDaysInMonth[time.month] + 1; // <= day } else { return time.month <= kDaysInMonth[time.month]; // <= day } 

Der Body des letzten If-else- Blocks enthält Tippfehler in den return-Anweisungen: time.month wurde versehentlich ein zweites Mal anstelle von time.day geschrieben . Dieser Fehler führt dazu, dass die Funktion die ganze Zeit wahr ist . Der Fehler wird im Artikel " 31. Februar " ausführlich besprochen und ist ein cooles Beispiel für einen Fehler, der durch die Codeüberprüfung nicht leicht entdeckt werden kann. Dieser Fall ist auch eine gute Demonstration, wie wir die Datenflussanalyse verwenden.

Unwirklicher Motorfehler

 bool VertInfluencedByActiveBone( FParticleEmitterInstance* Owner, USkeletalMeshComponent* InSkelMeshComponent, int32 InVertexIndex, int32* OutBoneIndex = NULL); void UParticleModuleLocationSkelVertSurface::Spawn(....) { .... int32 BoneIndex1, BoneIndex2, BoneIndex3; BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE; if(!VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[0], &BoneIndex1) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[1], &BoneIndex2) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[2]) &BoneIndex3) { .... } 

Lösung
Zunächst ist zu beachten, dass das letzte Argument der Funktion VertInfluencedByActiveBone () einen Standardwert hat und nicht angegeben werden muss. Betrachten Sie nun den if- Block in vereinfachter Form:

 if (!foo(....) && !foo(....) && !foo(....) & arg) 

Der Fehler ist jetzt deutlich sichtbar. Aufgrund des Tippfehlers wird der dritte Aufruf der Funktion VertInfluencedByActiveBone () mit drei statt vier Argumenten ausgeführt, wobei der Rückgabewert dann an einer & -Operation teilnimmt ( bitweises UND: Der linke Operand ist der Wert des Typs bool, der von VertInfluencedByActiveBone (zurückgegeben wird. ) und der rechte Operand ist die Ganzzahlvariable BoneIndex3 ). Der Code ist noch kompilierbar. Dies ist die feste Version (ein Komma wurde hinzugefügt, die schließende Klammer wurde an das Ende des Ausdrucks verschoben):

 if(!VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[0], &BoneIndex1) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[1], &BoneIndex2) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[2], &BoneIndex3)) 

Dieser Fehler wurde ursprünglich in dem Artikel " Ein lang erwarteter Check von Unreal Engine 4 " erwähnt, in dem er den Titel "der schönste Fehler" trug, dem ich voll und ganz zustimme.

Android-Fehler

 void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex); // Expand shorthands if (ssize_t idx = tagNames.find("3a") != -1) { ssize_t end = tagNames.find(",", idx); char* start = tagNames.lockBuffer(tagNames.size()); start[idx] = '\0'; .... } .... } 

Lösung
Der Programmierer hatte falsche Annahmen über die Priorität von Operationen im Zustand des if- Blocks. Dieser Code funktioniert nicht wie erwartet:

 if (ssize_t idx = (tagNames.find("3a") != -1)) 

Der Variablen idx wird der Wert 0 oder 1 zugewiesen. Ob die Bedingung wahr oder falsch ist, hängt von diesem Wert ab. Dies ist ein Fehler. Dies ist die feste Version:

 ssize_t idx = tagNames.find("3a"); if (idx != -1) 

Dieser Fehler wurde im Artikel " Wir haben den Android-Quellcode von PVS-Studio überprüft oder nichts ist perfekt " erwähnt.

Hier ist eine weitere nicht triviale Herausforderung mit einem Android-Fehler:

 typedef int32_t GGLfixed; GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { if ((d>>24) && ((d>>24)+1)) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } 

Lösung
Das Problem liegt im Ausdruck (d >> 24) + 1 .

Der Programmierer wollte überprüfen, ob die 8 höchstwertigen Bits der Variablen d auf 1 gesetzt sind, jedoch nicht alle gleichzeitig. Mit anderen Worten, sie wollten überprüfen, ob das höchstwertige Byte irgendeinen Wert außer 0x00 und 0xFF speichert. Zuerst prüft der Programmierer die höchstwertigen Bits mit dem Ausdruck (d >> 24) auf Null. Dann verschieben sie die acht höchstwertigen Bits in das niedrigstwertige Byte, wobei erwartet wird, dass das höchstwertige Vorzeichenbit in allen anderen Bits dupliziert wird. Das heißt, wenn die Variable d den Wert 0b11111111'00000000'00000000'0000000000 hat, wird sie nach der Verschiebung zu 0b11111111'111111'111111. Wenn der int- Wert 0xFFFFFFFF um 1 erhöht wird, erwartet der Programmierer 0 (-1 + 1 = 0). Somit wird der Ausdruck (((d >> 24) +1) verwendet, um zu überprüfen, dass nicht alle acht höchstwertigen Bits auf 1 gesetzt sind.

Das höchstwertige Vorzeichenbit wird jedoch nicht unbedingt "gespreizt", wenn es verschoben wird. Dies ist, was der Standard sagt: „Der Wert von E1 >> E2 ist E1 rechtsverschobene E2-Bitpositionen. Wenn E1 einen vorzeichenlosen Typ hat oder wenn E1 einen vorzeichenbehafteten Typ und einen nicht negativen Wert hat, ist der Wert des Ergebnisses der integrale Bestandteil des Quotienten von E1 / 2 ^ E2. Wenn E1 einen vorzeichenbehafteten Typ und einen negativen Wert hat, ist der resultierende Wert implementierungsdefiniert . "

Dies ist also ein Beispiel für implementierungsdefiniertes Verhalten. Wie genau dieser Code funktioniert, hängt von der CPU-Architektur und der Compiler-Implementierung ab. Die höchstwertigen Bits können nach der Verschiebung durchaus als Nullen enden, und der Ausdruck ((d >> 24) +1) würde dann immer einen anderen Wert als 0 zurückgeben, d. H. Einen immer wahren Wert.

Das ist in der Tat eine nicht triviale Herausforderung. Wie der vorherige Fehler wurde dieser ursprünglich im Artikel " Wir haben den Android-Quellcode von PVS-Studio überprüft, oder nichts ist perfekt " besprochen.

2019


C ++


"Es ist alles die Schuld von GCC"

 int foo(const unsigned char *s) { int r = 0; while(*s) { r += ((r * 20891 + *s *200) | *s ^ 4 | *s ^ 3) ^ (r >> 1); s++; } return r & 0x7fffffff; } 

Der Programmierer macht den GCC 8-Compiler für den Fehler verantwortlich. Ist es wirklich die Schuld von GCC?

Lösung
Die Funktion gibt negative Werte zurück, da der Compiler keinen Code für das bitweise UND (&) generiert. Der Fehler hat mit undefiniertem Verhalten zu tun. Der Compiler stellt fest, dass die Variable r zum Berechnen und Speichern einer Summe mit nur positiven Werten verwendet wird. Die Variable r sollte nicht überlaufen, da dies undefiniertes Verhalten wäre, mit dem der Compiler überhaupt nicht rechnen muss. Da r am Ende der Schleife keinen negativen Wert haben kann, ist die Operation r & 0x7fffffff , die das Vorzeichenbit löscht, nicht erforderlich , und weist die Funktion einfach an, den Wert von r zurückzugeben .

Dieser Fehler wurde im Artikel " PVS-Studio 6.26 Released " beschrieben.

QT-Fehler

 static inline const QMetaObjectPrivate *priv(const uint* data) { return reinterpret_cast<const QMetaObjectPrivate*>(data); } bool QMetaEnum::isFlag() const { const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1; return mobj && mobj->d.data[handle + offset] & EnumIsFlag; } 

Lösung
Der Mobj- Zeiger wird auf unsichere Weise behandelt: zuerst dereferenziert, dann überprüft. Ein Klassiker.

Der Fehler wurde im Artikel " Ein dritter Check von Qt 5 mit PVS-Studio " erwähnt.

C #


Infer.NET-Fehler

 public static void WriteAttribute(TextWriter writer, string name, object defaultValue, object value, Func<object, string> converter = null) { if ( defaultValue == null && value == null || value.Equals(defaultValue)) { return; } string stringValue = converter == null ? value.ToString() : converter(value); writer.Write($"{name}=\"{stringValue}\" "); } 

Lösung
Bei der Auswertung des Ausdrucks value.Equals (defaultValue) kann eine Null-Dereferenzierung der Wertvariablen auftreten . Dies geschieht, wenn die Werte der Variablen so sind, dass defaultValue! = Null und value == null .

Dieser Fehler stammt aus dem Artikel " Welche Fehler lauern im Infer.NET-Code? "

FastReport-Fehler

 public class FastString { private const int initCapacity = 32; private void Init(int iniCapacity) { sb = new StringBuilder(iniCapacity); .... } public FastString() { Init(initCapacity); } public FastString(int iniCapacity) { Init(initCapacity); } public StringBuilder StringBuilder => sb; } .... Console.WriteLine(new FastString(256).StringBuilder.Capacity); 

Was gibt das Programm in der Konsole aus? Was ist los mit der FastString- Klasse?

Lösung
Das Programm gibt den Wert 32 aus. Der Grund ist der falsch geschriebene Name der Variablen, die im Konstruktor an die Init- Methode übergeben wurde:

 public FastString(int iniCapacity){ Init(initCapacity); } 

Der Konstruktorparameter iniCapacity wird nicht verwendet. Stattdessen wird die Konstante initCapacity übergeben .

Der Fehler wurde im Artikel " Die schnellsten Berichte im Wilden Westen - und eine Handvoll Fehler ... " besprochen.

Roslyn Bug

 private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....)) { .... var nodeOrToken = current.ChildThatContainsPosition(....); .... current = nodeOrToken.AsNode(); } .... } public SyntaxNode AsNode() { if (_token != null) { return null; } return _nodeOrParent; } 

Lösung
Potenzielle Null-Dereferenzierung des Stroms im Ausdruck current.FullSpan.Contains (....) . Der aktuellen Variablen kann durch Aufrufen der nodeOrToken.AsNode () -Methode ein Nullwert zugewiesen werden.

Dieser Fehler stammt aus dem Artikel " Überprüfen des Roslyn-Quellcodes ".

Unity Bug

 .... staticFields = packedSnapshot.typeDescriptions .Where(t => t.staticFieldBytes != null & t.staticFieldBytes.Length > 0) .Select(t => UnpackStaticFields(t)) .ToArray() .... 

Lösung
Ein Tippfehler: Der Operator & wird anstelle von && verwendet . Dies führt dazu, dass die Prüfung t.staticFieldBytes.Length> 0 die ganze Zeit ausgeführt wird, auch wenn die Variable t.staticFieldBytes null ist , was wiederum zu einer Null-Dereferenzierung führt.

Dieser Fehler wurde ursprünglich im Artikel " Erläutern von Fehlern in den Open Source-Komponenten von Unity3D " angezeigt .

Java


IntelliJ IDEA-Fehler

 private static boolean checkSentenceCapitalization(@NotNull String value) { List<String> words = StringUtil.split(value, " "); .... int capitalized = 1; .... return capitalized / words.size() < 0.2; // allow reasonable amount of // capitalized words } 

Warum berechnet das Programm die Anzahl der großgeschriebenen Wörter falsch?

Lösung
Es wird erwartet, dass die Funktion true zurückgibt, wenn die Anzahl der großgeschriebenen Wörter weniger als 20% beträgt. Die Prüfung funktioniert jedoch nicht, da die Ganzzahldivision nur 0 oder 1 ergibt. Die Funktion gibt nur dann false zurück , wenn alle Wörter in Großbuchstaben geschrieben sind. Andernfalls ergibt die Division 0 und die Funktion gibt true zurück .

Dieser Fehler stammt aus dem Artikel " PVS-Studio für Java ".

Spotbugs Bug

 public static String getXMLType(@WillNotClose InputStream in) throws IOException { .... String s; int count = 0; while (count < 4) { s = r.readLine(); if (s == null) { break; } Matcher m = tag.matcher(s); if (m.find()) { return m.group(1); } } throw new IOException("Didn't find xml tag"); .... } 

Was stimmt nicht mit der Suche nach dem XML-Tag?

Lösung
Die Bedingung count <4 ist immer wahr, da die Anzahl der Variablen innerhalb der Schleife nicht erhöht wird. Das xml-Tag sollte in den ersten vier Zeilen der Datei gesucht werden, aber aufgrund des fehlenden Inkrements liest das Programm die gesamte Datei.

Wie der vorherige Fehler wurde dieser im Artikel " PVS-Studio für Java " beschrieben.

Das ist alles für heute. Besuchen Sie uns bei den kommenden Veranstaltungen - suchen Sie nach dem Einhorn. Wir werden neue interessante Herausforderungen anbieten und natürlich Preise vergeben. Wir sehen uns!

Source: https://habr.com/ru/post/de476268/


All Articles