Kontrol af Clang 11 med PVS-Studio

(28. oktober 2020 )

Nu og da skal vi skrive artikler om, hvordan vi har tjekket en anden ny version af en eller anden compiler. Det er ikke rigtig sjovt. Men som praksis viser, hvis vi holder op med at gøre det i et stykke tid, begynder folk at tvivle på, om PVS-Studio er værd at titlen er en god fanger af bugs og sårbarheder. Hvad hvis den nye kompilator også kan gøre det? Sikker på, kompilatorer udvikler sig, men det gør PVS-Studio også – og det viser igen og igen dets evne til at fange bugs selv i højkvalitetsprojekter som compilers.

Tid til kontrol af Clang igen

For at fortælle dig sandheden skrev jeg denne artikelbaseret på det tidligere indlæg “ Kontrol af GCC 10-kompilatoren med PVS-Studio ”. Så hvis nogle afsnit virker velkendte, skyldes det, at du allerede har læst dem før :).

Det er ingen hemmelighed, at kompilatorer anvender deres egne indbyggede analysatorer for statisk kode, og de udvikler sig også. Derfor skriver vi artikler ind imellem for at vise, at vores statiske analysator, PVS-Studio, kan finde fejl selv inde i kompilatorer, og at vi er vores salt værd :).

Faktisk kan du ikke sammenlign klassiske statiske analysatorer med compilere. Statiske analysatorer registrerer ikke kun fejl i kildekoden, men involverer også en højt udviklet infrastruktur. For det første inkluderer det integration med sådanne systemer som SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins og Visual Studio. Det inkluderer mekanismer til massedæmpning af advarsler, som giver dig mulighed for at begynde at bruge PVS-Studio med det samme selv i et stort projekt. Det inkluderer afsendelse af meddelelser via e-mail. Og så videre og så videre. Men det første spørgsmål, som udviklere stadig vil stille, er: “Kan dit PVS-Studio finde noget, som kompilatorer ikke kan?” Og det betyder, at vi er dømt til at skrive artikler om, hvordan vi kontrollerer kompilatorerne selv igen og igen.

Lad os vende tilbage til Clang. Det er ikke nødvendigt at dvæle ved emnet og fortælle dig, hvad projektet handler om. Faktisk kontrollerede vi ikke kun koden til selve Clang 11, men også koden til LLVM 11-biblioteket, den er baseret på. Set fra denne artikels synspunkt betyder det ikke noget, om der blev fundet en defekt i kompilatorens eller bibliotekets kode.

Jeg fandt koden til Clang / LLVM meget tydeligere end den for GCC. I det mindste vrimler det ikke af alle disse forfærdelige makroer, og det bruger i vid udstrækning C ++ s moderne funktioner.

Alligevel er projektet stadig stort nok til at gøre undersøgelsen af ​​analyserapporten kedelig uden forudgående tilpasning. Det, der for det meste kommer i vejen, er “semi-falske” positive. Med “semi-falske” positive mener jeg tilfælde, hvor analysatoren er teknisk korrekt til at påpege visse problemer, men disse advarsler er uden praktisk brug. For eksempel henviser mange sådanne advarsler til enhedstest og genereret kode.

Her er et eksempel på enhedstest:

Spaces.SpacesInParentheses = false; // <=
Spaces.SpacesInCStyleCastParentheses = true; // <=
verifyFormat("Type *A = ( Type * )P;", Spaces);
verifyFormat("Type *A = ( vector )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);

analysator advarer os om, at variablerne tildeles de samme værdier, som de allerede har:

  • V1048 Variablen Spaces.SpacesInParentheses fik den samme værdi. FormatTest.cpp 11554
  • V1048 Variablen Spaces.SpacesInCStyleCastParentheses fik den samme værdi. FormatTest.cpp 11556

Denne advarsel er teknisk set til det punkt, og uddraget skal forenkles eller rettes. Men det er også klart, at denne kode er fin, som den er, og der er ingen mening med at rette noget i den.

Her er et andet eksempel: analysatoren udsender masser af advarsler på den autogenererede fil Options.inc. Se på “væg” af kode, den indeholder:

Denne hovedpart af koden udløser en strøm af advarsler:

  • V501 Der er identiske underudtryk til venstre og til højre for operatoren ==: nullptr == nullptr Options.inc 26
  • V501 Der er identiske underudtryk til venstre og til højre for operatoren ==: nullptr == nullptr Options.inc 27
  • V501 Der er identiske underudtryk til venstre og til højre for operatoren ==: nullptr == nullptr Options.inc 28
  • og så videre – en advarsel pr. linje …

Alligevel er alt dette ikke en big deal. Det kan løses ved at ekskludere irrelevante filer fra analyse, markere bestemte makroer og funktioner, undertrykke visse diagnostiske typer osv. Ja, det kan det, men det er ikke et meget interessant job at udføre, når du skriver en artikel. Derfor gjorde jeg det samme som i artiklen om kontrol af GCC-kompilatoren: Jeg fortsatte med at læse rapporten, indtil jeg samlede 11 interessante eksempler til at medtage i artiklen. Hvorfor 11? Jeg troede bare, at da det var den 11. version af Clang, havde jeg brug for 11 eksempler :).

