Vérification de Clang 11 avec PVS-Studio

(28 octobre 2020 )

De temps en temps, nous devons écrire des articles sur la façon dont nous avons vérifié une autre version fraîche dun compilateur. Ce n’est pas vraiment très amusant. Cependant, comme le montre la pratique, si nous arrêtons de faire cela pendant un certain temps, les gens commencent à se demander si PVS-Studio vaut son titre de bon capteur de bogues et de vulnérabilités. Et si le nouveau compilateur pouvait le faire aussi? Bien sûr, les compilateurs évoluent, mais PVS-Studio aussi – et cela prouve, encore et encore, sa capacité à détecter les bogues même dans des projets de haute qualité tels que les compilateurs.

Il est temps de revérifier Clang

Pour vous dire la vérité, jai écrit cet article basé sur le post précédent «  Vérification du compilateur GCC 10 avec PVS-Studio « . Donc, si certains paragraphes vous semblent familiers, cest parce que vous les avez déjà lus :).

Ce nest un secret pour personne que les compilateurs utilisent leurs propres analyseurs de code statique intégrés, et ceux-ci se développent également. Cest pourquoi nous écrivons des articles de temps en temps pour montrer que notre analyseur statique, PVS-Studio, peut trouver des bogues même à lintérieur des compilateurs et que nous valons notre sel :).

En fait, vous ne pouvez pas comparer des analyseurs statiques classiques avec des compilateurs. Les analyseurs statiques détectent non seulement les bogues dans le code source, mais impliquent également une infrastructure hautement développée. Dune part, il inclut lintégration avec des systèmes tels que SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins et Visual Studio. Il comprend des mécanismes de suppression massive des avertissements, ce qui vous permet de commencer à utiliser PVS-Studio dès le départ même dans un grand projet. Cela comprend lenvoi de notifications par e-mail. Et ainsi de suite. Mais la première question que les développeurs se poseront toujours est la suivante: « Votre PVS-Studio peut-il trouver tout ce que les compilateurs ne peuvent pas? » Et cela signifie que nous sommes condamnés à écrire des articles sur la façon dont nous vérifions les compilateurs eux-mêmes encore et encore.

Revenons à Clang. Inutile de s’attarder sur le sujet et de vous dire en quoi consiste le projet. En fait, nous avons vérifié non seulement le code de Clang 11 lui-même, mais également le code de la bibliothèque LLVM 11 sur laquelle il est basé. Du point de vue de cet article, peu importe si un défaut a été trouvé dans le code du compilateur ou de la bibliothèque.

Jai trouvé le code de Clang / LLVM beaucoup plus clair que celui de GCC. Au moins, il ne regorge pas de toutes ces horribles macros et il utilise largement les fonctionnalités modernes de C ++.

Malgré cela, le projet est encore assez grand pour rendre lexamen du rapport danalyse fastidieux sans personnalisation préalable. Ce qui gêne le plus, ce sont les «semi-faux» positifs. Par positifs «semi-faux», jentends les cas où lanalyseur est techniquement correct pour signaler certains problèmes, mais ces avertissements nont aucune utilité pratique. Par exemple, beaucoup de ces avertissements font référence aux tests unitaires et au code généré.

Voici un exemple de tests unitaires:

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

Le lanalyseur nous avertit que les variables reçoivent les mêmes valeurs quelles ont déjà:

  • V1048 La variable Spaces.SpacesInParentheses a reçu la même valeur. FormatTest.cpp 11554
  • V1048 La variable «Spaces.SpacesInCStyleCastParentheses» a reçu la même valeur. FormatTest.cpp 11556

Techniquement, cet avertissement est pertinent et lextrait doit être simplifié ou corrigé. Mais il est également clair que ce code est parfait tel quel et qu’il n’est pas utile de le réparer.

Voici un autre exemple: l’analyseur génère une tonne d’avertissements sur le fichier généré automatiquement Options.inc. Regardez le «mur» de code quil contient:

