Clang 11: n tarkistaminen PVS-Studiolla

(28. lokakuuta 2020 )

Meidän on silloin tällöin kirjoitettava artikkeleita siitä, kuinka olemme tarkistaneet jonkin toisen kääntäjän uuden version. Se ei ole todella hauskaa. Kuten käytäntö kuitenkin osoittaa, jos lopetamme sen tekemisen jonkin aikaa, ihmiset alkavat epäillä, onko PVS-Studio arvonsa hyvä vikojen ja haavoittuvuuksien saaja. Entä jos uusi kääntäjä voi tehdä sen myös? Toki, kääntäjät kehittyvät, mutta niin kehittyy myös PVS-Studio – ja se osoittaa yhä uudelleen kykynsä tarttua virheisiin jopa laadukkaissa projekteissa, kuten kääntäjissä.

Aika Clangin tarkistamiseen uudelleen

Totta puhuen kirjoitin tämän artikkelin edellisessä viestissä “ GCC 10-kääntäjän tarkistaminen PVS-Studion avulla . Joten jos jotkut kappaleet näyttävät tutuilta, se johtuu siitä, että olet jo lukenut ne aikaisemmin :).

Ei ole mikään salaisuus, että kääntäjät käyttävät omia sisäänrakennettuja staattisten koodien analysaattoreita, ja myös ne kehittyvät. Siksi kirjoitamme silloin tällöin artikkeleita osoittaaksemme, että staattinen analysaattorimme, PVS-Studio, voi löytää vikoja jopa kääntäjistä ja että olemme suolamme arvoisia :).

Itse asiassa et voi vertaa klassisia staattisia analysaattoreita kääntäjiin. Staattiset analysaattorit havaitsevat lähdekoodivirheiden lisäksi myös erittäin kehittyneen infrastruktuurin. Ensinnäkin se sisältää integroinnin sellaisten järjestelmien kanssa kuin SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins ja Visual Studio. Se sisältää mekanismeja varoitusten massiiviseen tukahduttamiseen, jonka avulla voit aloittaa PVS-Studion käytön heti suuressa projektissa. Se sisältää ilmoitusten lähettämisen sähköpostitse. Ja niin edelleen. Ensimmäinen kysymys, jonka kehittäjät kuitenkin kysyvät, on: ”Voiko PVS-Studio löytää jotain, mitä kääntäjät eivät voi?” Ja se tarkoittaa, että olemme tuomittuja kirjoittamaan artikkeleita siitä, miten tarkistamme kääntäjiä itse yhä uudelleen.

Palataan takaisin Clangiin. Ei ole tarpeen pysähtyä aiheeseen ja kertoa, mistä projektissa on kyse. Tarkistimme itse Clang 11: n koodin lisäksi myös sen perustaman LLVM 11 -kirjaston koodin. Tämän artikkelin näkökulmasta ei ole väliä onko havaittu vikaa kääntäjän vai kirjaston koodissa.

Minusta Clang / LLVM-koodi oli paljon selkeämpi kuin GCC: n. Ainakin se ei ole täynnä kaikkia näitä kauhistuttavia makroja, ja se käyttää laajasti C ++: n moderneja ominaisuuksia.

Siitä huolimatta projekti on silti riittävän suuri, jotta analyysiraportin tutkiminen olisi työläs ilman edeltäviä mukautuksia. Enimmäkseen se estää ”puolivirheitä”. ”Puolivirheellisillä” positiivisilla tarkoitan tapauksia, joissa analysaattori on teknisesti oikea osoittamaan tiettyjä asioita, mutta näillä varoituksilla ei ole käytännön hyötyä. Esimerkiksi monet tällaiset varoitukset viittaavat yksikkötesteihin ja luotuun koodiin.

Tässä on esimerkki yksikötesteistä:

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

analysaattori varoittaa meitä siitä, että muuttujille määritetään samat arvot kuin niillä:

  • V1048 Muuttujalle Spaces.SpacesInParentheses annettiin sama arvo. FormatTest.cpp 11554
  • V1048 Muuttujalle ”Spaces.SpacesInCStyleCastParentheses” annettiin sama arvo. FormatTest.cpp 11556

Teknisesti tämä varoitus on asiaankuuluva ja koodinpätkä on yksinkertaistettava tai korjattava. Mutta on myös selvää, että tämä koodi on hieno sellaisenaan, eikä siinä ole mitään syytä korjata mitään.

Tässä on toinen esimerkki: analysaattori antaa paljon varoituksia automaattisesti tuotetulle tiedostolle Options.inc. Katso sen sisältämän koodin ”seinä”:

Tämä suurin osa koodista laukaisee varoituksen:

  • V501 Operaattorin == vasemmalla ja oikealla puolella on identtisiä alilausekkeita: nullptr == nullptr Options.inc 26
  • V501 Operaattorin == vasemmalla ja oikealla puolella on identtisiä alilausekkeita: nullptr == nullptr Options.inc 27
  • V501 There ovat identtisiä alilausekkeita operaattorin == vasemmalla ja oikealla puolella: nullptr == nullptr Options.inc 28
  • ja niin edelleen – yksi varoitus riviä kohti …

Silti kaikki ei ole iso juttu. Se voidaan ratkaista poistamalla epäolennaiset tiedostot analyysistä, merkitsemällä tietyt makrot ja toiminnot, tukahduttamalla tietyt diagnostiikkatyypit jne. Kyllä, voi, mutta se ei ole kovin mielenkiintoinen tehtävä artikkelia kirjoitettaessa. Siksi tein saman asian kuin artikkelissa GCC-kääntäjän tarkistamisesta: Luin raporttia, kunnes keräsin 11 mielenkiintoista esimerkkiä sisällytettäväksi artikkeliin. Miksi 11? Ajattelin vain, että koska se oli Clangin 11. versio, tarvitsin 11 esimerkkiä :).

11 epäilyttävää koodinpätkää

Katkelma 1, moduulitoiminto 1

Tämä on hieno! Pidän sellaisista virheistä!

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

PVS-Studion diagnostiikkaviesti: V1063 1-moduulin toiminto on merkityksetön. Tulos on aina nolla. llvm-stress.cpp 631

Ohjelmoija käyttää modulo-operaatiota satunnaisen arvon 0 tai 1 saamiseksi. Mutta arvo 1 näyttää hämmentävän kehittäjiä ja saa heidät kirjoittamaan klassisen anti-mallin, jossa modulo-operaatio suoritetaan yhdellä 2: n sijaan. X% 1 -operaatio on merkityksetön, koska se arvioi aina arvoksi 0 . Tämä on kiinteä versio:

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

Äskettäin lisätty V1063-diagnoosi on kauhean yksinkertainen, mutta, kuten näette, se toimii täydellisesti.

Tiedämme, että kääntäjien kehittäjät pitävät silmällä työtämme ja lainaa ideoitamme. Se on aivan hieno. On hienoa tietää, että PVS-Studio on edistymisen liikkeellepaneva voima . Katsotaanpa, kuinka paljon samanlaisen diagnoosin ilmestyminen Clangissa ja GCC: ssä kestää :).

Katkelma 2, kirjoitusvirhe ehdossa

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-Studion diagnostiikkaviesti: V501 Kohteen vasemmalla ja oikealla puolella on identtisiä alilausekkeita & & operaattori:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

! T1.isNull () -tarkistus suoritetaan kahdesti. Tämä on tietysti kirjoitusvirhe; ehdon toisen osan on tarkistettava muuttuja T2 .

Katkelma 3, potentiaalinen array-index-out-of- bounds

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

PVS-Studio-diagnostiikkaviesti: V557-matriisin ylitys on mahdollista. Hakemisto-indeksi osoittaa matriisisidoksen yli. ASTReader.cpp 7318

Oletetaan, että taulukko tallentaa yhden elementin ja Hakemisto -muuttujan arvo on myös 1. Sitten (1> 1) ehto on väärä, ja siksi taulukko indeksoidaan sen rajojen ulkopuolelle. Tässä on oikea tarkistus:

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

Katkelma 4, argumenttien arviointijärjestys

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

PVS-Studion diagnostiikkaviesti: V567 Määrittelemätön toiminta. Argumenttien arvioinnin järjestystä ei ole määritetty funktiolle ”addSection”. Harkitse SecNo-muuttujan tarkastamista. Object.cpp 1223

Huomaa, että SecNo -argumenttia käytetään kahdesti, mikä kasvaa samalla. Ongelmana on, et voi kertoa missä järjestyksessä argumentit arvioidaan. Tulos vaihtelee siksi kääntäjän versiosta tai kokoamisparametreista riippuen.

Tässä on synteettinen esimerkki tämän kohdan havainnollistamiseksi:

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

Kääntäjästä riippuen tämä koodi voi tuottaa joko “1, 2” tai “2, 1”. Suoritin sen Compiler Explorerilla ja sain seuraavat lähdöt:

  • kun käännettiin Clang 11.0.0: lla, ohjelma lähdöt 1 , 1.
  • käännettynä GCC 10.2: lla ohjelma tuotos s 2, 1.

Mielenkiintoista on, että tämä yksinkertainen tapaus saa Clangin antamaan varoituksen:

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

Jostain syystä tätä varoitusta ei kuitenkaan annettu todellisessa koodissa. Joko se on poistettu käytöstä, koska se ei ole kovin käytännöllinen, tai kyseinen tapaus on liian monimutkainen kääntäjän selviytymiseksi.

Katkelma 5, outo päällekkäinen tarkistus

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