11 mistænkelige kodestykker

Stykke 1, moduloperation på 1

Dette er sejt! Jeg kan lide bugs sådan!

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(Val0->getType()) && (getRandom() % 1)) {
unsigned NumElem =
cast(Val0->getType())->getNumElements();
CondTy = FixedVectorType::get(CondTy, NumElem);
}
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V1063 Den modulo ved 1 operation er meningsløs. Resultatet vil altid være nul. llvm-stress.cpp 631

Programmøren bruger en modulo-operation for at få en tilfældig værdi på enten 0 eller 1. Men værdien 1 synes at forvirre udviklere og få dem til at skrive det klassiske antimønster, hvor modulo-operationen udføres på 1 i stedet for 2. X% 1 operationen er meningsløs, da den altid evalueres til 0 . Dette er den faste version:

if (isa(Val0->getType()) && (getRandom() % 2)) {

Den nyligt tilføjede V1063-diagnostik er frygtelig enkel, men som du kan se, fungerer den perfekt.

Vi ved, at kompilatorudviklere holder øje med vores arbejde og låner vores ideer. Det er helt fint. Det er rart at vide, at PVS-Studio er drivkraften bag fremskridt . Lad os se, hvor meget det vil tage for en lignende diagnostik at blive vist i Clang og GCC :).

Stykke 2, en tastefejl i en tilstand

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;
}

Diagnosticeringsmeddelelse for PVS-Studio: V501 Der er identiske underudtryk til venstre og til højre for & & operatør:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Kontrollen ! T1.isNull () udføres to gange. Dette er naturligvis en skrivefejl; den anden del af betingelsen skal kontrollere variablen T2 .

Stykke 3, potentiel array-index-out-of- grænser

std::vector 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();
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V557 Matrixoverskridelse er mulig. Indekset Indeks peger ud over array-bundet. ASTReader.cpp 7318

Antag, at arrayet gemmer et element, og værdien af ​​ Index -variablen er også 1. Derefter er (1> 1) -tilstanden er falsk, og derfor vil arrayet blive indekseret uden for dets grænser. Her er den korrekte kontrol:

if (Index >= DeclsLoaded.size()) {

Stykke 4, argumentevalueringsrækkefølge

void IHexELFBuilder::addDataSections() {
....
uint32_t SecNo = 1;
....
Section = &Obj->addSection(
".sec" + std::to_string(SecNo++), RecAddr,
ELF::SHF_ALLOC | ELF::SHF_WRITE, SecNo);
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V567 Uspecificeret adfærd. Rækkefølgen af ​​argumentevaluering er ikke defineret for funktionen addSection. Overvej at inspicere SecNo variablen. Object.cpp 1223

Bemærk, at argumentet SecNo bruges to gange, mens det i mellemtiden øges. Problemet er, at du ikke kan fortælle i hvilken nøjagtig rækkefølge argumenterne evalueres. Resultatet vil derfor variere afhængigt af compilerversionen eller kompileringsparametrene.

Her er et syntetisk eksempel for at illustrere dette punkt:

#include 
int main()
{
int i = 1;
printf("%d, %d\n", i, i++);
return 0;
}

Afhængig af kompilatoren kan denne kode enten udsende “1, 2” eller “2, 1”. Jeg kørte det på Compiler Explorer og fik følgende output:

  • når det blev kompileret med Clang 11.0.0, udgav programmet 1 , 1.
  • når det kompileres med GCC 10.2, er programmet output s 2, 1.

Interessant nok får denne enkle sag Clang til at udstede en advarsel:

:6:26: warning:
unsequenced modification and access to "i" [-Wunsequenced]
printf("%d, %d\n", i, i++);

Af en eller anden grund blev denne advarsel dog ikke udstedt på den rigtige kode. Enten er den deaktiveret som en ikke særlig praktisk sag, eller så er sagen for kompliceret til, at compileren kan klare.

Stykke 5, en mærkelig duplikatkontrol

template <class ELFT>
void GNUStyle::printVersionSymbolSection(const ELFFile *Obj,
const Elf_Shdr *Sec) { ....
Expected 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("");
continue;
}
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V571 Tilbagevendende kontrol. Betingelsen if (! NameOrErr) var allerede verificeret i linje 4666. ELFDumper.cpp 4667

Den anden kontrol er en klon af den første og er derfor overflødig. Måske kunne det fjernes sikkert. Men hvad der er mere sandsynligt er, at det indeholder en skrivefejl og var beregnet til at kontrollere en anden variabel.

Stykke 6, potentiel nul pointerdifferens

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);
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V595 CDecl -markøren blev brugt, før den blev bekræftet mod nullptr. Kontroller linjer: 5275, 5284. RewriteObjC.cpp 5275

Når den første kontrol udføres, tøver udvikleren aldrig med at omlægge CDecl markøren:

if (CDecl->isImplicitInterfaceDecl())

Men hvis du ser på koden et par linjer længere, bliver det klart, at markøren kan være nul:

(CDecl ? CDecl->ivar_size() : 0)

Den første kontrol skulle sandsynligvis se sådan ud:

if (CDecl && CDecl->isImplicitInterfaceDecl())

Stykke 7, potentiel null pointerdifferens

bool
Sema::InstantiateClass(....)
{
....
NamedDecl *ND = dyn_cast(I->NewDecl);
CXXRecordDecl *ThisContext =
dyn_cast_or_null(ND->getDeclContext());
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember());
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V595 ND -markøren blev brugt, før den blev bekræftet mod nullptr.Kontroller linjer: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Denne fejl svarer til den forrige. Det er farligt at afvige en markør uden forudgående kontrol, når dens værdi opnås ved hjælp af en cast af dynamisk type. Mere end det, bekræfter efterfølgende kode, at en sådan kontrol er nødvendig.

Stykke 8, en funktion, der fortsætter med at køre på trods af en fejltilstand

bool VerifyObject(llvm::yaml::Node &N,
std::map Expected) {
....
auto *V = llvm::dyn_cast_or_null(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
}
std::string VS = V->getValue(Tmp).str();
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V1004 V -markøren blev brugt usikkert, efter at den var verificeret mod nullptr. Kontroller linjer: 61, 65. TraceTests.cpp 65

V -markøren kan være en nul-markør. Dette er naturligvis en fejltilstand, som endda rapporteres med en fejlmeddelelse. Men funktionen vil bare fortsætte med at køre, som om der ikke skete noget, og ender med at henvise den meget nul pointer. Programmøren ville sandsynligvis have funktionen til at stoppe på dette tidspunkt, i hvilket tilfælde den skulle rettes som følger:

auto *V = llvm::dyn_cast_or_null(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
return false;
}
std::string VS = V->getValue(Tmp).str();

Stykke 9, en skrivefejl

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); // <=
}
}

Diagnosticeringsmeddelelse for PVS-Studio: V1001 T variabel tildeles, men bruges ikke i slutningen af ​​funktionen. CommonArgs.cpp 873

Se på de sidste linjer i funktionen. Den lokale variabel T ændres, men bruges ikke på nogen måde. Dette skal være en skrivefejl, og funktionen skal sandsynligvis slutte som følger:

T += F;
return Args.MakeArgString(T);

Stykke 10, nul som deleren

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;
}
....
}

Diagnostiske meddelelser fra PVS-Studio:

  • V609 Mod med nul. Nævneren ‘d.s.low’ == 0. udivmoddi4.c 61
  • V609 Del med nul. Nævneren ‘d.s.low’ == 0. udivmoddi4.c 62

Jeg ved ikke, om dette er en fejl eller en eller anden vanskelig kontraktion, men koden ser mærkelig ud. Det har to regelmæssige heltalsvariabler, hvoraf den ene er divideret med den anden. Men den interessante del er, at opdeling kun finder sted, hvis begge variabler er nuller. Hvilken opgave skal den udføre?

Stykke 11, kopier-indsæt

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;
}
....
}

Diagnosticeringsmeddelelse for PVS-Studio: V581 De betingede udtryk for if -sætningerne placeret ved siden af ​​hinanden er identiske. Kontroller linjer: 3108, 3113. MallocChecker.cpp 3113

Et kodefragment blev klonet, men blev aldrig ændret bagefter. Denne klon skal enten fjernes eller ændres for at udføre en nyttig kontrol.

Konklusion

Husk at du kan bruge denne gratis licens mulighed for at kontrollere open source-projekter. Vi tilbyder også andre måder at bruge PVS-Studio gratis på, nogle af dem tillader endda analyse af proprietær kode. Se den fulde liste over muligheder her: “ Måder at få en gratis PVS-Studio-licens ”. Tak fordi du læste!

Skriv et svar

Din e-mailadresse vil ikke blive publiceret. Krævede felter er markeret med *