Geekly articles weekly

Comment se tirer une balle dans le pied en C et C ++. Haiku OS Cookbook

L'histoire de la rencontre entre l'analyseur statique PVS-Studio et le code Haiku OS remonte à l'année 2015. Ce fut une expérience passionnante et une expérience utile pour les équipes des deux projets. Pourquoi l'expérience? À ce moment-là, nous n'avions pas l'analyseur pour Linux et nous ne l'aurions pas avant un an et demi. Quoi qu'il en soit, les efforts des passionnés de notre équipe ont été récompensés: nous nous sommes réunis avec les développeurs de Haiku et avons augmenté la qualité du code, élargi notre base d'erreurs avec de rares bogues créés par les développeurs et affiné l'analyseur. Vous pouvez maintenant vérifier le code Haiku pour les erreurs facilement et rapidement.
Image 1


Présentation


Rencontrez les personnages principaux de notre histoire - le Haiku avec code open source et l'analyseur statique PVS-Studio pour C, C ++, C # et Java. Lorsque nous avons fouillé l'analyse du projet il y a 4,5 ans, nous ne devions traiter que le fichier analyseur exécutable compilé. Toute l'infrastructure d'analyse des paramètres du compilateur, d'exécution d'un préprocesseur, de mise en parallèle de l'analyse, etc., a été extraite de l'utilitaire Compiler Monitoring UI , écrit en C #. Cet utilitaire a été porté en partie sur la plate-forme Mono pour être exécuté sous Linux. Le projet Haiku est construit à l'aide du compilateur croisé sous divers systèmes d'exploitation, à l'exception de Windows. Encore une fois, je voudrais mentionner la commodité et l'exhaustivité de la documentation liées au bâtiment Haiku. J'aimerais également remercier les développeurs de Haiku pour leur aide dans la construction du projet.

Il est beaucoup plus simple d'effectuer l'analyse maintenant. Voici la liste de toutes les commandes pour construire et analyser le projet:

cd /opt git clone https://review.haiku-os.org/buildtools git clone https://review.haiku-os.org/haiku cd ./haiku mkdir generated.x86_64; cd generated.x86_64 ../configure --distro-compatibility official -j12 \ --build-cross-tools x86_64 ../../buildtools cd ../../buildtools/jam make all cd /opt/haiku/generated.x86_64 pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \ -q -j1 @nightly-anyboot pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \ -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12

À propos, l'analyse du projet a été implémentée dans un conteneur Docker. Récemment, nous avons préparé une nouvelle documentation sur ce sujet: Exécution de PVS-Studio dans Docker . Cela peut rendre très facile pour certaines entreprises d'appliquer des techniques d'analyse statique à leurs projets.

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; // <= argpos = 0; if (sigsetjmp(toplevel, 1)) { if (connected) disconnect(0, NULL); if (rval > 0) // <= rval = argpos + 1; return (rval); } .... }

La variable rval n'a pas été initialisée lors de la déclaration, donc sa comparaison avec la valeur nulle conduira à un résultat indéfini. Si les circonstances échouent, la valeur incertaine de la variable rval peut devenir une 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; // <= int error; char c; if (cpp == NULL || lenp == NULL) return -1; if (*cpp != NULL) { switch (res->ai_family) { // <= case AF_INET: if (*lenp < 7) return -1; break; .... } } .... }

Voici un cas similaire d'utilisation de la variable non initialisée, sauf que res est un pointeur non initialisé qui a lieu ici.

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