PVS-Studio-diagnostiikkaviesti: V571 Toistuva tarkistus. Jos (! NameOrErr) -ehto vahvistettiin jo rivillä 4666. ELFDumper.cpp 4667

Toinen tarkistus on ensimmäisen klooni ja on siten tarpeeton. Ehkä se voidaan poistaa turvallisesti. Mutta mikä todennäköisempää on, että se sisältää kirjoitusvirheen ja oli tarkoitettu tarkistamaan jokin muu muuttuja.

Koodinpätkä 6, potentiaalinen nollaosoittimen poikkeama

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-diagnostiikkaviesti: V595 CDecl-osoitinta käytettiin ennen kuin sitä tarkistettiin nullptr: n suhteen. Tarkista rivit: 5275, 5284. RewriteObjC.cpp 5275

Ensimmäistä tarkistusta suorittaessaan kehittäjä ei koskaan epäröi poistaa CDecl -osoitinta:

if (CDecl->isImplicitInterfaceDecl())

Mutta jos katsot koodia muutama rivi eteenpäin, käy selväksi, että osoitin voi olla tyhjä:

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

Ensimmäisen tarkistuksen oli tarkoitus näyttää tältä:

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

Katkelma 7, mahdollinen tyhjä osoittimen poikkeama

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-diagnostiikkaviesti: V595 ND -osoitinta käytettiin ennen sen tarkistamista nullptr.Tarkista rivit: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Tämä virhe on samanlainen kuin edellinen. On vaarallista jättää osoittimen huomiotta ilman ennakkotarkastusta, kun sen arvo on saatu dynaamisen tyyppivalun avulla. Sen lisäksi seuraava koodi vahvistaa, että tällaista tarkistusta tarvitaan.

Koodinpätkä 8, toiminto, joka toimii edelleen virhetilasta huolimatta

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

PVS-Studion diagnostiikkaviesti: V1004 V-osoitinta käytettiin epävarmasti sen jälkeen, kun se oli tarkistettu nullptr: n suhteen. Tarkista rivit: 61, 65. TraceTests.cpp 65

V -osoitin voi olla tyhjä osoitin. Tämä on tietysti virhetila, joka ilmoitetaan jopa virheilmoituksella. Mutta toiminto jatkaa toimintaansa ikään kuin mitään ei olisi tapahtunut, ja se johtaa lopulta siihen, että aivan tyhjä osoitin. Ohjelmoija todennäköisesti halusi toiminnon pysähtyvän tässä vaiheessa, jolloin se tulisi korjata seuraavasti:

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

Katkelma 9, kirjoitusvirhe

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-Studion diagnostiikkaviesti: V1001 T muuttuja on määritetty, mutta sitä ei käytetä funktion lopussa. CommonArgs.cpp 873

Katso funktion viimeisiä rivejä. Paikallinen muuttuja T muuttuu, mutta sitä ei käytetä millään tavalla. Tämän on oltava kirjoitusvirhe ja funktion pitäisi todennäköisesti päättyä seuraavasti:

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

Katkelma 10, nolla kuin jakaja

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-Studion diagnostiikkaviestit:

  • V609 Mod nolla. Nimittäjä ‘d.s.low’ == 0. udivmoddi4.c 61
  • V609 Jaa nolla. Nimittäjä ‘d.s.low’ == 0. udivmoddi4.c 62

En tiedä onko kyseessä vika vai jokin hankala keksintö, mutta koodi näyttää oudolta. Sillä on kaksi säännöllistä kokonaislukumuuttujaa, joista toinen on jaettu toisella. Mutta mielenkiintoinen osa on, että jakooperaatio tapahtuu vain, jos molemmat muuttujat ovat nollia. Mikä tehtävä sen on tarkoitus suorittaa?

Katkelma 11, copy-paste

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-diagnostiikkaviesti: V581 Rinnakkain olevien if-lausekkeiden ehdolliset lausekkeet ovat identtiset. Tarkista rivit: 3108, 3113. MallocChecker.cpp 3113

Koodifragmentti kloonattiin, mutta sitä ei koskaan muutettu jälkikäteen. Tämä klooni on joko poistettava tai muunnettava hyödyllisen tarkistuksen suorittamiseksi.

Päätelmä

Muista, että voit käyttää tätä ilmaista käyttöoikeutta vaihtoehto tarkistaa avoimen lähdekoodin projektit. Tarjoamme myös muita tapoja käyttää PVS-Studiota ilmaiseksi, jotkut niistä jopa sallivat oman koodin analysoinnin. Katso täydellinen luettelo vaihtoehdoista täältä: “ Tapoja saada ilmainen PVS-Studio-lisenssi ”. Kiitos lukemisesta!

Vastaa

Sähköpostiosoitettasi ei julkaista. Pakolliset kentät on merkitty *