Tous ceux qui programment des microcontrôleurs connaissent probablement FreeRTOS, ou du moins ont entendu parler de ce système d'exploitation. Les gars d'Amazon ont décidé d'étendre les capacités de ce système d'exploitation pour travailler avec les services AWS Internet of Things - c'est ainsi qu'Amazon FreeRTOS est apparu. Nous, les développeurs de l'analyseur de code PVS-Studio, avons été invités à vérifier ces projets dans l'e-mail et dans les commentaires ci-dessous les articles. Eh bien, vous avez demandé - nous l'avons fait. Ce qui en est ressorti - lisez la suite.
Un peu sur les projets
Pour commencer, je vais vous parler un peu du «papa» du projet vérifié - FreeRTOS (vous pouvez trouver le code source
ici ). Comme le dit Wikipedia, FreeRTOS est un système d'exploitation multitâche en temps réel pour les systèmes embarqués.
Il a été écrit dans le bon vieux C, ce qui n'est pas surprenant - ce système d'exploitation devrait fonctionner dans des conditions typiques des microcontrôleurs: faible puissance de calcul, une petite quantité de RAM, etc. Le langage C vous permet de travailler avec des ressources à un faible niveau et a des performances élevées, il est donc le mieux adapté pour le développement d'un tel système d'exploitation.
Revenons maintenant à Amazon, qui ne reste pas immobile et se développe dans divers domaines prometteurs. Par exemple, Amazon développe le moteur AAA du jeu Amazon Lumberyard, que nous avons
également testé .
L'un de ces domaines est l'Internet des objets (Internet des objets, IoT). Pour se développer dans ce domaine, Amazon a décidé d'écrire son propre système d'exploitation - et ils ont pris le noyau FreeRTOS comme base.
Le système résultant - Amazon FreeRTOS - se positionne comme «offrant la possibilité de se connecter en toute sécurité à Amazon Web Services, comme AWS IoT Core ou AWS IoT Greengrass». Le code source de ce projet
est stocké sur Github.
Dans cet article, nous examinerons s'il existe des erreurs dans FreeRTOS, ainsi que la sécurité du système d'exploitation d'Amazon en termes d'analyse de code statique.
Comment était le chèque
Le code a été vérifié à l'aide d'un outil de recherche d'erreur automatique: l'analyseur de code statique PVS-Studio. Il est capable de détecter les erreurs dans les programmes écrits en C, C ++, C # et Java.
Avant de commencer l'analyse, vous devez assembler le projet - je serai donc sûr que j'ai toutes les dépendances nécessaires et que tout est en ordre avec le projet. Il existe plusieurs façons de vérifier un projet - par exemple, en utilisant un système de surveillance de compilation. J'ai effectué l'analyse à l'aide du plug-in pour Visual Studio - il est bon que les référentiels des deux projets disposent d'un ensemble de fichiers de projet qui facilitent la création sous Windows.
Tout ce qu'il me fallait, c'était de collecter les projets pour m'assurer qu'il y avait tout le nécessaire pour la vérification. Ensuite, j'ai lancé l'analyse et le tour est joué! - devant moi est un rapport d'analyseur prêt à l'emploi.
Les bibliothèques tierces incluses dans ces projets peuvent également contenir des erreurs et, bien sûr, elles peuvent également affecter le fonctionnement du programme. Cependant, je les ai exclus de l'analyse dans un souci de pureté de la narration.
Ainsi, les projets sont analysés, des rapports sont reçus, des erreurs intéressantes sont écrites. Il est temps de passer à leur analyse!
Ce qui cache FreeRTOS
Au départ, je m'attendais à écrire deux articles distincts: un pour chaque système d'exploitation. J'ai déjà frotté mes mains, me préparant à écrire un bon article sur FreeRTOS. Anticipant la détection d'au moins quelques erreurs juteuses (comme
CWE-457 ), j'ai regardé avec impatience les quelques avertissements de l'analyseur, et ... et rien. Je n'ai trouvé aucune erreur intéressante.
De nombreux avertissements émis par l'analyseur n'étaient pas pertinents pour FreeRTOS. Par exemple, ces avertissements étaient des lacunes 64 bits comme le
cast de
size_t en
uint32_t . Cela est dû au fait que FreeRTOS est conçu pour fonctionner sur des appareils dont la taille du pointeur ne dépasse pas 32 bits.
J'ai soigneusement vérifié tous les avertissements
V1027 liés aux
lancements entre pointeurs vers des structures indépendantes. Si les structures réductibles ont le même alignement, une telle fonte n'est pas une erreur. Et je n'ai trouvé aucun casting dangereux!
Tous les autres endroits suspects étaient liés au style de codage ou étaient équipés d'un commentaire expliquant pourquoi cela se fait ici exactement et pourquoi ce n'est pas une erreur.
En général, je souhaite contacter les développeurs de FreeRTOS. Vous êtes vraiment super! Nous n'avons presque jamais rencontré de projets aussi propres et de haute qualité que le vôtre. Et j'ai été très heureux de lire du code propre, bien rangé et bien documenté. Chapeau à toi.
Bien que je n'aie pas trouvé d'erreurs intéressantes ce jour-là, j'ai compris que je ne m'arrêterais pas là. Je rentrais chez moi avec la ferme conviction que quelque chose d'intéressant se trouverait dans la version d'Amazon à 100%, et que demain je collecterais certainement suffisamment d'erreurs pour l'article. Comme vous l'avez probablement deviné, j'avais raison.
Ce qui cache Amazon FreeRTOS
La version d'Amazon du système s'est avérée être ... pour le dire légèrement, un peu pire. L'héritage de FreeRTOS est resté tout aussi propre, mais les nouvelles révisions se sont avérées assez intéressantes.
Dans certains endroits, la logique du programme a été violée, quelque part mal travaillée avec des pointeurs. À certains endroits, le code pouvait conduire à un comportement indéfini, mais quelque part le programmeur ne savait tout simplement pas le motif d'erreur qu'il avait fait. J'ai même trouvé de sérieuses vulnérabilités potentielles.
Quelque chose que j'ai retardé avec l'introduction. Commençons à analyser les bogues!
Violation de la logique du programme
Commençons par les zones à problèmes, qui indiquent clairement que le programme ne fonctionne pas exactement comme le programmeur s'y attendait. Le premier de ces endroits sera un travail suspect avec un tableau:
static _requestPool_t _requestPool = { 0 }; .... static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { .... _requestPool.pRequestDatas[reqIndex].pConnHandle = &_connHandle; _requestPool.pRequestDatas[reqIndex].pConnConfig = &_connConfig; _requestPool.pRequestDatas[reqIndex].reqNum = reqIndex; _requestPool.pRequestDatas[reqIndex].currRange = currentRange; _requestPool.pRequestDatas[reqIndex].currDownloaded = 0; _requestPool.pRequestDatas[reqIndex].numReqBytes = numReqBytes; .... _requestPool.pRequestDatas->scheduled = true; .... }
PVS-Studio a émis deux avertissements pour ce morceau de code:
- V619 Le tableau '_requestPool.pRequestDatas' est utilisé comme pointeur vers un seul objet. iot_demo_https_s3_download_async.c 973
- V574 Le pointeur '_requestPool.pRequestDatas' est utilisé simultanément comme tableau et comme pointeur vers un seul objet. Vérifiez les lignes: 931, 973. iot_demo_https_s3_download_async.c 973
Au cas où, laissez-moi vous rappeler: le nom du tableau est un pointeur sur son premier élément. Autrement dit, si
_requestPool.pRequestDatas est un tableau de structures, alors
_requestPool.pRequestDatas [i] .scheduled est l'accès au membre
planifié de la
i- ème structure du tableau. Et si vous écrivez
_requestPool.pRequestDatas-> planifié , cela signifie l'accès au membre
planifié de la première structure du tableau.
C'est ce qui se passe dans l'extrait de code ci-dessus. La dernière ligne modifie toujours la valeur uniquement pour un membre de la première structure du tableau. En soi, cet appel est déjà suspect, mais la situation ici est encore plus évidente: dans tout le corps de la fonction, le tableau
_requestPool.pRequestDatas est accessible par index, et ce n'est qu'à la fin de l'opération d'indexation que l'on a oublié d'appliquer.
Si je comprends bien, la dernière ligne devrait ressembler à ceci:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
L'erreur suivante réside dans une petite fonction, je vais donc la donner dans son intégralité:
static BaseType_t prvGGDJsoneq( const char * pcJson, const jsmntok_t * const pxTok, const char * pcString ) { uint32_t ulStringSize = ( uint32_t ) pxTok->end - ( uint32_t ) pxTok->start; BaseType_t xStatus = pdFALSE; if( pxTok->type == JSMN_STRING ) { if( ( uint32_t ) strlen( pcString ) == ulStringSize ) { if( ( int16_t ) strncmp( &pcJson[ pxTok->start ],
Avertissement PVS-Studio: V642 [CWE-197] L'enregistrement du résultat de la fonction 'strncmp' dans la variable de type 'short' est inapproprié. Les bits significatifs pourraient être perdus en brisant la logique du programme. aws_greengrass_discovery.c 637
Jetons un coup d'œil à la définition de la fonction strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
Dans l'exemple, un résultat de type
int , dont la taille est de 32 bits, est converti en une variable de type
int16_t . Avec une telle conversion de "rétrécissement", les bits les plus significatifs de la valeur de retour seront perdus. Par exemple, si la fonction
strncmp renvoie
0x00010000 , celle-
ci sera
perdue lors de la conversion et la condition sera satisfaite.
En fait, il est étrange de voir un tel casting dans un état. Pourquoi même le faire si vous pouvez comparer un
int normal à zéro? D'un autre côté, si le programmeur voulait consciemment que la fonction retourne
vrai , même si ce n'est pas le cas, alors pourquoi ce comportement délicat n'est-il pas décrit par le commentaire? Mais c'est déjà un signet. En général, je suis porté à croire que c'est une erreur. Que pensez-vous?
Comportement et pointeurs indéfinis
Maintenant, il y aura un exemple assez important. Il masque le déréférencement potentiel d'un pointeur nul:
static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL; .... IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex)); if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true; status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; } .... iotCleanup : if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); } pCurrentHttpsResponse->syncStatus = status; } .... }
Avertissement PVS-Studio: V522 [CWE-690] Il peut y avoir un déréférencement d'un pointeur nul potentiel 'pCurrentHttpsResponse'. iot_https_client.c 1184
Les déréférences de problème sont tout en bas
si . Voyons ce qui se passe ici.
Au début de la fonction, les
variables pCurrentHttpsResponse et
pQItem sont initialisées à
NULL et la variable d'
état est
initialisée à
IOT_HTTPS_OK , ce qui signifie que tout se passe bien.
Ensuite,
pQItem reçoit la valeur renvoyée par la fonction
IotDeQueue_PeekHead , qui renvoie un pointeur sur le début d'une file d'attente doublement connectée.
Que se passe-t-il si la file d'attente est vide? Dans ce cas, la fonction
IotDeQueue_PeekHead retournera
NULL :
static inline IotLink_t* IotDeQueue_PeekHead (const IotDeQueue_t* const pQueue) { return IotListDouble_PeekHead(pQueue); } .... static inline IotLink_t* IotListDouble_PeekHead (const IotListDouble_t* const pList) { IotLink_t* pHead = NULL; if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } } return pHead; }
Ensuite, la condition
pQItem == NULL est satisfaite et le flux de contrôle
passe par
goto à la partie inférieure du corps de la fonction. À ce moment, le pointeur
pCurrentHttpsResponse restera nul et le
statut ne
sera plus égal à
IOT_HTTPS_OK . En conséquence, nous tomberons dans cette même branche
si , et ... larges! Vous connaissez vous-même les conséquences de ce déréférencement.
Ok C'était un exemple légèrement orné. J'attire maintenant votre attention sur un déréférencement potentiel très simple et compréhensible:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
Avertissement PVS-Studio: V595 [CWE-476] Le pointeur 'pxMbedSignature' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 52, 54. iot_pki_utils.c 52
Cette fonction obtient deux pointeurs vers
uint8_t . Les deux pointeurs sont vérifiés pour
NULL , ce qui est une bonne pratique - de telles situations doivent être résolues immédiatement.
Mais voici la malchance: au moment où
pxMbedSignature est vérifié, il sera déjà déréférencé littéralement une ligne au-dessus. Ta-daa!
Un autre exemple de code spéculatif:
CK_RV vAppendSHA256AlgorithmIdentifierSequence ( uint8_t * x32ByteHashedMessage, uint8_t * x51ByteHashOidBuffer ) { CK_RV xResult = CKR_OK; uint8_t xOidSequence[] = pkcs11STUFF_APPENDED_TO_RSA_SIG; if( ( x32ByteHashedMessage == NULL ) || ( x51ByteHashOidBuffer == NULL ) ) { xResult = CKR_ARGUMENTS_BAD; } memcpy( x51ByteHashOidBuffer, xOidSequence, sizeof( xOidSequence ) ); memcpy( &x51ByteHashOidBuffer[ sizeof( xOidSequence ) ], x32ByteHashedMessage, 32 ); return xResult; }
Avertissements de PVS-Studio:- V1004 [CWE-628] Le pointeur 'x51ByteHashOidBuffer' a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] Le pointeur 'x32ByteHashedMessage' a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 275, 281. iot_pkcs11.c 281
L'analyseur avertit que les paramètres de fonction qui sont des pointeurs sont utilisés de manière non sécurisée après avoir été testés pour
NULL . En effet, les arguments sont vérifiés, mais si l'un d'eux s'avère être
NULL , aucune action n'est entreprise, sauf pour écrire dans
xResult . Ce morceau de code semble dire: «Oui, cela signifie que les arguments se sont avérés mauvais. Nous allons l'écrire maintenant, pendant que vous continuez, continuez. "
Conclusion :
NULL sera transmis à
memcpy . Que peut-il en résulter? Où les valeurs seront-elles copiées et lesquelles? En fait, deviner cela ne vaut pas la peine, car la norme
indique clairement qu'un tel appel conduit à un comportement indéfini (voir paragraphe 1).
Le rapport de l'analyseur contient toujours des exemples de fonctionnement incorrect avec des pointeurs que j'ai trouvés dans Amazon FreeRTOS, mais je pense que les exemples donnés sont déjà suffisants pour vous montrer les capacités de PVS-Studio à détecter de telles erreurs. Considérez quelque chose de nouveau.
VRAI! = 1
Plusieurs erreurs que j'ai trouvées étaient liées à un modèle, qui, malheureusement, est souvent oublié.
Le fait est que le type
bool (de C ++) est différent du type
BOOL (habituellement utilisé en C). Le premier ne peut contenir que
vrai ou
faux . Le second est un typedef d'un type entier (
int ,
long , etc.). Pour lui, «faux» est la valeur
0 et «vrai» est toute valeur autre que zéro.
Puisqu'il n'y a pas de type booléen intégré en C, ces constantes sont définies par commodité:
#define FALSE 0 #define TRUE 1
Prenons maintenant un exemple:
int mbedtls_hardware_poll(void* data, unsigned char* output, size_t len, size_t* olen) { int lStatus = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; HCRYPTPROV hProv = 0; (void)data; if (TRUE == CryptAcquireContextA( &hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) { if (TRUE == CryptGenRandom(hProv, len, output)) { lStatus = 0; *olen = len; } CryptReleaseContext(hProv, 0); } return lStatus; }
Avertissements de PVS-Studio:- V676 [CWE-253] Il est incorrect de comparer la variable de type BOOL avec TRUE. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Il est incorrect de comparer la variable de type BOOL avec TRUE. L'expression correcte est: «FAUX! = CryptGenRandom (hProv, len, output)». aws_entropy_hardware_poll.c 51
Vous avez trouvé une erreur? Et c'est :) Fonctions
CryptAcquireContextA et
CryptGenRandom sont des fonctions standard de l'en
- tête
wincrypt.h . En cas de succès, ils renvoient une valeur différente de zéro. J'insiste -
non nul . Donc, théoriquement, cela peut être n'importe quelle valeur autre que zéro:
1 ,
314 ,
42 ,
420 .
Apparemment, le programmeur qui a écrit la fonction de l'exemple n'y a pas pensé, et par conséquent, les valeurs obtenues sont comparées à l'unité.
Avec quelle probabilité la condition
VRAI == CryptGenRandom (....) n'est-elle pas remplie? C'est difficile à dire. Peut-être que
CryptGenRandom renvoie une unité plus souvent que les autres valeurs, et peut-être qu'il n'en renvoie toujours qu'une seule. Nous ne pouvons pas le savoir avec certitude: l'implémentation de cette fonction cryptographique est cachée aux yeux des programmeurs mortels :)
Il est important de se rappeler que de telles comparaisons sont potentiellement dangereuses. Et au lieu de:
if (TRUE == GetBOOL())
utilisez une option plus sûre:
if (FALSE != GetBOOL())
Problèmes d'optimisation
Plusieurs avertissements de l'analyseur ont été associés à des constructions à exécution lente. Par exemple:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
Avertissement PVS-Studio: V817 Il est plus efficace de rechercher un caractère '/' plutôt qu'une chaîne. iot_demo_https_common.c 205
Brièvement et clairement, n'est-ce pas? La fonction
strstr ici est utilisée pour rechercher un seul caractère, qui est passé au paramètre sous forme de chaîne (il est placé entre guillemets).
Cet endroit peut être potentiellement optimisé en remplaçant
strstr par
strchr :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
Ensuite, la recherche fonctionnera un peu plus rapidement. Un peu, mais sympa.
De telles optimisations sont, bien sûr, bonnes, et l'analyseur a trouvé un autre endroit qui peut être optimisé beaucoup plus sensiblement:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Avertissement PVS-Studio: V814 Baisse des performances. La fonction 'strlen' a été appelée plusieurs fois à l'intérieur du corps d'une boucle. aws_iot_ota_update_demo.c 235
Hmmm ... A l'intérieur de la boucle, à chaque itération,
strlen est appelé, qui calcule à chaque fois la longueur de la même ligne. Pas l'opération la plus efficace :)
Jetons un coup d'œil à la définition de
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
L'utilisateur est invité à saisir ici le nom de son appareil. Par défaut, il est vide, et dans ce cas, tout va bien. Mais que se passe-t-il si l'utilisateur veut y entrer un nom long et beau? Par exemple, j'aimerais appeler mon idée originale "
La machine à café passionnée et sophistiquée BarBarista-N061E The Ultimate Edition ". Pouvez-vous imaginer quelle serait ma surprise si après cela ma belle machine à café commençait à fonctionner un peu plus lentement? Mess!
Pour corriger l'erreur,
strlen doit être retiré du corps de la boucle. Après tout, le nom de l'appareil ne change pas pendant l'exécution du programme. Ehhh, ici serait
constexpr de C ++ ...
D'accord, d'accord, j'avoue: ici j'étais un peu épaissi. Comme mon collègue Andrei Karpov l'a noté, les compilateurs modernes savent ce qu'est un
strlen et il a personnellement observé comment ils utilisent simplement une constante en code binaire, s'ils comprennent que la longueur de la chaîne ne peut pas changer. Il y a donc une forte probabilité que dans le mode de construction de la version finale, au lieu du calcul réel de la longueur de la chaîne, une valeur précédemment calculée sera simplement utilisée. Cependant, cela ne fonctionne pas toujours, donc l'écriture d'un tel code n'est pas une bonne pratique.
Quelques mots sur MISRA
L'analyseur PVS-Studio dispose d'un large ensemble de règles qui vous permettent de vérifier la conformité de votre code avec les normes MISRA C et MISRA C ++. Quelles sont ces normes?
MISRA est la norme de codage pour les systèmes embarqués hautement réactifs. Il contient un ensemble de règles et de directives strictes pour l'écriture de code et la configuration du processus de développement. Il existe un certain nombre de ces règles, qui visent non seulement à éliminer les erreurs graves, mais aussi à diverses "odeurs de code", ainsi qu'à écrire le code maximum clair et lisible.
Ainsi, suivre la norme MISRA permet non seulement d'éviter les erreurs et les vulnérabilités, mais aussi de manière significative - significative! - réduire la probabilité de leur occurrence dans un code existant.
MISRA est utilisé dans les industries aérospatiale, médicale, automobile et militaire - où la vie humaine dépend de la qualité du logiciel embarqué.
Apparemment, les développeurs d'Amazon FreeRTOS connaissent cette norme et la suivent pour la plupart. C'est vrai: si vous écrivez un système d'exploitation large pour les systèmes embarqués, vous devez penser à la sécurité.
Cependant, j'ai trouvé pas mal de violations de la norme MISRA. Je ne donnerai pas ici d'exemples de règles comme «ne pas utiliser l'union» ou «une fonction ne devrait avoir qu'un seul retour à la fin du corps» - malheureusement, elles ne sont pas spectaculaires, comme la plupart des règles MISRA. Je ferais mieux de vous donner des exemples de violations qui pourraient potentiellement entraîner des conséquences graves.
Commençons par les macros:
#define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 )
#define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) )
#define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) )
Avertissements de PVS-Studio:- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter le paramètre «ms» de la macro «FreeRTOS_ms_to_tick». FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter le paramètre 'ulIn' de la macro 'SOCKETS_htonl'. iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] La macro et ses paramètres doivent être placés entre parenthèses. Pensez à inspecter les paramètres «x», «c» de la macro «LEFT_ROTATE». iot_device_metrics.c 90
Oui, c'est exactement ce que tu pensais. Les paramètres de ces macros ne sont pas placés entre crochets. Si quelqu'un écrit accidentellement quelque chose comme
val = LEFT_ROTATE(A[i] | 1, B);
alors une telle macro «appel» s'ouvrira dans:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
Rappelez-vous les priorités des opérations? Tout d'abord, un décalage au niveau du bit est effectué, et seulement après, il s'agit d'un «ou» au niveau du bit. Par conséquent, la logique du programme sera violée. Un exemple plus simple: que se passe-t-il si l'expression "
x + y " est passée à la macro
FreeRTOS_ms_to_tick ? L'un des principaux objectifs de MISRA est de prévenir la survenue de telles situations.
Quelqu'un peut objecter: "si vous avez des programmeurs qui ne connaissent pas cela, alors aucune norme ne vous sauvera!" Et je ne serai pas d'accord avec cela. Les programmeurs sont aussi des personnes, et quelle que soit l'expérience d'une personne, elle aussi peut se fatiguer et faire une erreur à la fin de la journée de travail. C'est l'une des raisons pour lesquelles MISRA recommande fortement l'utilisation d'outils d'analyse automatisés pour valider la conformité d'un projet avec la norme.
Je me tourne vers les développeurs d'Amazon FreeRTOS: PVS-Studio a trouvé 12 macros plus dangereuses, vous êtes donc plus prudent avec elles :)
Une autre violation intéressante de la MISRA:
static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData; if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; } IotSemaphore_Post(&(_uploadFinishedSem)); }
Pouvez-vous trouver l'erreur vous-même?
Avertissement PVS-Studio: V2537 [MISRA C 2.7] Les fonctions ne doivent pas avoir de paramètres inutilisés. Pensez à inspecter le paramètre: «rc». iot_demo_https_s3_upload_async.c 234
Regardez de plus près: le paramètre
rc n'est utilisé nulle part dans le corps de la fonction. De plus, dans le commentaire de la fonction, il est explicitement écrit que ce paramètre est le code retour d'une autre fonction, et qu'il peut signaler une erreur. Alors pourquoi ce paramètre n'est-il traité d'aucune façon? Il y a clairement quelque chose qui cloche ici.
Cependant, même sans ces commentaires, les paramètres inutilisés indiquent souvent une logique de programme cassée. Sinon, pourquoi sont-ils nécessaires dans la signature de fonction?
Ici, j'ai donné une petite fonction qui convient bien à un exemple dans l'article. En plus d'elle, j'ai trouvé 10 autres paramètres inutilisés. Beaucoup d'entre eux sont utilisés dans des fonctions plus importantes, et les trouver n'est pas la chose la plus simple.
Il est suspect qu'ils n'aient pas été retrouvés auparavant. En effet, les compilateurs peuvent facilement détecter de tels cas.
Conclusion
Ce n'étaient pas tous les problèmes rencontrés par l'analyseur, mais l'article était déjà assez volumineux. J'espère que grâce à cela, les développeurs d'Amazon FreeRTOS pourront corriger certaines lacunes, et voudront peut-être même
essayer PVS-Studio par eux-mêmes. De cette façon, il sera possible d'étudier les avertissements de manière plus approfondie, et en effet, travailler avec une interface pratique est beaucoup plus facile que de regarder un rapport texte.
Merci d'avoir lu nos articles! Rendez-vous dans le prochain numéro: D
PS Il se trouve que cet article a été publié le 31 octobre. Par conséquent, je souhaite à tous un joyeux Halloween!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: George Gribkov.
À la demande des développeurs intégrés: détection des erreurs dans Amazon FreeRTOS .