Le programmeur avait probablement besoin de normaliser l'objet à l'aide d'une variable intermédiaire. Mais maintenant, le pointeur de police contient le pointeur vers l'objet normalisé , qui sera supprimé après avoir quitté la portée, où l'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 / annuler les champs de classe. Dans ce cas, la modification des champs de classe ne se produit pas, mais un nouvel objet sans nom de cette classe est créé puis immédiatement détruit. Malheureusement, il existe de nombreux endroits de ce type dans le projet:

  • 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; // line 336 bool fSubpixelPrecise : 1; bool fValidClipping : 1; bool fDrawingText : 1; bool fAttached : 1; bool fIdentityTransform : 1; Transformable fTransform; float fPenSize; const BRegion* fClippingRegion; drawing_mode fDrawingMode; source_alpha fAlphaSrcMode; alpha_function fAlphaFncMode; cap_mode fLineCapMode; join_mode fLineJoinMode; float fMiterLimit; PatternHandler fPatternHandler; // line 355 mutable AGGTextRenderer fTextRenderer; };

Un autre exemple d'initialisation incorrecte. Les champs de classe sont initialisés dans l'ordre de leur déclaration dans la classe elle-même. Dans cet exemple, le champ fInternal sera le premier à s'initialiser à l'aide de la valeur fPatternHandler non initialisée .

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

Cet 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 - si et sinon les branches sont identiques. Mais où sont 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 un fichier presque côte à côte, mais néanmoins un remplacement a eu lieu quelque part ici.

La recherche dans les fichiers n'a abouti qu'à mutex.h avec le contenu suivant:

 /* on FreeBSD these are different functions */ #define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x)

Les développeurs de projets doivent vérifier si un tel remplacement est correct ou non. J'ai vérifié ce projet sous Linux et un tel remplacement me semblait suspect.

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; // <= free(fVectorIcon); // <= fExtensions.clear(); fAttributes.clear(); }

Vous pouvez passer le pointeur nul dans la fonction gratuite , mais une telle utilisation est certainement suspecte. Ainsi, l'analyseur a trouvé des lignes de code mélangées. Tout d'abord, l'auteur du code a dû libérer la mémoire par le pointeur fVectorIcon , seulement après avoir affecté 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) { // everything went fine! return handle; } free(handle); // <= .... }

Ceci est un autre exemple de passage explicite d'un pointeur nul à la fonction libre . Cette ligne peut être supprimée, car la fonction se ferme après avoir réussi à obtenir le pointeur.

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

Quelqu'un a fait une erreur ici. L'opérateur || doit être utilisé à la place de &&. Seulement dans ce cas, l'exception std :: bad_alloc () sera levée en cas d'échec de l'allocation de mémoire (à l'aide de 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; // <= fMsg = __null; fPos = -1; } void Err::SetMsg(const char *msg) { if (fMsg) { delete fMsg; // <= fMsg = __null; } if (msg) { fMsg = new(std::nothrow) char[strlen(msg)+1]; // <= if (fMsg) strcpy(fMsg, msg); } }

Le pointeur fMsg est utilisé pour allouer de la mémoire à un tableau de caractères. L'opérateur de suppression est utilisé pour libérer 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); // <= free(wrappers); // <= wrapperPool = stackWrappersPool; wrappers = stackWrappers; maxPages = 1; } .... }

Ici malloc_flags est une fonction qui appelle malloc . Ensuite, placement-new construit l'objet ici. Comme la classe PageWriteWrapper est implémentée de la manière suivante:

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

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

C'est une erreur courante d'utiliser l'opérateur de suppression au lieu de supprimer []. Il est plus facile de faire une erreur lors de l'écriture d'une classe, car le code du destructeur est souvent loin des emplacements de mémoire. Ici, le programmeur libère de manière incorrecte la mémoire stockée par le pointeur fOutBuffer dans le destructeur.

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: // TODO: destructors are not called! delete (void*)entry->key; break; case HASH_EMPTY_FREE: free((void*)entry->key); break; } switch (valueMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete entry->value; break; case HASH_EMPTY_FREE: free(entry->value); break; } next = entry->next; delete entry; } .... }

En plus d'un choix incorrect entre supprimer / supprimer [] et gratuit , vous pouvez également rencontrer un comportement non défini lorsque vous essayez d'effacer la mémoire par un pointeur sur le type void (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; }

