Kontrollerer Clang 11 med PVS-Studio

(28. oktober 2020 )

Nå og da må vi skrive artikler om hvordan vi har sjekket en annen fersk versjon av noen kompilator. Det er ikke veldig gøy. Men som praksis viser, hvis vi slutter å gjøre det en stund, begynner folk å tvile på om PVS-Studio er verdt tittelen som en god fanger av feil og sårbarheter. Hva om den nye kompilatoren kan gjøre det også? Visst, kompilatorer utvikler seg, men det gjør også PVS-Studio – og det viser igjen og igjen evnen til å fange feil selv i høykvalitetsprosjekter som kompilatorer.

Tid for å sjekke om Clang

For å fortelle deg sannheten, skrev jeg denne artikkelen basert på det tidligere innlegget “ Kontrollerer GCC 10-kompilatoren med PVS-Studio ”. Så hvis noen avsnitt virker kjent, er det fordi du allerede har lest dem før :).

Det er ingen hemmelighet at kompilatorene bruker sine egne innebygde analysatorer for statisk kode, og de utvikler seg også. Derfor skriver vi artikler innimellom for å vise at den statiske analysatoren vår, PVS-Studio, kan finne feil selv i kompilatorer, og at vi er verdt saltet vårt :).

Faktisk kan du ikke sammenligne klassiske statiske analysatorer med kompilatorer. Statiske analysatorer oppdager ikke bare feil i kildekoden, men involverer også en høyt utviklet infrastruktur. For det første inkluderer den integrasjon med slike systemer som SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins og Visual Studio. Den inkluderer mekanismer for massedemping av advarsler, som lar deg begynne å bruke PVS-Studio med en gang selv i et stort prosjekt. Det inkluderer å sende varsler via e-post. Og så videre. Men det første spørsmålet utviklere fortsatt vil stille er: «Kan PVS-Studio finne noe som kompilatorer ikke kan?» Og det betyr at vi er dømt til å skrive artikler om hvordan vi sjekker kompilatorene selv om og om igjen.

La oss komme tilbake til Clang. Det er ikke nødvendig å dvele ved emnet og fortelle deg hva prosjektet handler om. Vi sjekket faktisk ikke bare koden til selve Clang 11, men også koden til LLVM 11-biblioteket den er basert på. Fra denne artikkels synspunkt spiller det ingen rolle om en feil ble funnet i kompilatorens eller bibliotekets kode.

Jeg fant koden til Clang / LLVM mye tydeligere enn GCC. I det minste vrimler det ikke av alle de forferdelige makroene, og det bruker i stor grad C ++ s moderne funksjoner.

Likevel er prosjektet fortsatt stort nok til å gjøre det mulig å undersøke analyserapporten kjedelig uten forutgående tilpasning. Det som for det meste kommer i veien er «semi-falske» positive. Med «semi-falske» positiver mener jeg tilfeller der analysatoren er teknisk korrekt for å påpeke visse problemer, men disse advarslene ikke har noen praktisk bruk. Mange slike advarsler refererer for eksempel til enhetstester og generert kode.

Her er et eksempel på enhetstester:

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 oss om at variablene tildeles de samme verdiene de allerede har:

  • V1048 Variabelen Spaces.SpacesInParentheses ble tildelt samme verdi. FormatTest.cpp 11554
  • V1048 Varianten ‘Spaces.SpacesInCStyleCastParentheses’ ble tildelt samme verdi. FormatTest.cpp 11556

Teknisk sett er denne advarselen til punktet, og kodebiten må forenkles eller repareres. Men det er også klart at denne koden er fin som den er, og det er ikke noe poeng å fikse noe i den.

Her er et annet eksempel: analysatoren sender ut massevis av advarsler på den autogenererte filen Options.inc. Se på «veggen» til koden den inneholder:

Denne hoveddelen av koden utløser en flom av advarsler:

  • V501 Det er identiske underuttrykk til venstre og til høyre for operatøren ==: nullptr == nullptr Options.inc 26
  • V501 Det er identiske underuttrykk til venstre og til høyre for operatoren ==: nullptr == nullptr Options.inc 27
  • V501 Der er identiske underuttrykk til venstre og høyre for operatøren ==: nullptr == nullptr Options.inc 28
  • og så videre – en advarsel per linje …

