Von Zeit zu Zeit müssen wir Artikel über die Überprüfung der nächsten Version eines Compilers schreiben. Es ist nicht interessant. Wie die Praxis zeigt, beginnen die Leute zu bezweifeln, ob der PVS-Studio-Analysator den Titel eines guten Fängers von Fehlern und potenziellen Schwachstellen verdient, wenn dies längere Zeit nicht getan wird. Vielleicht weiß der neue Compiler schon, wie das geht? Ja, Compiler stehen nicht still. PVS-Studio entwickelt sich jedoch auch weiter und demonstriert immer wieder die Fähigkeit, Fehler auch im Code von qualitativ hochwertigen Projekten wie Compilern zu finden.
Zeit, den Clang-Code noch einmal zu überprüfen
Um ehrlich zu sein, habe ich den Artikel " Überprüfen des GCC 10-Compilers mit PVS-Studio " als Grundlage für diesen Text verwendet . Wenn es Ihnen also so vorkommt, als hätten Sie bereits irgendwo einige Absätze gelesen, dann denken Sie nicht :).
Es ist kein Geheimnis, dass Compiler über eigene integrierte statische Code-Analysatoren verfügen, und sie werden auch entwickelt. Daher schreiben wir von Zeit zu Zeit Artikel darüber, wie der statische Analysator PVS-Studio selbst in Compilern Fehler finden kann und dass wir nicht umsonst Brot essen :).
Tatsächlich können Sie klassische statische Analysatoren nicht mit Compilern vergleichen. Bei statischen Analysatoren geht es nicht nur darum, Fehler im Code zu finden, sondern auch um eine entwickelte Infrastruktur. Dies ist beispielsweise die Integration in Systeme wie SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins und Visual Studio. Hierbei handelt es sich um fortschrittliche Mechanismen zur Massenunterdrückung von Warnungen, mit denen Sie PVS-Studio auch in einem großen alten Projekt schnell verwenden können. Dies ist eine Benachrichtigungsverteilung. Und so weiter und so fort. Es ist jedoch immer noch die erste Frage, die gestellt wird: "Kann PVS-Studio etwas finden, das Compiler nicht finden können?" Dies bedeutet, dass wir immer wieder Artikel über die Überprüfung dieser Compiler selbst schreiben werden.
Kommen wir zurück zum Thema der Überprüfung des Clang-Projekts. Es ist nicht nötig, sich mit diesem Projekt zu befassen und zu sagen, was es ist. Tatsächlich wurde nicht nur der Code von Clang 11 selbst getestet, sondern auch die LLVM 11-Bibliothek, auf der er basiert. Aus Sicht dieses Artikels spielt es keine Rolle, ob ein Fehler im Compiler oder im Bibliothekscode gefunden wird.
Der Clang / LLVM-Code schien mir viel klarer als der GCC-Code. Zumindest fehlen all diese schrecklichen Makros, und die modernen Funktionen der C ++ - Sprache werden aktiv genutzt.
Trotzdem ist das Projekt groß und ohne vorläufige Einstellungen des Analysators ist es immer noch sehr mühsam, den Bericht anzuzeigen. Grundsätzlich stören halb falsch positive Ergebnisse. Mit halb falsch positiven Ergebnissen meine ich Situationen, in denen der Analysator formal richtig ist, aber es keinen Sinn macht, zu warnen. Beispielsweise werden viele solcher Positiven für Komponententests und generierten Code ausgegeben.
Beispiel für Tests:
Spaces.SpacesInParentheses = false; // <=
Spaces.SpacesInCStyleCastParentheses = true; // <=
verifyFormat("Type *A = ( Type * )P;", Spaces);
verifyFormat("Type *A = ( vector<Type *, int *> )P;", Spaces);
verifyFormat("x = ( int32 )y;", Spaces);
verifyFormat("int a = ( int )(2.0f);", Spaces);
verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
verifyFormat("#define x (( int )-1)", Spaces);
// Run the first set of tests again with:
Spaces.SpacesInParentheses = false; // <=
Spaces.SpaceInEmptyParentheses = true;
Spaces.SpacesInCStyleCastParentheses = true; // <=
verifyFormat("call(x, y, z);", Spaces);
verifyFormat("call( );", Spaces);Der Analysator warnt, dass den Variablen dieselben Werte zugewiesen werden, die sie bereits enthalten:
- V1048 Der Variablen 'Spaces.SpacesInParentheses' wurde der gleiche Wert zugewiesen. FormatTest.cpp 11554
- V1048 Der Variablen 'Spaces.SpacesInCStyleCastParentheses' wurde der gleiche Wert zugewiesen. FormatTest.cpp 11556
Formal hat der Analysator die richtige Antwort zurückgegeben, und dies ist das Codefragment, das vereinfacht oder korrigiert werden sollte. Gleichzeitig ist klar, dass tatsächlich alles in Ordnung ist und es auch keinen Sinn macht, etwas zu bearbeiten.
Ein weiteres Beispiel: Der Analysator gibt eine große Anzahl von Warnungen an die automatisch generierte Datei Options.inc aus. Sie können das "Codeblatt" in der Datei sehen:
Und PVS-Studio sendet Warnungen an all dies:
- V501 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: nullptr == nullptr Options.inc 26
- V501 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: nullptr == nullptr Options.inc 27
- V501 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: nullptr == nullptr Options.inc 28
- und so weiter für jede Zeile ...
Das alles ist nicht beängstigend. All dies kann besiegt werden: Deaktivieren Sie die Überprüfung unnötiger Dateien, markieren Sie einige Makros und Funktionen, unterdrücken Sie bestimmte Arten von Alarmen und so weiter. Es ist möglich, aber dies als Teil der Aufgabe des Schreibens eines Artikels zu tun, ist nicht interessant. Daher habe ich genau das Gleiche getan wie im Artikel über den GCC-Compiler. Ich habe den Bericht studiert, bis ich 11 interessante Codebeispiele hatte, um einen Artikel zu schreiben. Warum 11? Ich dachte, da die Clang-Version 11 ist, dann lassen Sie die Fragmente 11 sein :).
11 verdächtige Codefragmente
Fragment N1, Modulo-Division durch eins
Cooler Fehler! Ich liebe diese!
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);
}
....
}PVS-Studio-Warnung: V1063 Die Modulo-by-1-Operation ist bedeutungslos. Das Ergebnis ist immer Null. llvm-stress.cpp 631 Die Modulo-
Division wird verwendet, um einen zufälligen Wert von 0 oder 1 zu erhalten. Aber anscheinend ist dieser Wert von 1 verwirrend, und die Leute machen das klassische Fehlermuster, indem sie durch eins teilen, obwohl es notwendig ist, durch zwei zu teilen. Die Operation X% 1 ist bedeutungslos, da das Ergebnis immer 0 ist . Richtige Version des Codes:
if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2)) {Diagnostics V1063, das kürzlich in PVS-Studio veröffentlicht wurde, ist unglaublich einfach, aber wie Sie sehen, funktioniert es.
Wie wir wissen, schauen sich Compiler-Entwickler an, was wir tun, und leihen unsere Best Practices aus. Daran ist nichts auszusetzen. Wir freuen uns, dass PVS-Studio der Motor des Fortschritts ist . Wir legen die Zeit fest, nach der dieselbe Diagnose in Clang und GCC angezeigt wird :).
Fragment N2, Tippfehler in der Bedingung
class ReturnValueSlot {
....
bool isNull() const { return !Addr.isValid(); }
....
};
static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
const FunctionDecl *F2, unsigned NumParams) {
....
unsigned I1 = 0, I2 = 0;
for (unsigned I = 0; I != NumParams; ++I) {
QualType T1 = NextParam(F1, I1, I == 0);
QualType T2 = NextParam(F2, I2, I == 0);
if (!T1.isNull() && !T1.isNull() && !Context.hasSameUnqualifiedType(T1, T2))
return false;
}
return true;
}PVS-Studio-Warnung: V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke :! T1.isNull () &&! T1.isNull () SemaOverload.cpp 9493
Zweimal prüfen! T1.isNull () . Dies ist ein offensichtlicher Tippfehler, und der zweite Teil der Bedingung sollte die T2- Variable überprüfen .
Fragment N3, Potential außerhalb der Grenzen des Arrays
std::vector<Decl *> DeclsLoaded;
SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
....
unsigned Index = ID - NUM_PREDEF_DECL_IDS;
if (Index > DeclsLoaded.size()) {
Error("declaration ID out-of-range for AST file");
return SourceLocation();
}
if (Decl *D = DeclsLoaded[Index])
return D->getLocation();
....
}PVS-Studio-Warnung: V557-Array-Überlauf ist möglich. Der 'Index'-Index zeigt über die Array-Grenze hinaus. ASTReader.cpp 7318
Angenommen das Array ein Element enthält, und das Index - Variable ist auch einer. Die Bedingung (1> 1) ist falsch, und als Ergebnis wird das Array überlaufen. Richtige Prüfung:
if (Index >= DeclsLoaded.size()) {Fragment N4, die Reihenfolge der Bewertung von Argumenten
void IHexELFBuilder::addDataSections() {
....
uint32_t SecNo = 1;
....
Section = &Obj->addSection<OwnedDataSection>(
".sec" + std::to_string(SecNo++), RecAddr,
ELF::SHF_ALLOC | ELF::SHF_WRITE, SecNo);
....
}PVS-Studio-Warnung: V567 Nicht angegebenes Verhalten. Die Reihenfolge der Argumentauswertung ist für die Funktion 'addSection' nicht definiert. Überprüfen Sie die Variable 'SecNo'. Object.cpp 1223
Beachten Sie, dass das SecNo- Argument zweimal verwendet und dabei erhöht wird. Es ist einfach unmöglich zu sagen, in welcher Reihenfolge die Argumente ausgewertet werden. Daher hängt das Ergebnis von der Compilerversion oder den Kompilierungseinstellungen ab.
Lassen Sie mich dies anhand eines synthetischen Beispiels erklären:
#include <cstdio>
int main()
{
int i = 1;
printf("%d, %d\n", i, i++);
return 0;
}Je nach Compiler können sowohl "1, 2" als auch "2, 1" gedruckt werden. Mit dem Compiler Explorer erhalte ich folgende Ergebnisse:
- Ein mit Clang 11.0.0 kompiliertes Programm erzeugt : 1, 1.
- Ein mit GCC 10.2 kompiliertes Programm erzeugt : 2, 1.
Interessanterweise gibt der Clang-Compiler für diesen einfachen Fall eine Warnung aus:
<source>:6:26: warning:
unsequenced modification and access to 'i' [-Wunsequenced]
printf("%d, %d\n", i, i++);Anscheinend hat diese Warnung unter realen Bedingungen nicht geholfen. Die Diagnose ist entweder deaktiviert, da dies für den praktischen Gebrauch unpraktisch ist, oder der Compiler konnte nicht vor einem komplexeren Fall warnen.
Fragment N5, seltsamer Wiederholungstest
template <class ELFT>
void GNUStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
const Elf_Shdr *Sec) {
....
Expected<StringRef> NameOrErr =
this->dumper()->getSymbolVersionByIndex(Ndx, IsDefault);
if (!NameOrErr) {
if (!NameOrErr) {
unsigned SecNdx = Sec - &cantFail(Obj->sections()).front();
this->reportUniqueWarning(createError(
"unable to get a version for entry " + Twine(I) +
" of SHT_GNU_versym section with index " + Twine(SecNdx) + ": " +
toString(NameOrErr.takeError())));
}
Versions.emplace_back("<corrupt>");
continue;
}
....
}PVS-Studio-Warnung: V571 Wiederkehrende Prüfung. Die Bedingung 'if (! NameOrErr)' wurde bereits in Zeile 4666 überprüft. ELFDumper.cpp 4667
Die zweite Prüfung dupliziert die erste und ist redundant. Vielleicht kann der zweite Scheck einfach entfernt werden. Dies ist jedoch höchstwahrscheinlich ein Tippfehler, und in der zweiten Bedingung sollte eine andere Variable verwendet werden.
Snippet N6, der einen potenziell Nullzeiger dereferenziert
void RewriteObjCFragileABI::RewriteObjCClassMetaData(
ObjCImplementationDecl *IDecl, std::string &Result)
{
ObjCInterfaceDecl *CDecl = IDecl->getClassInterface();
if (CDecl->isImplicitInterfaceDecl()) {
RewriteObjCInternalStruct(CDecl, Result);
}
unsigned NumIvars = !IDecl->ivar_empty()
? IDecl->ivar_size()
: (CDecl ? CDecl->ivar_size() : 0);
....
}PVS-Studio-Warnung: V595 Der Zeiger 'CDecl' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Prüfzeilen: 5275, 5284. RewriteObjC.cpp 5275
Bei der ersten Prüfung wird der CDecl- Zeiger immer fett dereferenziert:
if (CDecl->isImplicitInterfaceDecl())Und nur aus dem unten geschriebenen Code wird klar, dass dieser Zeiger null sein kann:
(CDecl ? CDecl->ivar_size() : 0)Höchstwahrscheinlich sollte die erste Überprüfung folgendermaßen aussehen:
if (CDecl && CDecl->isImplicitInterfaceDecl())Snippet N7, der einen potenziell Nullzeiger dereferenziert
bool
Sema::InstantiateClass(....)
{
....
NamedDecl *ND = dyn_cast<NamedDecl>(I->NewDecl);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext());
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember());
....
}PVS-Studio-Warnung: V595 Der Zeiger 'ND' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 2803, 2805. SemaTemplateInstantiate.cpp 2803
Eine Variation des vorherigen Fehlers. Es ist gefährlich, einen Zeiger zu dereferenzieren, ohne ihn vorher zu überprüfen, ob sein Wert mithilfe einer dynamischen Umwandlung erhalten wird. Darüber hinaus können Sie dem folgenden Code entnehmen, dass eine solche Prüfung erforderlich ist.
Fragment N8 setzt die Funktion die Ausführung trotz eines Fehlerzustands fort
bool VerifyObject(llvm::yaml::Node &N,
std::map<std::string, std::string> Expected) {
....
auto *V = llvm::dyn_cast_or_null<llvm::yaml::ScalarNode>(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
}
std::string VS = V->getValue(Tmp).str();
....
}PVS-Studio-Warnung: V1004 Der 'V'-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 61, 65. TraceTests.cpp 65
Zeiger V kann null sein. Dies ist eindeutig ein fehlerhafter Zustand, über den sogar berichtet wird. Danach setzt die Funktion die Ausführung jedoch so fort, als wäre nichts passiert, was zur Dereferenzierung dieses Nullzeigers führt. Vielleicht haben sie vergessen, die Funktion zu unterbrechen, und die richtige Option sollte folgendermaßen aussehen:
auto *V = llvm::dyn_cast_or_null<llvm::yaml::ScalarNode>(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
return false;
}
std::string VS = V->getValue(Tmp).str();Snippet N9, Tippfehler
const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
const InputInfo &Output) {
if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
if (StringRef(A->getValue()) == "single")
return Args.MakeArgString(Output.getFilename());
Arg *FinalOutput = Args.getLastArg(options::OPT_o);
if (FinalOutput && Args.hasArg(options::OPT_c)) {
SmallString<128> T(FinalOutput->getValue());
llvm::sys::path::replace_extension(T, "dwo");
return Args.MakeArgString(T);
} else {
// Use the compilation dir.
SmallString<128> T(
Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
llvm::sys::path::replace_extension(F, "dwo");
T += F;
return Args.MakeArgString(F); // <=
}
}PVS-Studio-Warnung: V1001 Die Variable 'T' wird zugewiesen, aber am Ende der Funktion nicht verwendet. CommonArgs.cpp 873
Beachten Sie das Ende der Funktion. Die lokale Variable T wird geändert, aber in keiner Weise verwendet. Dies ist höchstwahrscheinlich ein Tippfehler und die Funktion sollte mit den folgenden Codezeilen enden:
T += F;
return Args.MakeArgString(T);Fragment N10, Divisor ist Null
typedef int32_t si_int;
typedef uint32_t su_int;
typedef union {
du_int all;
struct {
#if _YUGA_LITTLE_ENDIAN
su_int low;
su_int high;
#else
su_int high;
su_int low;
#endif // _YUGA_LITTLE_ENDIAN
} s;
} udwords;
COMPILER_RT_ABI du_int __udivmoddi4(du_int a, du_int b, du_int *rem) {
....
if (d.s.low == 0) {
if (d.s.high == 0) {
// K X
// ---
// 0 0
if (rem)
*rem = n.s.high % d.s.low;
return n.s.high / d.s.low;
}
....
}PVS-Studio-Warnungen:
- V609 Mod um Null. Nenner 'dslow' == 0.udivmoddi4.c 61
- V609 Durch Null teilen. Nenner 'dslow' == 0.udivmoddi4.c 62
Ich weiß nicht, ob dies ein Fehler oder eine knifflige Idee ist, aber der Code ist sehr seltsam. Es gibt zwei gewöhnliche ganzzahlige Variablen, und eine ist durch die andere teilbar. Interessanterweise geschieht dies nur, wenn beide Variablen Null sind. Was bedeutet das alles?
Fragment N11, Kopieren-Einfügen
bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(....)
{
....
StringRef FName = II->getName();
....
if (FName == "postEvent" &&
FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
return true;
}
if (FName == "postEvent" &&
FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
return true;
}
....
}PVS-Studio-Warnung: V581 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 3108, 3113. MallocChecker.cpp 3113
Das Code-Snippet wurde kopiert, aber in keiner Weise geändert. Das zweite Snippet sollte entweder entfernt oder geändert werden, um eine nützliche Überprüfung durchzuführen.
Fazit
Ich möchte Sie daran erinnern, dass Sie diese kostenlose Lizenzoption verwenden können , um Open Source-Projekte zu überprüfen . Übrigens gibt es andere Möglichkeiten für die kostenlose Lizenzierung von PVS-Studio, auch für geschlossene Projekte. Sie sind hier aufgelistet: " Kostenlose PVS-Studio-Lizenzoptionen ". Vielen Dank für Ihre Aufmerksamkeit.
Unsere anderen Artikel zur Compilerprüfung
- LLVM (Clang) -Analyse (August 2011), zweite Analyse (August 2012), dritte Analyse (Oktober 2016), vierte Analyse (April 2019)
- GCC-Analyse (August 2016), zweite Analyse (April 2020)
- Huawei Ark Compiler-Analyse (Dezember 2019)
- Analyse der .NET Compiler Platform ("Roslyn") (Dezember 2015), zweite Analyse (April 2019)
- Roslyn Analyzer-Analyse (August 2019)
- PascalABC.NET-Analyse (März 2017)
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Andrey Karpov. Überprüfen von Clang 11 mit PVS-Studio .