Un opérateur d'affectation surchargé n'a pas de valeur de retour. Dans ce cas, l'opérateur retournera une valeur aléatoire, ce qui peut conduire à des erreurs étranges.

Voici des problèmes similaires dans d'autres fragments 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); /* NOTREACHED */ }

Le commentaire d'un utilisateur NOTREACHED ne signifie rien ici. Vous devez annoter des fonctions comme noreturn afin d'écrire correctement du code pour de tels scénarios. Pour ce faire, il existe des attributs noreturn: standard et spécifiques au compilateur. Tout d'abord, ces attributs sont pris en compte par les compilateurs pour la génération de code appropriée ou la notification de certains types de bogues à l'aide d'avertissements. 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; }

Le jet de mot-clé a été accidentellement oublié ici. Ainsi, l'exception ParseException ne sera pas générée tandis que l'objet de cette classe sera simplement détruit lors de la sortie de la portée. Après cela, la fonction continuera à fonctionner 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) { // <= fprintf(stderr, "%s\n", exception.what()); return 1; } } int Run(int argc, char** argv) { .... _ParseSyscalls(argv[1]); .... } void _ParseSyscalls(const char* filename) { ifstream file(filename, ifstream::in); if (!file.is_open()) throw new IOException(string("Failed to open '") + filename + "'."); // <= .... }

L'analyseur a détecté l'exception IOException levée par le pointeur. Lancer un pointeur conduit au fait que l'exception ne sera pas interceptée. L'exception est donc finalement interceptée par référence. De plus, l'utilisation d'un pointeur oblige la partie prenante à appeler l'opérateur de suppression pour détruire l'objet créé, ce qui n'avait pas été fait.

Quelques autres fragments de code avec des problèmes:

  • 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, destiné à l'effacement sécurisé des données privées. Malheureusement, la macro SAFE_FREE qui se développe dans le memset , les appels gratuits et l'affectation NULL ne rend pas le code plus sûr, car il est entièrement supprimé par le compilateur lors de l'optimisation avec O2 .

Soit dit en passant, ce n'est rien d'autre que CWE-14 : suppression du code par le compilateur pour effacer les tampons.

Voici la liste des endroits où l'effacement des tampons n'est pas effectué en fait:

  • 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 avec des variables 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 de la variable non signée avec des valeurs négatives. Peut-être, on devrait comparer la variable restante uniquement avec null ou implémenter une vérification de dépassement.

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) // <= return errno; } if (usecs > 0) { if (usleep((useconds_t)usecs) < 0) return errno; } return B_OK; }

Pour obtenir le point principal de l'erreur, examinons les signatures des fonctions de sommeil et d' utilisation :

 extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds);

Comme nous pouvons le voir, la fonction sleep renvoie la valeur non signée 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); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. delete device; DisableSlot(slot); fDcba->baseAddress[slot] = 0; fPortSlots[device->HubPort()] = 0; // <= delete_area(fDevices[slot].trb_area); delete_area(fDevices[slot].input_ctx_area); delete_area(fDevices[slot].device_ctx_area); memset(&fDevices[slot], 0, sizeof(xhci_device)); fDevices[slot].state = XHCI_STATE_DISABLED; }

Un objet périphérique est libéré par l'opérateur de suppression . C'est assez logique pour la fonction FreeDevice . Mais, pour une raison quelconque, pour libérer d'autres ressources, l'objet déjà supprimé est adressé.

Un tel code est extrêmement dangereux et peut être rencontré à 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) { .... /* tell the card we're done and... */ error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL .... } static int malo_hal_send_helper(struct malo_hal *mh, int bsize, const void *data, size_t dsize, int waitfor) { mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD); mh->mh_cmdbuf[1] = htole16(bsize); memcpy(&mh->mh_cmdbuf[4], data , dsize); // <= data .... }

