Der Winter steht vor der Tür, das Jahr strebt das Ende an, was bedeutet, dass es an der Zeit ist, die interessantesten Fehler zu berücksichtigen, die der PVS-Studio-Analysator im Jahr 2020 entdeckt hat.
Es ist erwähnenswert, dass das vergangene Jahr von einer Vielzahl neuer Diagnoseregeln geprägt war, deren Auslösung es ihnen ermöglichte, in diese Spitze zu gelangen. Wir verbessern auch den Analysatorkern weiter und fügen neue Szenarien für seine Verwendung hinzu. Sie können dies alles in unserem Blog lesen . Wenn Sie an anderen Sprachen interessiert sind, die von unserem Analysegerät unterstützt werden (C, C # und Java), lesen Sie die Artikel meiner Kollegen. Kommen wir nun zu den denkwürdigsten Fehlern, die PVS-Studio im letzten Jahr gefunden hat.
Zehnter Platz: Modulo Division durch eins
V1063 Die Modulo-by-1-Operation ist bedeutungslos. Das Ergebnis ist immer Null. llvm-stress.cpp 631
void Act() override {
....
// If the value type is a vector, and we allow vector select,
// then in 50% of the cases generate a vector select.
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
unsigned NumElem =
cast<FixedVectorType>(Val0->getType())->getNumElements();
CondTy = FixedVectorType::get(CondTy, NumElem);
}
....
}
Der Entwickler wollte einen zufälligen Wert zwischen 0 und 1 mithilfe der Modulo-Division erhalten. Eine Operation wie X% 1 gibt jedoch immer 0 zurück. In diesem Fall wäre es richtig, die Bedingung wie folgt umzuschreiben:
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))
Dieser Fehler wurde oben aus dem Artikel " Überprüfen von Clang 11 mit PVS-Studio " aufgenommen.
Neunter Platz: Vier Schecks
PVS-Studio gab vier Warnungen für den nächsten Abschnitt des Codes aus:
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: x> = 0. editor.cpp 1137
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: y> = 0. editor.cpp 1137
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: x <40. Editor.cpp 1137
- V560 Ein Teil des bedingten Ausdrucks ist immer wahr: y <30. Editor.cpp 1137
int editorclass::at( int x, int y )
{
if(x<0) return at(0,y);
if(y<0) return at(x,0);
if(x>=40) return at(39,y);
if(y>=30) return at(x,29);
if(x>=0 && y>=0 && x<40 && y<30)
{
return contents[x+(levx*40)+vmult[y+(levy*30)]];
}
return 0;
}
Alle Warnungen gelten für die letzte if- Anweisung. Das Problem ist, dass alle vier darin durchgeführten Überprüfungen immer true zurückgeben . Ich würde nicht sagen, dass dies ein schwerwiegender Fehler ist, aber es stellte sich als ziemlich lustig heraus. Im Allgemeinen sind diese Überprüfungen redundant und können entfernt werden.
Dieser Fehler wurde oben im Artikel eingegeben: " VVVVVV ??? VVVVVV !!! ".
Achter Platz: Löschen statt Löschen []
V611 Der Speicher wurde mit dem Operator 'new T []' zugewiesen, aber mit dem Operator 'delete' freigegeben. Überprüfen Sie diesen Code. Es ist wahrscheinlich besser, 'delete [] poke_data;' zu verwenden. CCDDE.CPP 410
BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
....
char *poke_data = new char [length + 2*sizeof(int)]; // <=
....
if(DDE_Class->Poke_Server( .... ) == FALSE) {
CCDebugString("C&C95 - POKE failed!\n");
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (FALSE);
}
DDE_Class->Close_Poke_Connection();
delete poke_data; // <=
return (TRUE);
}
Der Analysator hat einen Fehler festgestellt, der darauf zurückzuführen ist, dass Speicher auf inkompatible Weise zugewiesen und freigegeben wurde. Verwenden Sie den Operator delete [] und nicht delete , um den für ein Array zugewiesenen Speicher freizugeben .
Dieser Fehler hat es aus dem Artikel " Command & Conquer Game Code: Bugs aus den 90ern. Band zwei " nach oben geschafft.
Siebter Platz: Puffer außerhalb der Grenzen
Schauen wir uns die Funktion net_hostname_get an , die als nächstes verwendet wird.
#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
#endif
In diesem Fall wurde während der Vorverarbeitung die Option für den Zweig #else ausgewählt . Das heißt, in der vorverarbeiteten Datei wird die Funktion wie folgt implementiert:
static inline const char *net_hostname_get(void)
{
return "zephyr";
}
Die Funktion gibt einen Zeiger auf ein Array von 7 Bytes zurück (berücksichtigen Sie die Endnull am Ende der Zeichenfolge).
Schauen wir uns nun den Code an, der zu einem Array-Überlauf führt.
static int do_net_init(void)
{
....
(void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
....
}
PVS-Studio- Warnung : V512 [CWE-119] Ein Aufruf der Funktion 'memcpy' führt dazu, dass der Puffer 'net_hostname_get ()' außerhalb des Bereichs liegt. log_backend_net.c 114
Nach der Vorverarbeitung wird MAX_HOSTNAME_LEN wie folgt erweitert:
(void)memcpy(hostname, net_hostname_get(),
sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));
Dementsprechend tritt beim Kopieren von Daten ein Überlaufen des Zeichenfolgenliteral auf. Wie sich dies auf die Programmausführung auswirkt, ist schwer vorherzusagen, da dies zu undefiniertem Verhalten führt.
Dieser Fehler hat es bis zum Anfang des Artikels geschafft: " Untersuchung der Qualität des Zephyr-Betriebssystemcodes ".
Sechster Platz: Etwas sehr Seltsames
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
memcpy(cpy_mntpt, mntpt, strlen(mntpt));
}
return cpy_mntpt;
}
PVS-Studio- Warnung : V575 [CWE-628] Die Funktion 'memcpy' kopiert nicht die gesamte Zeichenfolge. Verwenden Sie die Funktion 'strcpy / strcpy_s', um das Terminal null beizubehalten. shell.c 427
Jemand hat versucht, ein Analogon der strdup- Funktion zu erstellen , aber sie sind fehlgeschlagen.
Beginnen wir mit der Warnung des Analysators. Es heißt, dass die memcpy- Funktion die Zeile kopiert, aber das Terminal Null nicht kopiert, was sehr verdächtig ist.
Dieses Terminal 0 scheint hier kopiert zu sein:
((u8_t *)mntpt)[strlen(mntpt)] = '\0';
Aber nein! Hier ist ein Tippfehler, der bewirkt, dass das Terminal Null in sich selbst kopiert wird! Beachten Sie, dass das Schreiben in das Array mntpt und nicht in cpy_mntpt erfolgt . Infolgedessen gibt die Funktion mntpt_prepare eine unvollständige Terminal- Nullzeichenfolge zurück .
Eigentlich wollte der Programmierer so schreiben:
((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';
Es ist jedoch immer noch nicht klar, warum es so schwierig gemacht wurde! Dieser Code kann wie folgt vereinfacht werden:
static char *mntpt_prepare(char *mntpt)
{
char *cpy_mntpt;
cpy_mntpt = k_malloc(strlen(mntpt) + 1);
if (cpy_mntpt) {
strcpy(cpy_mntpt, mntpt);
}
return cpy_mntpt;
}
Dieser Fehler hat es bis zum Anfang des oben genannten Artikels geschafft: " Überprüfen der Qualität des Zephyr-Betriebssystemcodes ".
Fünfter Platz: Falscher Überlaufschutz
V547 [CWE-570] Der Ausdruck 'rel_wait <0' ist immer falsch. Der vorzeichenlose Typwert ist niemals <0. Os_thread_windows.c 359
static DWORD
get_rel_wait(const struct timespec *abstime)
{
struct __timeb64 t;
_ftime64_s(&t);
time_t now_ms = t.time * 1000 + t.millitm;
time_t ms = (time_t)(abstime->tv_sec * 1000 +
abstime->tv_nsec / 1000000);
DWORD rel_wait = (DWORD)(ms - now_ms);
return rel_wait < 0 ? 0 : rel_wait;
}
In diesem Fall wird die rel_wait Variable ist der unsigned DWORD - Typ . Dies bedeutet, dass der Vergleich rel_wait <0 nicht sinnvoll ist, da das Ergebnis immer wahr ist.
Der Fehler selbst ist nicht sehr interessant. Aber es stellte sich als interessant heraus, wie sie versuchten, das Problem zu beheben. Es stellte sich heraus, dass die Änderungen nicht behoben wurden, sondern nur den Code vereinfachten. Weitere Informationen zu dieser Geschichte finden Sie in dem Artikel meines Kollegen: " Warum PVS-Studio keine automatischen Code-Änderungen anbietet ".
Der Fehler wurde oben im Artikel eingegeben: " Statische Analyse des Codes der Sammlung von PMDK-Bibliotheken von Intel und Fehler, die keine Fehler sind ".
Vierter Platz: Schreiben Sie nicht an std, Bruder
V1061 Das Erweitern des ' std' -Namespace kann zu undefiniertem Verhalten führen. size_iterator.hh 210
// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
util::SizedIterator second)
{
util::swap(*first, *second);
}
} // namespace std
Der Artikel, aus dem der Auslöser stammt: " Analysieren des Codes des DeepSpeech-Projekts oder warum Sie nicht in den Namespace std schreiben sollten " beschreibt ausführlich, warum Sie dies nicht tun sollten.
Dritter Platz: Bildlaufleiste, die fehlgeschlagen ist
V501 . Links und rechts vom Operator '-' befinden sich identische Unterausdrücke: bufferHeight - bufferHeight TermControl.cpp 592
bool TermControl::_InitializeTerminal()
{
....
auto bottom = _terminal->GetViewport().BottomExclusive();
auto bufferHeight = bottom;
ScrollBar().Maximum(bufferHeight - bufferHeight);
ScrollBar().Minimum(0);
ScrollBar().Value(0);
ScrollBar().ViewportSize(bufferHeight);
....
}
Dies wird als "Auslösen mit der Geschichte" bezeichnet. In diesem Fall funktionierte die Bildlaufleiste im Windows-Terminal aufgrund eines Fehlers nicht. Basierend auf diesem Fehler wurde ein ganzer Artikel geschrieben, in dem mein Kollege Nachforschungen anstellte und herausfand, warum dies geschah. Bist du interessiert? Hier ist es: "Die Bildlaufleiste, die nicht konnte ".
Zweiter Platz: verwirrter Radius und Höhe
Und wieder werden wir über einige Warnungen des Analysators sprechen:
- V764 Möglicherweise falsche Reihenfolge der an die Funktion 'CreateWheel' übergebenen Argumente: 'height' und 'radius'. StandardJoints.cpp 791
- V764 Möglicherweise falsche Reihenfolge der an die Funktion 'CreateWheel' übergebenen Argumente: 'height' und 'radius'. StandardJoints.cpp 833
- V764 Möglicherweise falsche Reihenfolge der an die Funktion 'CreateWheel' übergebenen Argumente: 'height' und 'radius'. StandardJoints.cpp 884
Hier sind die Funktionsaufrufe:
NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);
Und so sieht ihre Anzeige aus:
static NewtonBody* CreateWheel (DemoEntityManager* const scene,
const dVector& location, dFloat radius, dFloat height)
Argumente wurden in Funktionsaufrufen umgekehrt.
Dieser Fehler hat es aus dem Artikel " Newton Game Dynamics mit dem statischen Analysator PVS-Studio erneut überprüfen " nach oben geschafft.
Erster Platz: Löschen des Ergebnisses
V519 Der Variablen 'color_name' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 621, 627. string.cpp 627
static bool parseNamedColorString(const std::string &value,
video::SColor &color)
{
std::string color_name;
std::string alpha_string;
size_t alpha_pos = value.find('#');
if (alpha_pos != std::string::npos) {
color_name = value.substr(0, alpha_pos);
alpha_string = value.substr(alpha_pos + 1);
} else {
color_name = value;
}
color_name = lowercase(value); // <=
std::map<const std::string, unsigned>::const_iterator it;
it = named_colors.colors.find(color_name);
if (it == named_colors.colors.end())
return false;
....
}
Diese Funktion sollte den Farbnamen mit dem Transparenzparameter analysieren und seinen hexadezimalen Code zurückgeben. Abhängig vom Ergebnis der Überprüfung der Bedingung wird entweder das Ergebnis der Zeilenaufteilung oder eine Kopie des Funktionsarguments an die Variable color_name übergeben .
In der Funktion Kleinbuchstaben () wird jedoch nicht die resultierende Zeichenfolge selbst in Kleinbuchstaben konvertiert, sondern das ursprüngliche Funktionsargument. Infolgedessen verlieren wir einfach die Farbe, die parseNamedColorString () hätte zurückgeben sollen .
color_name = lowercase(color_name);
Dieser Fehler hat es aus dem Artikel " PVS-Studio: Analyse von Pull-Anforderungen in Azure DevOps mit selbst gehosteten Agenten " nach oben geschafft.
Fazit
Im letzten Jahr haben wir viele Fehler in Open Source-Projekten gefunden. Dies waren häufige Fehler beim Kopieren und Einfügen, ständige Fehler, Speicherlecks und viele andere Probleme. Unser Analysegerät steht nicht still, und oben sind einige positive Aspekte der neuen Diagnostik aufgeführt, die dieses Jahr geschrieben wurden.
Ich hoffe, Ihnen haben die gesammelten Fehler gefallen. Ich persönlich fand sie sehr interessant. Aber natürlich kann Ihre Vision von meiner abweichen, sodass Sie Ihre "Top 10 ..." erreichen können, indem Sie Artikel aus unserem Blog lesen oder sich die Liste der Fehler ansehen, die PVS-Studio in Open Source-Projekten gefunden hat.
Ich mache Sie auch auf Artikel mit den 10 häufigsten C ++ - Fehlern der letzten Jahre aufmerksam: 2016 , 2017 , 2018 , 2019 .
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Vladislav Stolyarov. Die 10 häufigsten Fehler in C ++ - Projekten im Jahr 2020 .