Kontroll av Clang 11 med PVS-Studio

(28 okt 2020 )

Då och då måste vi skriva artiklar om hur vi har kollat ​​en ny version av någon kompilator. Det är inte riktigt kul. Men som praxis visar, om vi slutar göra det en stund, börjar folk tvivla på om PVS-Studio är värt titeln som en bra fångare av buggar och sårbarheter. Vad händer om den nya kompilatorn kan göra det också? Visst, kompilatorer utvecklas, men det gör PVS-Studio också – och det bevisar om och om igen sin förmåga att fånga buggar även i högkvalitativa projekt som kompilatorer.

Dags för att kontrollera Clang igen

För att säga sanningen skrev jag den här artikelbaserade i det tidigare inlägget “ Kontrollerar GCC 10-kompilatorn med PVS-Studio ”. Så om vissa stycken verkar bekanta beror det på att du redan har läst dem förut :).

Det är ingen hemlighet att kompilatorer använder sina egna inbyggda statiska kodanalysatorer, och de utvecklas också. Det är därför vi skriver artiklar då och då för att visa att vår statiska analysator, PVS-Studio, kan hitta buggar även inuti kompilatorer och att vi är värda vårt salt :).

Du kan faktiskt inte jämföra klassiska statiska analysatorer med kompilatorer. Statiska analysatorer upptäcker inte bara buggar i källkoden utan involverar också en mycket utvecklad infrastruktur. För det första inkluderar det integration med sådana system som SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins och Visual Studio. Den innehåller mekanismer för massdämpning av varningar, vilket gör att du kan börja använda PVS-Studio direkt även i ett stort projekt. Det inkluderar att skicka meddelanden via e-post. Och så vidare. Men den första frågan som utvecklare fortfarande kommer att ställa är: ”Kan din PVS-Studio hitta något som kompilatorer inte kan?” Och det betyder att vi är dömda att skriva artiklar om hur vi kollar kompilatorerna själva om och om igen.

Låt oss komma tillbaka till Clang. Det finns ingen anledning att döma över ämnet och berätta vad projektet handlar om. Vi kollade faktiskt inte bara koden för själva Clang 11 utan också koden för LLVM 11-biblioteket den är baserad på. Ur den här artikelns synvinkel spelar det ingen roll om en defekt hittades i kompilatorns eller bibliotekets kod.

Jag tyckte att Clang / LLVM-koden var mycket tydligare än GCC. Det är åtminstone inte full av alla dessa hemska makron och det använder i stor utsträckning C ++: s moderna funktioner.

Ändå är projektet fortfarande tillräckligt stort för att göra granskningen av analysrapporten tråkig utan föregående anpassning. Det som oftast hamnar i vägen är ”halvfalsa” positiva. Med ”semi-falska” positiva menar jag fall där analysatorn är tekniskt korrekt för att påpeka vissa problem men dessa varningar är till ingen praktisk användning. Till exempel hänvisar många sådana varningar till enhetstester och genererad kod.

Här är ett exempel 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);

analysatorn varnar oss för att variablerna tilldelas samma värden som de redan har:

  • V1048 Variabeln Spaces.SpacesInParentheses tilldelades samma värde. FormatTest.cpp 11554
  • V1048 Variabeln Spaces.SpacesInCStyleCastParentheses tilldelades samma värde. FormatTest.cpp 11556

Tekniskt sett är denna varning till punkten och kodavsnittet behöver förenklas eller fixas. Men det är också klart att den här koden är bra som den är och det är ingen mening att fixa något i den.

Här är ett annat exempel: analysatorn matar ut massor av varningar på den autogenererade filen Options.inc. Titta på ”väggen” i koden som den innehåller:

Den här huvuddelen av koden utlöser en översvämning av varningar:

  • V501 Det finns identiska underuttryck till vänster och till höger om operatören ==: nullptr == nullptr Options.inc 26
  • V501 Det finns identiska underuttryck till vänster och till höger om operatören ==: nullptr == nullptr Options.inc 27
  • V501 Där är identiska underuttryck till vänster och till höger om operatören ==: nullptr == nullptr Options.inc 28
  • och så vidare – en varning per rad …

