L'histoire de la rencontre de l'analyseur statique PVS-Studio avec le code du système d'exploitation Haiku remonte à 2015. Ce fut une expérience intéressante et une expérience utile pour les équipes des deux projets. Pourquoi une expérience? Il n'y avait pas alors d'analyseur pour Linux et il n'y aura pas encore un an et demi. Mais le travail des passionnés de notre équipe a été récompensé: nous avons rencontré les développeurs de Haiku et amélioré la qualité du code, reconstitué la base de données avec de rares erreurs de programmation et affiné l'analyseur. Vérifier le code de Haiku pour les erreurs est maintenant rapide et facile.
Présentation
Les héros de notre histoire sont le système d'exploitation
Haiku open source et l'analyseur statique
PVS-Studio pour C, C ++, C # et Java. Il y a 4,5 ans, nous avons commencé à analyser le projet, nous n'avions qu'à travailler avec le fichier exécutable de l'analyseur compilé. Toute l'infrastructure pour analyser les paramètres de compilation, démarrer un préprocesseur, paralléliser l'analyse, etc. a été extrait de l'utilitaire
Compiler Monitoring UI en C #, qui a été porté en partie sur la plate-forme Mono afin de fonctionner sous Linux. Le projet Haiku lui-même est construit à l'aide d'un compilateur croisé sous divers systèmes d'exploitation, à l'exception de Windows. Encore une fois, je tiens à noter la commodité et l'exhaustivité de la documentation de l'assemblage Haiku, et je remercie également les développeurs Haiku pour leur aide dans la construction du projet.
Maintenant, l'analyse est beaucoup plus facile. La liste de toutes les commandes pour construire et analyser le projet ressemble à ceci:
cd /opt git clone https:
Soit dit en passant, l'analyse du projet a été réalisée dans le conteneur Docker. Récemment, nous avons préparé une nouvelle documentation sur ce sujet:
Exécution de PVS-Studio dans Docker . Cela peut grandement simplifier l'utilisation de techniques d'analyse de projet statique pour certaines entreprises.
Variables non initialisées
V614 Variable non initialisée 'rval' utilisée. fetch.c 1727
int auto_fetch(int argc, char *argv[]) { volatile int argpos; int rval;
La variable
rval n'a pas
été initialisée lorsqu'elle a été déclarée, donc la comparer avec une valeur nulle conduira à un résultat indéfini. Si les circonstances échouent, la valeur non définie de la variable
rval peut devenir la valeur de retour de la fonction
auto_fetch .
V614 Pointeur
non initialisé «res» utilisé. commandes.c 2873
struct addrinfo { int ai_flags; int ai_family; int ai_socktype; int ai_protocol; socklen_t ai_addrlen; char *ai_canonname; struct sockaddr *ai_addr; struct addrinfo *ai_next; }; static int sourceroute(struct addrinfo *ai, char *arg, char **cpp, int *lenp, int *protop, int *optp) { static char buf[1024 + ALIGNBYTES]; char *cp, *cp2, *lsrp, *ep; struct sockaddr_in *_sin; #ifdef INET6 struct sockaddr_in6 *sin6; struct ip6_rthdr *rth; #endif struct addrinfo hints, *res;
Un cas similaire d'utilisation d'une variable non initialisée, uniquement ici est le pointeur non initialisé
res .
V506 Le pointeur vers la variable locale «normalisée» est stocké en dehors de la portée de cette variable. Un tel pointeur deviendra invalide. TextView.cpp 5596
void BTextView::_ApplyStyleRange(...., const BFont* font, ....) { if (font != NULL) { BFont normalized = *font; _NormalizeFont(&normalized); font = &normalized; } .... fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode, font, color); }
Probablement, le programmeur avait besoin de normaliser l'objet via une variable intermédiaire. Mais maintenant, dans le pointeur de
police , le pointeur vers l'objet temporaire
normalisé a été
enregistré , qui sera détruit après avoir quitté la portée dans laquelle cet objet temporaire a été créé.
V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 27
int8 BUnicodeChar::Type(uint32 c) { BUnicodeChar(); return u_charType(c); }
Une erreur très courante chez les programmeurs C ++ consiste à utiliser l'appel du constructeur soi-disant pour initialiser / mettre à zéro les champs de classe. Dans ce cas, il n'y a pas de modification des champs de la classe, mais un nouvel objet sans nom de cette classe est créé, qui est immédiatement détruit. Malheureusement, il y a beaucoup de tels endroits:
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 37
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 49
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 58
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 67
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 77
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 89
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 103
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 115
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 126
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 142
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 152
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 163
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 186
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 196
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 206
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 214
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 222
- V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> BUnicodeChar :: BUnicodeChar (....)' doit être utilisé. UnicodeChar.cpp 230
V670 Le membre de classe non initialisé 'fPatternHandler' est utilisé pour initialiser le membre 'fInternal'. N'oubliez pas que les membres sont initialisés dans l'ordre de leurs déclarations à l'intérieur d'une classe. Painter.cpp 184
Painter::Painter() : fInternal(fPatternHandler), .... fPatternHandler(), .... { .... }; class Painter { .... private: mutable PainterAggInterface fInternal;
Un autre exemple d'initialisation incorrecte. Les champs de classe sont initialisés dans l'ordre dans lequel ils sont déclarés dans la classe elle-même. Dans cet exemple, le champ
fInternal sera tout d'abord initialisé à l'aide de la valeur non initialisée
fPatternHandler .
#Définir suspect
V523 L'
instruction 'then' est équivalente à l'instruction 'else'. subr_gtaskqueue.c 191
#define TQ_LOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_lock_spin(&(tq)->tq_mutex); \ else \ mtx_lock(&(tq)->tq_mutex); \ } while (0) #define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex, MA_OWNED) #define TQ_UNLOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_unlock_spin(&(tq)->tq_mutex); \ else \ mtx_unlock(&(tq)->tq_mutex); \ } while (0) void grouptask_block(struct grouptask *grouptask) { .... TQ_LOCK(queue); gtask->ta_flags |= TASK_NOENQUEUE; gtaskqueue_drain_locked(queue, gtask); TQ_UNLOCK(queue); }
L'extrait de code ne semble pas suspect jusqu'à ce que vous regardiez le résultat du préprocesseur:
void grouptask_block(struct grouptask *grouptask) { .... do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex); else mtx_lock(&(queue)->tq_mutex); } while (0); gtask->ta_flags |= 0x4; gtaskqueue_drain_locked(queue, gtask); do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex); else mtx_unlock(&(queue)->tq_mutex); } while (0); }
L'analyseur a vraiment raison - les branches
if et
else sont identiques. Mais où sont
passées les fonctions
mtx_lock_spin et
mtx_unlock_spin ? Les macros
TQ_LOCK ,
TQ_UNLOCK et la fonction
grouptask_block sont déclarées dans le même fichier et presque côte à côte, cependant, il y avait une substitution quelque part.
Une recherche sur le contenu des fichiers n'a trouvé que
mutex.h avec le contenu suivant:
#define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x)
Si une telle substitution est correcte ou non, cela vaut la peine d'être vérifié pour les développeurs du projet. J'ai vérifié le projet sous Linux, et une telle substitution me semblait suspecte.
Erreurs avec la fonction gratuite
V575 Le pointeur nul est passé dans la fonction 'libre'. Inspectez le premier argument. setmime.cpp 727
void MimeType::_PurgeProperties() { fShort.Truncate(0); fLong.Truncate(0); fPrefApp.Truncate(0); fPrefAppSig.Truncate(0); fSniffRule.Truncate(0); delete fSmallIcon; fSmallIcon = NULL; delete fBigIcon; fBigIcon = NULL; fVectorIcon = NULL;
Vous pouvez transmettre un pointeur nul à la fonction
gratuite , mais une telle entrée est clairement suspecte. Ainsi, l'analyseur a trouvé les lignes de code confuses. Tout d'abord, vous avez dû libérer la mémoire à l'aide du pointeur
fVectorIcon , puis uniquement la définir sur
NULL .
V575 Le pointeur nul est passé dans la fonction 'libre'. Inspectez le premier argument. driver_settings.cpp 461
static settings_handle * load_driver_settings_from_file(int file, const char *driverName) { .... handle = new_settings(text, driverName); if (handle != NULL) {
Un autre exemple de passage explicite d'un pointeur nul à la fonction
libre . Cette ligne peut être supprimée, car une fois le pointeur correctement reçu, la fonction se ferme.
V575 Le pointeur nul est passé dans la fonction 'libre'. Inspectez le premier argument. PackageFileHeapWriter.cpp 166
void* _GetBuffer() { .... void* buffer = malloc(fBufferSize); if (buffer == NULL && !fBuffers.AddItem(buffer)) { free(buffer); throw std::bad_alloc(); } return buffer; }
Voici une erreur. Au lieu de l'opérateur &&, l'opérateur || doit être utilisé. Ce n'est que dans ce cas qu'une exception sera levée
std :: bad_alloc () s'il n'était pas possible d'allouer de la mémoire en utilisant la fonction
malloc .
Erreurs avec l'opérateur de suppression
V611 La mémoire a été allouée à l'aide de l'opérateur 'new T []' mais a été libérée à l'aide de l'opérateur 'delete'. Pensez à inspecter ce code. Il vaut probablement mieux utiliser 'delete [] fMsg;'. Err.cpp 65
class Err { public: .... private: char *fMsg; ssize_t fPos; }; void Err::Unset() { delete fMsg;
Le pointeur
fMsg est utilisé pour allouer de la mémoire pour stocker un tableau de caractères, et l'opérateur de
suppression est utilisé pour libérer de la mémoire, au lieu de
supprimer [] .
V611 La mémoire a été allouée à l'aide de l'opérateur "nouveau" mais a été libérée à l'aide de la fonction "libre". Pensez à inspecter les logiques d'opération derrière la variable «wrapperPool». vm_page.cpp 3080
status_t vm_page_write_modified_page_range(....) { .... PageWriteWrapper* wrapperPool = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1]; PageWriteWrapper** wrappers = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages]; if (wrapperPool == NULL || wrappers == NULL) { free(wrapperPool);
Ici
malloc_flags est la fonction qui fait
malloc . Et puis
placement-new construit l'objet ici. Et puisque la classe
PageWriteWrapper est implémentée comme suit:
class PageWriteWrapper { public: PageWriteWrapper(); ~PageWriteWrapper(); void SetTo(vm_page* page); bool Done(status_t result); private: vm_page* fPage; struct VMCache* fCache; bool fIsActive; }; PageWriteWrapper::PageWriteWrapper() : fIsActive(false) { } PageWriteWrapper::~PageWriteWrapper() { if (fIsActive) panic("page write wrapper going out of scope but isn't completed"); }
alors les destructeurs d'objets de cette classe ne seront pas appelés en raison de l'utilisation de la fonction free pour libérer de la mémoire.
V611 La mémoire a été allouée à l'aide de l'opérateur 'new T []' mais a été libérée à l'aide de l'opérateur 'delete'. Pensez à inspecter ce code. Il vaut probablement mieux utiliser 'delete [] fOutBuffer;'. Vérifiez les lignes: 26, 45. PCL6Rasterizer.h 26
class PCL6Rasterizer : public Rasterizer { public: .... ~PCL6Rasterizer() { delete fOutBuffer; fOutBuffer = NULL; } .... virtual void InitializeBuffer() { fOutBuffer = new uchar[fOutBufferSize]; } private: uchar* fOutBuffer; int fOutBufferSize; };
L'utilisation de l'opérateur de
suppression au lieu de
supprimer [] est une erreur très courante. La façon la plus simple de faire une erreur lors de l'écriture d'un cours est Le code destructeur est souvent situé loin des emplacements d'allocation de mémoire. Ici, le programmeur libère de façon incorrecte de la mémoire dans le destructeur par le pointeur
fOutBuffer .
V772 L' appel d'un opérateur de suppression pour un pointeur vide entraînera un comportement indéfini. Hashtable.cpp 207
void Hashtable::MakeEmpty(int8 keyMode,int8 valueMode) { .... for (entry = fTable[index]; entry; entry = next) { switch (keyMode) { case HASH_EMPTY_DELETE:
En plus du mauvais choix entre
supprimer /
supprimer [] et
libre , vous pouvez ajouter un comportement non défini au programme d'une autre manière - essayez d'effacer la mémoire par un pointeur sur un type non défini
(void *) .
Fonctions sans valeur de retour
V591 La fonction non vide doit renvoyer une valeur. Référencable.h 228
BReference& operator=(const BReference<const Type>& other) { fReference = other.fReference; }
L'opérateur d'affectation substitué n'a pas une valeur de retour suffisante. Dans ce cas, l'opérateur retournera une valeur aléatoire, ce qui peut conduire à des erreurs étranges.
Problèmes similaires dans d'autres extraits de code de cette classe:
- V591 La fonction non vide doit renvoyer une valeur. Référencable.h 233
- V591 La fonction non vide doit renvoyer une valeur. Référencable.h 239
V591 La fonction non vide doit renvoyer une valeur. main.c 1010
void errx(int, const char *, ...) ; char * getoptionvalue(const char *name) { struct option *c; if (name == NULL) errx(1, "getoptionvalue() invoked with NULL name"); c = getoption(name); if (c != NULL) return (c->value); errx(1, "getoptionvalue() invoked with unknown option '%s'", name); }
Un commentaire utilisateur NOTREACHED ici ne signifie rien. Pour écrire correctement du code pour de tels scénarios, vous devez baliser des fonctions comme noreturn. Il existe des attributs noreturn pour cela: standard et spécifiques au compilateur. Tout d'abord, de tels attributs sont pris en compte par les compilateurs pour la génération de code correcte ou la notification de certains types d'erreurs à l'aide de warings. Divers outils d'analyse statique prennent également en compte les attributs pour améliorer la qualité de l'analyse.
Gestion des exceptions
V596 L'objet a été créé mais il n'est pas utilisé. Le mot clé «throw» peut être manquant: throw ParseException (FOO); Response.cpp 659
size_t Response::ExtractNumber(BDataIO& stream) { BString string = ExtractString(stream); const char* end; size_t number = strtoul(string.String(), (char**)&end, 10); if (end == NULL || end[0] != '\0') ParseException("Invalid number!"); return number; }
Ici, le mot clé
throw est accidentellement oublié. Ainsi, une
exception ParseException ne sera pas
levée et un objet de cette classe sera simplement détruit lorsqu'il quittera la portée. Après quoi, la fonction continuera son travail comme si rien ne s'était passé, comme si le bon numéro avait été entré.
V1022 Une exception a été levée par le pointeur. Pensez plutôt à le jeter par valeur. gensyscallinfos.cpp 316
int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) {
L'analyseur a détecté une
IOException levée par le pointeur. Lancer un pointeur empêche l'exception d'être interceptée, donc l'exception est finalement interceptée par référence. En outre, l'utilisation d'un pointeur oblige l'intercepteur à appeler l'opérateur de
suppression pour détruire l'objet créé, ce qui n'est pas non plus effectué.
Deux autres domaines problématiques du code:
- V1022 Une exception a été levée par le pointeur. Pensez plutôt à le jeter par valeur. gensyscallinfos.cpp 347
- V1022 Une exception a été levée par le pointeur. Pensez plutôt à le jeter par valeur. gensyscallinfos.cpp 413
Sécurité formelle
V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider l'objet 'f_key'. La fonction memset_s () doit être utilisée pour effacer les données privées. dst_api.c 1018
#ifndef SAFE_FREE #define SAFE_FREE(a) \ do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0) .... #endif DST_KEY * dst_free_key(DST_KEY *f_key) { if (f_key == NULL) return (f_key); if (f_key->dk_func && f_key->dk_func->destroy) f_key->dk_KEY_struct = f_key->dk_func->destroy(f_key->dk_KEY_struct); else { EREPORT(("dst_free_key(): Unknown key alg %d\n", f_key->dk_alg)); } if (f_key->dk_KEY_struct) { free(f_key->dk_KEY_struct); f_key->dk_KEY_struct = NULL; } if (f_key->dk_key_name) SAFE_FREE(f_key->dk_key_name); SAFE_FREE(f_key); return (NULL); }
L'analyseur a détecté du code suspect conçu pour nettoyer en toute sécurité les données privées. Malheureusement, la macro
SAFE_FREE , qui est développée en
memset , appels
gratuits et affectation
NULL , ne rend pas le code plus sûr, car tout cela est supprimé par le compilateur lors de l'optimisation d'
O2 .
Soit dit en passant, cela n'a rien à voir avec
CWE-14 : suppression du code par le compilateur pour effacer les tampons.
La liste complète des endroits où les tampons ne sont pas réellement effacés:
- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider le tampon 'encoded_block'. La fonction memset_s () doit être utilisée pour effacer les données privées. dst_api.c 446
- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider l'objet 'key_st'. La fonction memset_s () doit être utilisée pour effacer les données privées. dst_api.c 685
- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider le tampon 'in_buff'. La fonction memset_s () doit être utilisée pour effacer les données privées. dst_api.c 916
- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider l'objet 'ce'. La fonction memset_s () doit être utilisée pour effacer les données privées. fs_cache.c 1078
Comparaisons non signées
V547 L' expression 'restant <0' est toujours fausse. La valeur du type non signé n'est jamais <0. DwarfFile.cpp 1947
status_t DwarfFile::_UnwindCallFrame(....) { .... uint64 remaining = lengthOffset + length - dataReader.Offset(); if (remaining < 0) return B_BAD_DATA; .... }
L'analyseur a trouvé une comparaison explicite d'une variable non signée avec des valeurs négatives. Vous devriez peut-être comparer la variable
restante avec seulement zéro ou implémenter une vérification de débordement.
V547 L' expression 'sommeil ((non signé) secondes) <0' est toujours fausse. La valeur du type non signé n'est jamais <0. misc.cpp 56
status_t snooze(bigtime_t amount) { if (amount <= 0) return B_OK; int64 secs = amount / 1000000LL; int64 usecs = amount % 1000000LL; if (secs > 0) { if (sleep((unsigned)secs) < 0)
Pour comprendre quelle est l'erreur, tournons-nous vers les signatures des fonctions
sommeil et
sommeil :
extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds);
Comme nous pouvons le voir, la fonction
sleep renvoie une valeur d'un type non signé et son utilisation dans le code est incorrecte.
Pointeurs dangereux
V774 Le pointeur «périphérique» a été utilisé après la libération de la mémoire. xhci.cpp 1572
void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d\n", device->HubPort(), slot);
Un objet
périphérique est effacé par l'opérateur de
suppression . Il s'agit d'une action logique pour une fonction appelée
FreeDevice . Mais, pour une raison quelconque, pour libérer d'autres ressources dans le code, il y a encore un appel à un objet déjà supprimé.
Un tel code est extrêmement dangereux et se produit à plusieurs autres endroits:
- V774 Le pointeur «self» a été utilisé après la libération de la mémoire. TranslatorRoster.cpp 884
- V774 Le pointeur 'chaîne' a été utilisé après la libération de la mémoire. RemoteView.cpp 1269
- V774 Le pointeur «bs» a été utilisé après la libération de la mémoire. mkntfs.c 4291
- V774 Le pointeur «bs» a été utilisé après la libération de la mémoire. mkntfs.c 4308
- V774 Le pointeur 'al' a été utilisé après la réallocation de la mémoire. inode.c 1155
V522 Le déréférencement des données du pointeur nul peut avoir lieu. Le pointeur nul est passé dans la fonction 'malo_hal_send_helper'. Inspectez le troisième argument. Vérifier les lignes: 350, 394. if_malohal.c 350
static int malo_hal_fwload_helper(struct malo_hal *mh, char *helper) { .... error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT);
L'analyse interprocédurale a révélé une situation où
NULL est transmis à la fonction et le pointeur de
données avec cette valeur est ensuite déréférencé dans la fonction
memcpy .
V773 La fonction a été quittée sans libérer le pointeur 'inputFileFile'. Une fuite de mémoire est possible. command_recompress.cpp 119
int command_recompress(int argc, const char* const* argv) { .... BFile* inputFileFile = new BFile; error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY); if (error != B_OK) { fprintf(stderr, "Error: Failed to open input file \"%s\": %s\n", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... }
PVS-Studio peut détecter les fuites de mémoire . La mémoire pour
inputFileFile n'est pas libérée
ici en cas d'erreur. Quelqu'un croit qu'en cas d'erreur, vous ne pouvez pas vous soucier de libérer de la mémoire - le programme se terminera toujours. Mais ce n'est pas toujours le cas. Gérez correctement les erreurs et continuez à travailler - une exigence pour tant de programmes.
V595 Le pointeur 'fR répondre' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 49, 52. ReplyBuilder.cpp 49
RPC::CallbackReply* ReplyBuilder::Reply() { fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus)); fReply->Stream().InsertUInt(fOpCountPosition, fOpCount); if (fReply == NULL || fReply->Stream().Error() == B_OK) return fReply; else return NULL; }
À quelle fréquence les développeurs déréférencent-ils les pointeurs avant de les vérifier? Diagnostics
V595 est presque toujours leader dans le nombre d'avertissements dans un projet. L'utilisation dangereuse du pointeur
fRhness dans ce morceau de code.
V595 Le pointeur 'mq' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifier les lignes: 782, 786. oce_queue.c 782
static void oce_mq_free(struct oce_mq *mq) { POCE_SOFTC sc = (POCE_SOFTC) mq->parent; struct oce_mbx mbx; struct mbx_destroy_common_mq *fwcmd; if (!mq) return; .... }
Un exemple similaire. Le pointeur
mg est déréférencé quelques lignes avant d'être vérifié pour une valeur nulle. Il existe de nombreux endroits similaires dans le projet. À certains endroits, l'utilisation et la vérification des pointeurs sont très éloignées les unes des autres, donc deux exemples sont inclus dans l'article. Les autres pourront voir les développeurs dans le rapport d'analyse complet.
Erreurs diverses
V645 L'appel de fonction 'strncat' pourrait entraîner un débordement de tampon de 'sortie'. Les limites ne doivent pas contenir la taille du tampon, mais un certain nombre de caractères qu'il peut contenir. NamespaceDump.cpp 101
static void dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting) { char output[320]; char tabs[255] = ""; .... strlcat(tabs, "|--- ", sizeof(tabs)); .... while (....) { uint32 type = device->acpi->get_object_type(result); snprintf(output, sizeof(output), "%s%s", tabs, result + depth); switch(type) { case ACPI_TYPE_INTEGER: strncat(output, " INTEGER", sizeof(output)); break; case ACPI_TYPE_STRING: strncat(output, " STRING", sizeof(output)); break; .... } .... } .... }
La différence entre les fonctions
strlcat et
strncat n'est pas entièrement évidente pour une personne nouvelle dans la description de ces fonctions. La fonction
strlcat prend la
taille de la mémoire tampon entière comme troisième argument et la fonction
strncat prend la taille de l'espace libre dans la mémoire tampon , ce qui nécessite de calculer la valeur souhaitée avant d'appeler la fonction. Mais les développeurs l'oublient souvent ou ne le savent pas. Le passage de la fonction
strncat à la taille de la totalité du tampon peut entraîner un débordement de tampon, car la fonction considérera cette valeur comme le nombre de caractères autorisés pour la copie. La fonction
strlcat n'a pas ce problème, mais pour qu'elle fonctionne correctement, il est nécessaire de passer des chaînes qui se terminent par le terminal zéro.
La liste complète des endroits dangereux avec des lignes:
- V645 L'appel de fonction 'strncat' pourrait entraîner un débordement de tampon de 'sortie'. Les limites ne doivent pas contenir la taille du tampon, mais un certain nombre de caractères qu'il peut contenir. NamespaceDump.cpp 104
- V645 L'appel de fonction 'strncat' pourrait entraîner un débordement de tampon de 'sortie'. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141
- V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144
- V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283
- V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284
- V645 L'appel de fonction 'strncat' pourrait conduire au débordement de tampon 'features_string'. Les limites ne doivent pas contenir la taille du tampon, mais un certain nombre de caractères qu'il peut contenir. VirtioDevice.cpp 285
V792 La fonction 'SetDecoratorSettings' située à droite de l'opérateur '|' sera appelé quelle que soit la valeur de l'opérande gauche. Peut-être vaut-il mieux utiliser '||'. DesktopListener.cpp 324 class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> { public: .... virtual bool SetDecoratorSettings(Window* window, const BMessage& settings) = 0; .... }; bool DesktopObservable::SetDecoratorSettings(Window* window, const BMessage& settings) { if (fWeAreInvoking) return false; InvokeGuard invokeGuard(fWeAreInvoking); bool changed = false; for (DesktopListener* listener = fDesktopListenerList.First(); listener != NULL; listener = fDesktopListenerList.GetNext(listener)) changed = changed | listener->SetDecoratorSettings(window, settings); return changed; }
Très probablement, les opérateurs '|' et '||'. Une telle erreur conduit à des appels inutiles à la fonction SetDecoratorSettings .V627 Envisagez d'inspecter l'expression. L'argument de sizeof () est la macro qui se développe en nombre. device.c 72 #define PCI_line_size 0x0c static status_t wb840_open(const char* name, uint32 flags, void** cookie) { .... data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus, data->pciInfo->device, data->pciInfo->function, PCI_line_size, sizeof(PCI_line_size)) & 0xff; .... }
Passer la valeur 0x0c à l'opérateur sizeof semble suspect. Vous devriez peut-être calculer la taille d'un objet, par exemple des données . V562 Il est étrange de comparer une valeur de type booléen avec une valeur de 18: 0x12 == IsProfessionalSpdif (). CEchoGals_mixer.cpp 533 typedef bool BOOL; virtual BOOL IsProfessionalSpdif() { ... } #define ECHOSTATUS_DSP_DEAD 0x12 ECHOSTATUS CEchoGals::ProcessMixerFunction(....) { .... if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() )
La fonction IsProfessionalSpdif renvoie une valeur de type bool , tandis que dans la condition le résultat de la fonction est comparé avec le nombre 0x12 .Conclusion
Nous avons manqué la sortie de la première version bêta de Haiku l'automne dernier, car étaient occupés à publier PVS-Studio pour le langage Java. Mais la nature des erreurs de programmation est telle qu'elles ne vont nulle part à moins que vous ne les recherchiez et ne prêtiez aucune attention à la qualité du code. Les développeurs ont essayé d'utiliser Coverity Scan , mais la dernière exécution de l'analyse remonte à près de deux ans. Cela devrait bouleverser les utilisateurs de Haiku. Bien que l'analyse utilisant Coverity ait été mise en place en 2014, cela ne nous a pas empêché de rédiger deux gros articles avec revue d'erreur en 2015 ( partie 1 , partie 2 ).Ceux qui ont lu jusqu'à la fin attendent une autre révision du code Haiku d'un volume non inférieur avec de nouveaux types d'erreurs. Un rapport d'analyse complet sera envoyé aux développeurs avant de publier les revues de code, afin que certaines erreurs puissent être corrigées. Pour passer le temps entre les publications, je vous suggère de télécharger et d'essayer PVS-Studio sur votre projet.Vous voulez essayer Haiku et vous avez des questions? Les développeurs de Haiku vous invitent sur le
canal télégramme .
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Svyatoslav Razmyslov.
Comment se tirer une balle dans le pied en C et C ++. Haiku OS Cookbook