Clang 11 ellenőrzése PVS-Studio segítségével

(2020. október 28.) )

Hébe-hóba cikkeket kell írnunk arról, hogyan ellenőriztük valamelyik fordító újabb friss verzióját. Ez nem igazán szórakoztató. Amint azonban a gyakorlat azt mutatja, ha egy időre abbahagyjuk ezt, az emberek kételkedni kezdenek abban, hogy a PVS-Studio megéri-e a hibákat és a sebezhetőségeket jól elkapó címét. Mi van, ha az új fordító is képes erre? Persze, a fordítók fejlődnek, de a PVS-Studio is – és újra és újra bebizonyítja, hogy képes hibákat elkapni még olyan kiváló minőségű projektekben is, mint a fordítók.

A Clang újbóli ellenőrzésének ideje

Az igazat megvallva, ezt a cikket írtam a „ A GCC 10 fordító ellenőrzése a PVS-Studio segítségével korábbi bejegyzésről. Tehát, ha néhány bekezdés ismerősnek tűnik, az azért van, mert már korábban is elolvasta őket :).

Nem titok, hogy a fordítók saját beépített statikus kód-analizátorokat alkalmaznak, és ezek is fejlődnek. Ezért írunk hébe-hóba cikkeket annak bemutatására, hogy statikus elemzőnk, a PVS-Studio képes-e hibákat találni még a fordítókban is, és hogy megérjük a sóinkat :).

Valójában nem lehet hasonlítsa össze a klasszikus statikus analizátorokat a fordítókkal. A statikus analizátorok nemcsak a forráskód hibáit észlelik, hanem fejlett infrastruktúrát is magukban foglalnak. Egyrészt magában foglalja az integrációt olyan rendszerekkel, mint a SonarQube, a PlatformIO, az Azure DevOps, a Travis CI, a CircleCI, a GitLab CI / CD, a Jenkins és a Visual Studio. Ez magában foglalja a figyelmeztetések tömeges elnyomásának mechanizmusait, amelyek lehetővé teszik a PVS-Studio használatának megkezdését akár egy nagy projektnél is. Ez magában foglalja az értesítések e-mailben történő elküldését. És így tovább, és így tovább. De a fejlesztők továbbra is az első kérdést fogják feltenni: “Talál-e a PVS-Studio bármit, amit a fordítók nem találnak?” Ez azt jelenti, hogy arra vagyunk ítélve, hogy cikkeket írjunk arról, hogyan ellenőrizzük újra és újra magukat a fordítókat.

Térjünk vissza Clangra. Nem szükséges tovább foglalkozni a témával és elmondani, miről szól a projekt. Valójában nem csak a Clang 11 kódját, hanem az LLVM 11 könyvtár kódját is ellenőriztük. E cikk szempontjából nem számít, hogy hibát találtak-e a fordító vagy a könyvtár kódjában.

A Clang / LLVM kódját sokkal egyértelműbbnek találtam, mint a GCC kódját. Legalábbis nem hemzseg azoktól a szörnyű makróktól, és széles körben alkalmazza a C ++ modern funkcióit.

Ennek ellenére a projekt még mindig elég nagy ahhoz, hogy az elemzési jelentés megvizsgálása előzetes testreszabás nélkül unalmas legyen. Ami többnyire útjában áll, az a „félhamis” pozitívumok. A „félig hamis” pozitívumok alatt azokat az eseteket értem, amikor az elemző technikailag helytálló bizonyos kérdések rámutatására, de ezek a figyelmeztetések nincsenek gyakorlati hasznukra. Például sok ilyen figyelmeztetés egységtesztekre és generált kódokra vonatkozik.

Íme egy példa az egység tesztjeire:

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

Az elemző arra figyelmeztet, hogy a változókhoz ugyanazok az értékek vannak hozzárendelve, mint amelyek már rendelkeznek:

  • V1048 A „Spaces.SpacesInParentheses” változóhoz ugyanazt az értéket rendelték. FormatTest.cpp 11554
  • V1048 A „Spaces.SpacesInCStyleCastParentheses” változóhoz ugyanazt az értéket rendelték. FormatTest.cpp 11556

Technikailag ez a figyelmeztetés lényeges, és a kódrészletet egyszerűsíteni vagy javítani kell. De az is egyértelmű, hogy ez a kód rendben van, és nincs értelme javítani rajta semmit.

Íme egy másik példa: az elemző rengeteg figyelmeztetést ad ki az automatikusan generált Options.inc fájlról. Nézd meg a kód falát:

Ez a kódtömeg figyelmeztetések áradatát váltja ki:

  • V501 A == operátor bal és jobb oldalán azonos részkifejezések találhatók: nullptr == nullptr Options.inc 26
  • V501 Az == operátor bal és jobb oldalán azonos részkifejezések találhatók: nullptr == nullptr Options.inc 27
  • V501 Ott azonos részkifejezések a == operátor bal és jobb oldalán: nullptr == nullptr Options.inc 28
  • és így tovább – soronként egy figyelmeztetés …

Mindazonáltal nem nagy ügy. Ez megoldható úgy, hogy kizárja az irreleváns fájlokat az elemzésből, bizonyos makrókat és funkciókat megjelöl, bizonyos diagnosztikai típusokat elnyom és így tovább. Igen, lehet, de nem túl érdekes feladat, amikor cikket írsz. Ezért ugyanazt tettem, mint a GCC fordítójának ellenőrzéséről szóló cikkben: addig olvasgattam a jelentést, amíg 11 érdekes példát összegyűjtöttem, amelyeket bele kell foglalni a cikkbe. Miért 11? Csak arra gondoltam, hogy mivel a Clang 11. verziója volt, 11 példára volt szükségem :).

11 gyanús kódrészlet

1. kivonat, modulo művelet 1-en

Ez nagyon jó! Szeretem az ilyen hibákat!

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-Studio diagnosztikai üzenet: V1063 A modulo 1 művelet értelmetlen. Az eredmény mindig nulla lesz. llvm-stress.cpp 631

A programozó modulo műveletet használ, hogy 0 vagy 1 véletlenszerű értéket kapjon. De az 1 érték összezavarja a fejlesztőket és késztesse őket arra a klasszikus anti-minta írására, amelyben a modulo műveletet 2 helyett 1-n hajtják végre. Az X% 1 művelet értelmetlen, mivel mindig 0 . Ez a fix verzió:

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

A nemrégiben hozzáadott V1063 diagnosztika rettenetesen egyszerű, de, mint láthatja, tökéletesen működik.

Tudjuk, hogy a fordító fejlesztői szemmel tartják munkánkat és kölcsönzik ötleteinket. Ez teljesen rendben van. Örülök, hogy tudjuk, hogy a PVS-Studio a haladás mozgatórugója . Lássuk, mennyi idő kell ahhoz, hogy egy hasonló diagnosztika megjelenjen a Clang és a GCC-ben :).

2. kivonat, típushiba

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 diagnosztikai üzenet: V501 A (z) bal és jobb oldalán azonos kifejezések találhatók & & operátor:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

A ! T1.isNull () ellenőrzést kétszer hajtják végre. Ez nyilvánvalóan elgépelés; a feltétel második részének ellenőriznie kell a T2 változót.

3. kivonat, potenciális tömbindex-kívül határok

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 diagnosztikai üzenet: A V557 tömb túllépése lehetséges. Az „Index” index túlmutat a tömbhöz kötötten. ASTReader.cpp 7318

Tegyük fel, hogy a tömb egy elemet tárol, és az Index változó értéke is 1. Ezután az (1> 1) feltétel hamis, ezért a tömböt a határain túl indexelik. Itt van a helyes ellenőrzés:

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

4. részlet, argumentumértékelési sorrend

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 diagnosztikai üzenet: V567 Nem meghatározott viselkedés. Az argumentumok kiértékelésének sorrendje nincs meghatározva az „addSection” függvényhez. Fontolja meg a „SecNo” változó vizsgálatát. Object.cpp 1223

Ne feledje, hogy a SecNo argumentumot kétszer használják, és közben növekszik. A probléma az, hogy nem tudja megmondani, hogy az argumentumok milyen sorrendben kerülnek kiértékelésre. Az eredmény tehát a fordító verziójától vagy a fordítási paraméterektől függően változik.

Íme egy szintetikus példa ennek a pontnak a bemutatására:

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

A fordítótól függően ez a kód vagy „1, 2”, vagy „2, 1” lehet. A Compiler Explorer programon futtattam, és a következő kimeneteket kaptam:

  • a Clang 11.0.0 verzióval lefordítva a program kimeneteket 1 , 1.
  • a GCC 10.2 verziójával fordítva a program kimenet s 2, 1.

Érdekes, hogy ez az egyszerű eset Clang figyelmeztetést ad ki:

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

Valamilyen okból azonban ezt a figyelmeztetést nem a valós kódra adták ki. Vagy le van tiltva, mivel nem túl praktikus, vagy az eset túl bonyolult ahhoz, hogy a fordító megbirkózzon vele.