Cette masse de code déclenche un flot davertissements:

  • V501 Il existe des sous-expressions identiques à gauche et à droite de lopérateur ==: nullptr == nullptr Options.inc 26
  • V501 Il existe des sous-expressions identiques à gauche et à droite de lopérateur ==: nullptr == nullptr Options.inc 27
  • V501 Là sont des sous-expressions identiques à gauche et à droite de lopérateur ==: nullptr == nullptr Options.inc 28
  • et ainsi de suite – un avertissement par ligne…

Pourtant, tout cela n’est pas grave. Elle peut être résolue en excluant les fichiers non pertinents de lanalyse, en marquant certaines macros et fonctions, en supprimant certains types de diagnostic, etc. Oui, cest possible, mais ce nest pas un travail très intéressant à faire lorsque vous écrivez un article. C’est pourquoi j’ai fait la même chose que dans l’article sur la vérification du compilateur GCC: j’ai continué à lire le rapport jusqu’à ce que j’ai rassemblé 11 exemples intéressants à inclure dans l’article. Pourquoi 11? Je pensais juste que puisque cétait la 11ème version de Clang, javais besoin de 11 exemples :).

11 extraits de code suspects

Snippet 1, opération modulo sur 1

Cest cool! Jaime les bugs comme ça!

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

Message de diagnostic PVS-Studio: V1063 Lopération modulo par 1 na pas de sens. Le résultat sera toujours nul. llvm-stress.cpp 631

Le programmeur utilise une opération modulo pour obtenir une valeur aléatoire de 0 ou 1. Mais la valeur 1 semble dérouter les développeurs et leur faire écrire lanti-pattern classique dans lequel lopération modulo est effectuée sur 1 au lieu de 2. Lopération X% 1 na pas de sens car elle vaut toujours 0 . Voici la version corrigée:

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

Le diagnostic V1063 récemment ajouté est terriblement simple, mais, comme vous pouvez le voir, il fonctionne parfaitement.

Nous savons que les développeurs de compilateurs gardent un œil sur notre travail et empruntent nos idées. C’est très bien. Il est bon de savoir que PVS-Studio est le moteur du progrès . Voyons combien il faudra pour quun diagnostic similaire apparaisse dans Clang et GCC :).

Snippet 2, une faute de frappe dans une condition

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

Message de diagnostic PVS-Studio: V501 Il existe des sous-expressions identiques à gauche et à droite du Opérateur & &:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Le contrôle ! T1.isNull () est effectué deux fois. Cest évidemment une faute de frappe; la deuxième partie de la condition doit vérifier la variable T2 .

Extrait 3, potentiel 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();
....
}

Message de diagnostic PVS-Studio: Le dépassement de la matrice V557 est possible. Lindex «Index» pointe au-delà de la limite du tableau. ASTReader.cpp 7318

Supposons que le tableau stocke un élément et que la valeur de la variable Index est également 1. Alors la condition (1> 1) est faux et, par conséquent, le tableau sera indexé au-delà de ses limites. Voici la vérification correcte:

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

Extrait 4, ordre dévaluation des arguments

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

Message de diagnostic PVS-Studio: V567 Comportement non spécifié. L’ordre d’évaluation des arguments n’est pas défini pour la fonction «addSection». Pensez à inspecter la variable «SecNo». Object.cpp 1223

Notez que largument SecNo est utilisé deux fois, sincrémentant entre-temps. Le problème est que vous ne pouvez pas dire dans quel ordre exact les arguments seront évalués. Le résultat variera donc en fonction de la version du compilateur ou des paramètres de compilation.

Voici un exemple synthétique pour illustrer ce point:

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

Selon le compilateur, ce code peut afficher «1, 2» ou «2, 1». Je lai exécuté sur Compiler Explorer et jai obtenu les sorties suivantes:

  • une fois compilé avec Clang 11.0.0, le programme sorties 1 , 1.
  • une fois compilé avec GCC 10.2, le programme output s 2, 1.

Fait intéressant, ce cas simple amène Clang à émettre un avertissement:

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

Pour une raison quelconque, cependant, cet avertissement na pas été émis sur le code réel. Soit il est désactivé car il nest pas très pratique, soit ce cas est trop compliqué pour le compilateur.