Likevel er alt dette ikke så farlig. Det kan løses ved å ekskludere irrelevante filer fra analyse, markere visse makroer og funksjoner, undertrykke visse diagnostiske typer og så videre. Ja, det kan det, men det er ikke en veldig interessant jobb å gjøre når du skriver en artikkel. Derfor gjorde jeg det samme som i artikkelen om å sjekke GCC-kompilatoren: Jeg fortsatte å lese rapporten til jeg samlet 11 interessante eksempler å inkludere i artikkelen. Hvorfor 11? Jeg trodde bare at siden det var den 11. versjonen av Clang, trengte jeg 11 eksempler :).

11 mistenkelige kodebiter

Utdrag 1, moduloperasjon på 1

Dette er kult! Jeg liker slike feil!

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

Diagnosemelding for PVS-Studio: V1063 Modulen ved 1 operasjon er meningsløs. Resultatet vil alltid være null. llvm-stress.cpp 631

Programmereren bruker en moduloperasjon for å få en tilfeldig verdi på enten 0 eller 1. Men verdien 1 ser ut til å forvirre utviklere og få dem til å skrive det klassiske antimønsteret der modulooperasjonen utføres på 1 i stedet for 2. X% 1 operasjonen er meningsløs da den alltid evalueres til 0 . Dette er den faste versjonen:

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

Den nylig tilføyde V1063-diagnosen er veldig enkel, men som du ser, fungerer den perfekt.

Vi vet at kompilatorutviklere holder øye med arbeidet vårt og låner ideene våre. Det er helt greit. Det er hyggelig å vite at PVS-Studio er drivkraften bak fremgang . La oss se hvor mye det tar før en lignende diagnostikk vises i Clang og GCC :).

Utdrag 2, en skrivefeil 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;
}

Diagnosemelding for PVS-Studio: V501 Det er identiske underuttrykk til venstre og til høyre for & & operator:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Kontrollen ! T1.isNull () utføres to ganger. Dette er åpenbart en skrivefeil; den andre delen av tilstanden må sjekke varianten T2 .

Utdrag 3, potensiell array-indeks-ut-av- grenser

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

Diagnostisk melding for PVS-Studio: V557 Overrasking av matrisen er mulig. Indeksen peker utover array-bound. ASTReader.cpp 7318

Anta at matrisen lagrer ett element og verdien av indeks variabelen også er 1. Da er (1> 1) tilstanden er falsk, og derfor vil matrisen indekseres utenfor grensene. Her er riktig kontroll:

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

Utdrag 4, argumentevalueringsrekkefølge

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

PVS-Studio diagnostisk melding: V567 Uspesifisert oppførsel. Rekkefølgen av argumentevaluering er ikke definert for funksjonen ‘addSection’. Vurder å inspisere variabelen ‘SecNo’. Object.cpp 1223

Vær oppmerksom på at argumentet SecNo brukes to ganger, og blir steget i mellomtiden. Problemet er at du ikke kan fortelle i nøyaktig rekkefølge argumentene vil bli evaluert. Resultatet vil derfor variere avhengig av kompilatorversjonen eller kompileringsparametrene.

Her er et syntetisk eksempel for å illustrere dette punktet:

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

Avhengig av kompilatoren, kan denne koden sende ut enten “1, 2” eller “2, 1”. Jeg kjørte den på Compiler Explorer og fikk følgende utganger:

  • når den ble kompilert med Clang 11.0.0, utgav programmet 1 , 1.
  • når kompilert med GCC 10.2, programmet output s 2, 1.

Interessant nok får denne enkle saken Clang til å advare:

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

Av en eller annen grunn ble denne advarselen imidlertid ikke gitt på den virkelige koden. Enten er den deaktivert som ikke veldig praktisk, eller så er saken for komplisert for kompilatoren å takle.

Utdrag 5, en merkelig duplikatkontroll

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