Ändå är det inte så mycket. Det kan lösas genom att utesluta irrelevanta filer från analys, markera vissa makron och funktioner, undertrycka vissa diagnostiska typer och så vidare. Ja, det kan det, men det är inte ett mycket intressant jobb att göra när du skriver en artikel. Det var därför jag gjorde samma sak som i artikeln om att kontrollera GCC-kompilatorn: Jag fortsatte att läsa rapporten tills jag samlade 11 intressanta exempel att inkludera i artikeln. Varför 11? Jag trodde bara att eftersom det var den 11: e versionen av Clang, behövde jag 11 exempel :).

11 misstänkta kodavsnitt

Utdrag 1, moduloperation på 1

Det här är coolt! Jag gillar sådana buggar!

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

Diagnostiskt meddelande PVS-Studio: V1063 Modulen med 1-operation är meningslös. Resultatet blir alltid noll. llvm-stress.cpp 631

Programmeraren använder en moduloperation för att få ett slumpmässigt värde på antingen 0 eller 1. Men värdet 1 verkar förvirra utvecklare och få dem att skriva det klassiska antimönstret där moduloperationen utförs på 1 istället för 2. Operationen X% 1 är meningslös eftersom den alltid utvärderas till 0 . Det här är den fasta versionen:

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

Den nyligen tillagda V1063-diagnosen är väldigt enkel, men som du ser fungerar den perfekt.

Vi vet att kompilatorutvecklare håller koll på vårt arbete och lånar våra idéer. Det är helt bra. Det är trevligt att veta att PVS-Studio är drivkraften bakom framsteg . Låt oss se hur mycket det tar för en liknande diagnostik att visas i Clang och GCC :).

Utdrag 2, ett stavfel i ett tillstånd

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

Diagnosmeddelande för PVS-Studio: V501 Det finns identiska underuttryck till vänster och till höger om & & operatör:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Kontrollen ! T1.isNull () utförs två gånger. Detta är uppenbarligen ett skrivfel; den andra delen av villkoret måste kontrollera variabeln T2 .

Utdrag 3, potentiell 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();
....
}

Diagnostiskt meddelande PVS-Studio: V557 Matningsöverskridande är möjligt. Indexet ”Index” pekar utöver array-bound. ASTReader.cpp 7318

Antag att matrisen lagrar ett element och värdet för variabeln Index är också 1. Villkoret (1> 1) är falskt, och därför kommer matrisen att indexeras utanför dess gränser. Här är rätt kontroll:

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

Utdrag 4, argumentutvärderingsordning

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-diagnosmeddelande: V567 Ospecificerat beteende. Ordningen för utvärdering av argument är inte definierad för funktionen addSection. Överväg att inspektera variabeln ”SecNo”. Object.cpp 1223

Observera att argumentet SecNo används två gånger och ökas under tiden. Problemet är att du inte kan säga i exakt vilken ordning argumenten kommer att utvärderas. Resultatet kommer därför att variera beroende på kompilatorversionen eller kompileringsparametrarna.

Här är ett syntetiskt exempel för att illustrera denna punkt:

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

Beroende på kompilatorn kan den här koden mata ut antingen “1, 2” eller “2, 1”. Jag körde den på Compiler Explorer och fick följande utgångar:

  • när den kompilerades med Clang 11.0.0, utmatade programmet , 1.
  • när den kompileras med GCC 10.2, kommer programmet utdata s 2, 1.

Intressant nog får detta enkla fall Clang att utfärda en varning:

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

Av någon anledning utfärdades dock denna varning inte med den riktiga koden. Antingen är den inaktiverad som en inte särskilt praktisk sak eller så är fallet för komplicerat för kompilatorn att hantera.