Extrait 5, une étrange vérification des doublons

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

Message de diagnostic PVS-Studio: V571 Contrôle récurrent. La condition «if (! NameOrErr)» a déjà été vérifiée à la ligne 4666. ELFDumper.cpp 4667

La deuxième vérification est un clone de la première et est donc redondante. Peut-être quil pourrait être retiré en toute sécurité. Mais ce qui est plus probable, cest quil contient une faute de frappe et était destiné à vérifier une autre variable.

Extrait 6, déréférencement de pointeur nul potentiel

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

Message de diagnostic PVS-Studio: V595 Le pointeur CDecl a été utilisé avant dêtre vérifié par rapport à nullptr. Lignes de contrôle: 5275, 5284. RewriteObjC.cpp 5275

Lors de la première vérification, le développeur nhésite jamais à déréférencer le pointeur CDecl :

if (CDecl->isImplicitInterfaceDecl())

Mais si vous regardez le code quelques lignes plus loin, il devient clair que le pointeur peut être nul:

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

La première vérification devait probablement ressembler à ceci:

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

Extrait 7, déréférencement de pointeur nul potentiel

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

Message de diagnostic PVS-Studio: V595 Le pointeur ND a été utilisé avant dêtre vérifié. nullptr.Vérifiez les lignes: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Cette erreur est similaire à la précédente. Il est dangereux de déréférencer un pointeur sans vérification préalable lorsque sa valeur est acquise à l’aide d’une conversion de type dynamique. De plus, le code suivant confirme quune telle vérification est nécessaire.

Snippet 8, une fonction qui continue de fonctionner malgré un état derreur

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

Message de diagnostic PVS-Studio: V1004 Le pointeur V a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 61, 65. TraceTests.cpp 65

Le pointeur V peut être un pointeur nul. Il sagit évidemment dun état derreur, qui est même signalé par un message derreur. Mais la fonction continuera à fonctionner comme si de rien nétait et finira par déréférencer ce pointeur très nul. Le programmeur voulait probablement que la fonction sarrête à ce stade, auquel cas elle devrait être corrigée comme suit:

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

Extrait 9, une faute de frappe

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

Message de diagnostic PVS-Studio: V1001 Le T La variable est affectée mais nest pas utilisée à la fin de la fonction. CommonArgs.cpp 873

Regardez les dernières lignes de la fonction. La variable locale T change mais n’est en aucun cas utilisée. Il doit sagir dune faute de frappe et la fonction doit probablement se terminer comme suit:

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

Extrait 10, zéro comme le diviseur

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

Messages de diagnostic PVS-Studio:

  • V609 Mod par zéro. Dénominateur «d.s.low» == 0. udivmoddi4.c 61
  • V609 Diviser par zéro. Dénominateur «d.s.low» == 0. udivmoddi4.c 62

Je ne sais pas sil sagit dun bogue ou dun engin délicat, mais le code semble étrange. Il a deux variables entières régulières, dont lune est divisée par lautre. Mais la partie intéressante est que lopération de division na lieu que si les deux variables sont des zéros. Quelle tâche est-elle censée accomplir?

Extrait 11, copier-coller

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

Message de diagnostic PVS-Studio: V581 Les expressions conditionnelles des instructions if situées côte à côte sont identiques. Vérifiez les lignes: 3108, 3113. MallocChecker.cpp 3113

Un fragment de code a été cloné mais jamais modifié par la suite. Ce clone doit être supprimé ou modifié pour effectuer des vérifications utiles.

Conclusion

Noubliez pas que vous pouvez utiliser cette licence gratuite option pour vérifier les projets open-source. Nous proposons également dautres moyens dutiliser gratuitement PVS-Studio, certains dentre eux permettant même lanalyse de code propriétaire. Consultez la liste complète des options ici: «  Moyens dobtenir une licence PVS-Studio gratuite « . Merci davoir lu!

Laisser un commentaire

Votre adresse e-mail ne sera pas publiée. Les champs obligatoires sont indiqués avec *