5. részlet, egy furcsa ismétlődő ellenőrzés

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 diagnosztikai üzenet: V571 Ismétlődő ellenőrzés. Az „if (! NameOrErr)” feltételt már ellenőrizte a 4666. sor. ELFDumper.cpp 4667

A második ellenőrzés az első klónja, ezért felesleges. Talán biztonságosan eltávolítható. De annál valószínűbb, hogy elírási hibát tartalmaz, és valamilyen más változó ellenőrzésére készült.

6. kivonat, potenciális nullpont-eltérés

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 diagnosztikai üzenet: V595 A CDecl mutatót még a nullptr elleni ellenőrzés előtt használták. Ellenőrizze a sorokat: 5275, 5284. RewriteObjC.cpp 5275

Az első ellenőrzés végrehajtásakor a fejlesztő soha nem habozik elutasítani a CDecl mutatót:

if (CDecl->isImplicitInterfaceDecl())

De ha megnézzük a kódot néhány sorral tovább, akkor világossá válik, hogy a mutató null lehet:

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

Az első ellenőrzés valószínűleg így nézett ki:

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

Snippet 7, null null potenciális eltérés

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 diagnosztikai üzenet: V595 Az ND mutatót használták, mielőtt ellenőrizték volna nullptr.Ellenőrizze a sorokat: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Ez a hiba hasonló az előzőhöz. Előfordulhat, hogy előzetes ellenőrzés nélkül el kell vetni a mutatót, ha az értékét egy dinamikus típusú öntött formátum alapján kapják meg. Ráadásul a későbbi kód megerősíti, hogy ilyen ellenőrzésre van szükség.

Snippet 8, egy funkció a

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-Studio diagnosztikai üzenet: V1004 A„ V ”mutatót nem biztonságos módon használták, miután ellenőrizték a nullptr-rel. Ellenőrizze a sorokat: 61, 65. TraceTests.cpp 65

A V mutató nullmutató lehet. Ez nyilvánvalóan hibaállapot, amelyet akár hibaüzenettel is jelentenek. De a funkció csak tovább fog futni, mintha mi sem történt volna, és végül a nagyon nullpontos mutatót fogja levezetni. A programozó valószínűleg azt akarta, hogy a függvény leálljon ezen a ponton, ebben az esetben a következőképpen kell javítani:

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

9. kivonat, elírás

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 diagnosztikai üzenet: V1001 A T változó van hozzárendelve, de a függvény vége nem használja. CommonArgs.cpp 873

Nézze meg a függvény utolsó sorait. A T helyi változó megváltozik, de semmilyen módon nem használható. Ennek elírásnak kell lennie, és a függvénynek valószínűleg a következőképpen kell végződnie:

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

10. kivonat, nulla az osztó

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 diagnosztikai üzenetek:

  • V609 Mod nullával. Nevező ’d.s.low’ == 0. udivmoddi4.c 61
  • V609 Oszd el nullával. Nevező ’d.s.low’ == 0. udivmoddi4.c 62

Nem tudom, hogy ez hiba vagy valami trükkös elgondolás, de a kód mégis furcsának tűnik. Két szabályos egész változóval rendelkezik, amelyek egyikét elosztjuk a másikkal. De az érdekes rész az, hogy az osztási műveletre csak akkor kerül sor, ha mindkét változó nulla. Milyen feladatot kell végrehajtania?

11. kivonat, 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 diagnosztikai üzenet: V581 Az if utasítások egymás mellett elhelyezett feltételes kifejezései azonosak. Ellenőrizze a sorokat: 3108, 3113. MallocChecker.cpp 3113

Egy kódrészletet klónoztak, de utána soha nem módosítottak. Ezt a klónt vagy el kell távolítani, vagy módosítani kell egy hasznos ellenőrzés elvégzéséhez.

Következtetés

Ne feledje, hogy használhatja ezt az ingyenes licencet. opció a nyílt forráskódú projektek ellenőrzéséhez. Más módszereket kínálunk a PVS-Studio ingyenes használatára is, amelyek közül néhány lehetővé teszi a saját kód elemzését is. Itt tekintheti meg a lehetőségek teljes listáját: “ Ingyenes PVS-Studio licenc megszerzésének módjai ”. Köszönjük, hogy elolvastad!

Vélemény, hozzászólás?

Az email címet nem tesszük közzé. A kötelező mezőket * karakterrel jelöltük