Utdrag 5, en konstig 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;
}
....
}

Diagnosmeddelande för PVS-Studio: V571 Återkommande kontroll. Villkoret ”if (! NameOrErr)” verifierades redan i rad 4666. ELFDumper.cpp 4667

Den andra kontrollen är en klon av den första och är därför överflödig. Kanske kan det tas bort på ett säkert sätt. Men det som är mer troligt är att den innehåller ett stavfel och var tänkt att kontrollera någon annan variabel.

Utdrag 6, potentiell nollpekardereference

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

Diagnosmeddelande för PVS-Studio: V595 CDecl -pekaren användes innan den verifierades mot nullptr. Kontrollrader: 5275, 5284. RewriteObjC.cpp 5275

När den första kontrollen utförs, tvekar utvecklaren aldrig att hänvisa pekaren CDecl :

if (CDecl->isImplicitInterfaceDecl())

Men om du tittar på koden några rader längre fram, blir det tydligt att pekaren kan vara noll:

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

Den första kontrollen var troligen tänkt att se ut så här:

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

Utdrag 7, potentiell nullpekare

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 diagnosmeddelande: V595 ND -pekaren användes innan den verifierades mot nullptr.Kontrollrader: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Det här felet liknar det föregående. Det är farligt att avleda en pekare utan en tidigare kontroll när dess värde förvärvas med hjälp av en dynamisk typ. Mer än så bekräftar efterföljande kod att en sådan kontroll behövs.

Utdrag 8, en funktion som fortsätter att fungera trots ett felläge

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

Diagnosmeddelande PVS-Studio: V1004 V-pekaren användes säkert efter att den verifierades mot nullptr. Kontrollrader: 61, 65. TraceTests.cpp 65

V -pekaren kan vara en nollpekare. Detta är uppenbarligen ett felläge, som till och med rapporteras med ett felmeddelande. Men funktionen fortsätter bara att köra som om ingenting hände och kommer att sluta med att hänvisa den mycket nollpekaren. Programmeraren ville antagligen att funktionen skulle stoppa vid denna tidpunkt, i vilket fall den skulle fixas enligt följande:

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, ett stavfel

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 diagnosmeddelande: V1001 T variabel tilldelas men används inte i slutet av funktionen. CommonArgs.cpp 873

Titta på de sista raderna i funktionen. Den lokala variabeln T ändras men används inte på något sätt. Detta måste vara ett stavfel och funktionen bör antagligen sluta enligt följande:

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

Utdrag 10, noll som delaren

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

Diagnosmeddelanden för PVS-Studio:

  • V609 Mod med noll. Nämnaren ‘d.s.low’ == 0. udivmoddi4.c 61
  • V609 Dela med noll. Nämnaren ‘d.s.low’ == 0. udivmoddi4.c 62

Jag vet inte om det här är ett fel eller något knepigt, men koden ser konstig ut. Den har två vanliga heltalsvariabler, varav den ena delas av den andra. Men det intressanta är att delningsoperationen endast sker om båda variablerna är nollor. Vilken uppgift ska den utföra?

Utdrag 11, kopiera och klistra in

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

Diagnosmeddelande för PVS-Studio: V581 De villkorliga uttrycken för if -uttalandena som ligger bredvid varandra är identiska. Kontrollrader: 3108, 3113. MallocChecker.cpp 3113

Ett kodfragment klonades men ändrades aldrig efteråt. Denna klon bör antingen tas bort eller modifieras för att utföra en användbar kontroll.

Slutsats

Kom ihåg att du kan använda den här fria licensen alternativ för att kontrollera projekt med öppen källkod. Vi erbjuder också andra sätt att använda PVS-Studio gratis, några av dem tillåter även analys av proprietär kod. Se den fullständiga listan med alternativ här: “ Sätt att få en gratis PVS-Studio-licens ”. Tack för att du läste!

Lämna ett svar

Din e-postadress kommer inte publiceras. Obligatoriska fält är märkta *