L'analyse interprocédurale a révélé le cas où NULL est passé à la fonction et le pointeur de données avec une telle valeur est finalement 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 . Dans cet exemple, en cas d'erreur, la mémoire ne sera pas libérée. Quelqu'un pourrait penser qu'en cas d'erreur, vous ne devriez pas vous soucier de la libération de la mémoire, car le programme se terminera toujours. Mais ce n'est pas toujours le cas. De nombreux programmes doivent gérer correctement les erreurs et continuer à fonctionner.

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

C'est une erreur très courante de déréférencer les pointeurs avant de les vérifier. Le diagnostic V595 prévaut presque toujours dans le nombre d'avertissements d'un projet. Ce fragment de code inclut une utilisation dangereuse du pointeur fRhness .

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é plusieurs lignes plus tôt qu'il n'est vérifié pour null. Il y a beaucoup d'endroits similaires dans le projet. Dans certains extraits, l'utilisation et la vérification du pointeur sont assez éloignées les unes des autres, donc dans cet article, vous ne trouverez que quelques exemples de ce type. Les développeurs sont invités à consulter d'autres exemples dans le rapport d'analyse complet.

Divers


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 très évidente pour quelqu'un qui n'est pas familier avec la description de ces fonctions. La fonction strlcat attend la taille de la mémoire tampon entière comme troisième argument tandis que la fonction strncat - la taille de l'espace libre dans une mémoire tampon, ce qui nécessite d'évaluer une valeur nécessaire avant d'appeler la fonction. Mais les développeurs l'oublient souvent ou ne le savent pas. Le passage de la taille totale du tampon à la fonction strncat peut entraîner un débordement de tampon, car la fonction considérera cette valeur comme un nombre acceptable de caractères à copier. La fonction strlcat n'a pas un tel problème. Mais vous devez passer des chaînes, se terminant par terminal null pour que cela fonctionne correctement.

Voici la liste complète des endroits dangereux avec des chaînes:

  • 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'. Les limites ne doivent pas contenir la taille du tampon, mais un certain nombre de caractères qu'il peut contenir. NamespaceDump.cpp 107
  • 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 110
  • 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 113
  • 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 118
  • 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 119
  • 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 120
  • 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 123
  • 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 126
  • 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 129
  • 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 132
  • 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 135
  • 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 138
  • 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 141
  • 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 144
  • 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 283
  • 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 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, '|' et '||' les opérateurs étaient confus. Cette erreur entraîne des appels inutiles de 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 /* (1 byte) cache line size in 32 bit words */ 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. L'auteur aurait peut-être dû évaluer 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() ) // <= { Status = ECHOSTATUS_DSP_DEAD; } else { pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif(); } .... }

La fonction IsProfessionalSpdif renvoie la valeur de type bool . Ce faisant, le résultat de la fonction est comparé au nombre 0x12 dans la condition.

Conclusion


Nous avons manqué la sortie de la première version bêta de Haiku l'automne dernier, car nous étions occupés à publier PVS-Studio pour Java. Pourtant, la nature des erreurs de programmation est telle qu'elles ne disparaissent pas si vous ne les recherchez pas et ne faites pas attention à la qualité du code. Les développeurs du projet ont utilisé Coverity Scan , mais la dernière exécution remonte à près de deux ans. Cela doit être bouleversant pour les utilisateurs de Haiku. Même si l'analyse a été configurée en 2014 avec Coverity, cela ne nous a pas empêché d'écrire deux longs articles sur la revue des erreurs en 2015 ( partie 1 , partie 2 )

Un autre examen des erreurs par Haiku sera bientôt publié pour ceux qui liront ce message jusqu'à la fin. Le rapport complet de l'analyseur sera envoyé aux développeurs avant de publier cet examen des erreurs, donc certaines erreurs peuvent être corrigées au moment où vous lisez ceci. Pour passer le temps entre les articles, je vous suggère de télécharger et d'essayer PVS-Studio pour votre projet.

Voulez-vous essayer Haiku et vous avez des questions? Les développeurs de Haiku vous invitent sur le canal télégramme .

Source: https://habr.com/ru/post/fr461253/

More articles:


All Articles