Diagnostisk melding for PVS-Studio: V571 Gjentatt sjekk. ‘Hvis (! NameOrErr)’ -tilstanden var allerede bekreftet i linje 4666. ELFDumper.cpp 4667

Den andre sjekken er en klone av den første og er derfor overflødig. Kanskje det kunne fjernes trygt. Men det som er mer sannsynlig er at den inneholder en skrivefeil og var ment å sjekke noen annen variabel.

Utdrag 6, potensiell nullpekereferanse

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

Diagnosemelding for PVS-Studio: V595 CDecl -pekeren ble brukt før den ble bekreftet mot nullptr. Sjekk linjer: 5275, 5284. RewriteObjC.cpp 5275

Når den første kontrollen utføres, nøler utvikleren aldri med å henvise CDecl -pekeren:

if (CDecl->isImplicitInterfaceDecl())

Men hvis du ser på koden noen linjer lenger, blir det klart at pekeren kan være null:

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

Den første sjekken var sannsynligvis ment å se slik ut:

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

Utdrag 7, potensiell nullpeker

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

PVS-Studio diagnostisk melding: V595 ND -pekeren ble brukt før den ble bekreftet mot nullptr.Sjekk linjer: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Denne feilen ligner den forrige. Det er farlig å henvise til en peker uten forhåndskontroll når verdien ervervet ved hjelp av en dynamisk rollebesetning. Mer enn det, bekrefter påfølgende kode at en slik sjekk er nødvendig.

Utdrag 8, en funksjon som fortsetter å kjøre til tross for en feiltilstand

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

Diagnosemelding for PVS-Studio: V1004 V-pekeren ble brukt usikkert etter at den ble bekreftet mot nullptr. Sjekk linjer: 61, 65. TraceTests.cpp 65

Pekeren V kan være en nullpeker. Dette er åpenbart en feiltilstand, som til og med rapporteres med en feilmelding. Men funksjonen vil bare fortsette å kjøre som om ingenting skjedde, og vil ende opp med å referere den veldig nullpekeren. Programmereren ønsket sannsynligvis at funksjonen skulle stoppe på dette punktet, i så fall skulle den løses 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();

Utdrag 9, en skrivefeil

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

Diagnostisk melding for PVS-Studio: V1001 T variabel er tildelt, men brukes ikke ved slutten av funksjonen. CommonArgs.cpp 873

Se på de siste linjene i funksjonen. Den lokale variabelen T endres, men brukes ikke på noen måte. Dette må være en skrivefeil, og funksjonen bør sannsynligvis ende som følger:

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

Utdrag 10, null 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;
}
....
}

PVS-Studio diagnostiske meldinger:

  • V609 Mod med null. Nevner ‘d.s.low’ == 0. udivmoddi4.c 61
  • V609 Del med null. Nevner ‘d.s.low’ == 0. udivmoddi4.c 62

Jeg vet ikke om dette er en feil eller noe vanskelig grep, men koden ser rart ut. Den har to vanlige heltallvariabler, hvorav den ene er delt av den andre. Men den interessante delen er at delingsoperasjonen bare finner sted hvis begge variablene er nuller. Hvilken oppgave skal den utføre?

Utdrag 11, kopier og lim

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

Diagnosemelding for PVS-Studio: V581 De betingede uttrykkene for if -uttalelsene som ligger ved siden av hverandre, er identiske. Sjekk linjer: 3108, 3113. MallocChecker.cpp 3113

Et kodefragment ble klonet, men aldri endret etterpå. Denne klonen skal enten fjernes eller modifiseres for å utføre noen nyttige kontroller.

Konklusjon

Husk at du kan bruke denne gratislisensen alternativ for å sjekke open source-prosjekter. Vi tilbyr andre måter å bruke PVS-Studio gratis også, noen av dem tillater til og med analyse av proprietær kode. Se hele listen over alternativer her: “ Måter å få en gratis PVS-Studio-lisens ”. Takk for at du leser!

Legg igjen en kommentar

Din e-postadresse vil ikke bli publisert. Obligatoriske felt er merket med *