Siapa pun yang memprogram pengendali mikrokontroler mungkin tahu tentang FreeRTOS, atau setidaknya pernah mendengar tentang sistem operasi ini. Pengembang Amazon memutuskan untuk meningkatkan kemampuan sistem operasi ini untuk bekerja dengan layanan AWS Internet of Things. Ini adalah bagaimana Amazon FreeRTOS muncul. Kami, pengembang alat analisis kode statis PVS-Studio, diminta melalui pos dan dalam komentar untuk memeriksa proyek-proyek ini. Nah, sekarang dapatkan apa yang Anda minta. Teruslah membaca untuk mencari tahu apa yang keluar darinya.
Secara singkat tentang proyek
Untuk memulainya, saya akan memberi tahu Anda sedikit tentang pelopor proyek yang sedang diuji - FreeRTOS (kode sumber tersedia di sini melalui
tautan ). Seperti yang dinyatakan Wikipedia, FreeRTOS adalah sistem operasi multitasking waktu-nyata untuk sistem tertanam.
Ini ditulis dalam C tua yang baik, yang tidak mengherankan - sistem operasi ini harus bekerja dalam kondisi khas mikrokontroler: daya pemrosesan rendah, sejumlah kecil RAM, dan sejenisnya. Bahasa C memungkinkan Anda untuk bekerja dengan sumber daya pada tingkat rendah dan memiliki kinerja tinggi, sehingga sangat cocok untuk mengembangkan OS semacam itu.
Sekarang kembali ke Amazon, yang selalu bergerak mengembangkan berbagai arah yang menjanjikan. Misalnya, Amazon sedang mengembangkan mesin AAA Amazon Lumberyard, yang
juga telah kami
periksa .
Salah satu arahan tersebut adalah Internet of Things (IoT). Untuk mengembangkan di bidang ini, Amazon memutuskan untuk menulis sistem operasi mereka sendiri - dan mereka mengambil inti FreeRTOS sebagai dasar.
Sistem yang dihasilkan, Amazon FreeRTOS, diposisikan untuk "menyediakan koneksi aman ke Amazon Web Services, seperti AWS IoT Core atau AWS IoT Greengrass." Kode sumber proyek
ini tersedia di Github.
Dalam artikel ini, kami akan mencari tahu apakah ada kesalahan dalam FreeRTOS serta seberapa aman sistem operasi Amazon dalam hal analisis kode statis.
Jalannya cek
Pemeriksaan dilakukan menggunakan alat pencari kesalahan otomatis - alat analisa kode statis PVS-Studio. Itu mampu mendeteksi kesalahan dalam program yang ditulis dalam C, C ++, C #, dan Java.
Sebelum analisis, kita harus membangun proyek. Dengan cara ini, saya akan yakin bahwa saya memiliki semua dependensi yang diperlukan dan proyek siap untuk diperiksa. Satu dapat memeriksa proyek dalam sejumlah cara - misalnya, menggunakan sistem pemantauan kompilasi. Dalam hal ini, saya melakukan analisis menggunakan plugin untuk Visual Studio - ada baiknya repositori dari kedua proyek berisi set file proyek yang membuatnya mudah dibangun di bawah Windows.
Saya hanya harus membangun proyek untuk memastikan bahwa saya sudah semuanya siap untuk cek. Selanjutnya saya menjalankan analisis dan - voila! - Saya memiliki laporan analisa yang sudah jadi di depan saya.
Perpustakaan pihak ketiga yang termasuk dalam proyek-proyek ini juga dapat mengandung kesalahan, dan mereka, tentu saja, juga dapat mempengaruhi program. Namun, saya telah mengecualikan mereka dari analisis demi kemurnian narasi.
Jadi, proyek dianalisis, laporan diterima, kesalahan menarik disorot. Sudah waktunya untuk mendapatkan ulasan mereka!
Apa yang disembunyikan FreeRTOS
Awalnya, saya berharap untuk menulis dua artikel terpisah: satu untuk setiap sistem operasi. Saya sudah menggosok tangan saya? ketika saya bersiap untuk menulis artikel yang bagus tentang FreeRTOS. Mengantisipasi penemuan setidaknya beberapa bug berair (seperti
CWE-457 ), saya sedang melihat melalui sedikit peringatan dari penganalisa, dan ... tidak menemukan apa pun. Saya tidak menemukan kesalahan yang menarik.
Banyak peringatan yang dikeluarkan oleh analis tidak relevan dengan FreeRTOS. Sebagai contoh, peringatan tersebut adalah kesalahan 64-bit seperti casting
size_t ke
uint32_t . Ini terkait dengan fakta bahwa FreeRTOS dimaksudkan untuk bekerja pada perangkat dengan ukuran pointer tidak lebih besar dari 32 bit.
Saya telah memeriksa semua peringatan
V1027 dengan seksama yang mengindikasikan pengecoran antara pointer ke struktur yang tidak terkait. Jika struktur yang dicor memiliki keberpihakan yang sama, maka pengecoran semacam itu merupakan kesalahan. Dan saya belum menemukan satu casting berbahaya!
Semua tempat mencurigakan lainnya dikaitkan dengan gaya pengkodean atau dikelola dengan komentar yang menjelaskan mengapa itu dilakukan seperti itu dan mengapa itu bukan kesalahan.
Jadi saya ingin menarik pengembang FreeRTOS. Kawan, kau luar biasa! Kami hampir tidak pernah melihat proyek bersih dan berkualitas tinggi seperti milik Anda. Dan menyenangkan membaca kode yang bersih, rapi, dan terdokumentasi dengan baik. Angkat topi untuk kalian.
Meskipun saya tidak dapat menemukan bug yang menarik hari itu, saya tahu saya tidak akan berhenti di situ. Saya pulang dengan keyakinan bahwa versi Amazon akan 100% memiliki sesuatu yang menarik, dan besok saya pasti akan mengambil cukup banyak bug untuk artikel itu. Seperti yang Anda duga, saya benar.
Apa yang disembunyikan Amazon FreeRTOS
Versi sistem Amazon ternyata ... secara halus, sedikit lebih buruk. Warisan FreeRTOS tetap bersih sementara perbaikan baru menyembunyikan banyak masalah menarik.
Beberapa fragmen memiliki logika program rusak, beberapa pointer ditangani dengan tidak benar. Di beberapa tempat, kode dapat menyebabkan perilaku yang tidak terdefinisi, dan ada kasus-kasus di mana programmer tidak tahu tentang pola kesalahan yang dibuatnya. Saya bahkan menemukan beberapa potensi kerentanan serius.
Sepertinya saya sudah memperketat dengan pendahuluan. Mari kita mulai mencari tahu kesalahan!
Pemecahan logika program
Mari kita mulai dengan tempat-tempat bermasalah yang jelas-jelas menunjukkan bahwa program bekerja tidak seperti yang diharapkan oleh programmer. Penanganan array yang mencurigakan akan didahulukan:
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 mengeluarkan dua peringatan untuk kode ini:
- V619 Array '_requestPool.pRequestDatas' digunakan sebagai penunjuk ke objek tunggal. iot_demo_https_s3_download_async.c 973
- V574 Pointer '_requestPool.pRequestDatas' digunakan secara bersamaan sebagai array dan sebagai pointer ke objek tunggal. Periksa baris: 931, 973. iot_demo_https_s3_download_async.c 973
Untuk jaga-jaga, izinkan saya mengingatkan Anda: nama array adalah pointer ke elemen pertama. Yaitu, jika
_requestPool.pRequestDatas adalah susunan struktur,
_requestPool.pRequestDatas [i] .scheduled adalah nilai untuk anggota yang
dijadwalkan dari struktur
i array. Dan jika kita menulis
_requestPool.pRequestDatas-> terjadwal , maka akan tema ini yaitu struktur array pertama akan diakses.
Dalam kutipan kode di atas, itulah yang terjadi. Di baris terakhir, nilai hanya anggota dari struktur array pertama diubah. Dalam dirinya sendiri, pengaksesan semacam itu sudah mencurigakan, tetapi di sini kasusnya bahkan lebih jelas: array
_requestPool.pRequestDatas dinilai oleh indeks di seluruh badan fungsi. Tetapi pada akhirnya operasi pengindeksan itu dilupakan.
Seperti yang saya pahami, baris terakhir akan terlihat seperti ini:
_requestPool.pRequestDatas[reqIndex].scheduled = true;
Kesalahan berikutnya terletak pada fungsi kecil, jadi saya akan memberikannya sepenuhnya:
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 ],
Peringatan PVS-Studio: V642 [CWE-197] Menyimpan hasil fungsi 'strncmp' di dalam variabel tipe 'pendek' tidak tepat. Bit signifikan bisa hilang melanggar logika program. aws_greengrass_discovery.c 637
Mari kita lihat definisi fungsi strncmp:
int strncmp( const char *lhs, const char *rhs, size_t count );
Dalam contoh, hasil dari tipe
int , yang berukuran 32 bit dikonversi dalam variabel tipe
int16_t . Dengan konversi "penyempitan" ini, bit yang lebih lama dari nilai yang dikembalikan akan hilang. Misalnya, jika fungsi
strncmp mengembalikan
0x00010000 , unit akan hilang selama konversi, dan kondisi akan dieksekusi.
Sebenarnya aneh melihat casting dalam kondisi seperti itu. Mengapa ini diperlukan di sini, jika
int biasa dapat dibandingkan dengan nol? Di sisi lain, jika seorang programmer ingin fungsi ini kadang-kadang mengembalikan
true bahkan jika tidak seharusnya, mengapa tidak mendukung perilaku rumit seperti itu dengan komentar? Tapi dengan cara ini semacam pintu belakang. Lagi pula, saya cenderung berpikir itu adalah kesalahan. Apa yang kamu pikirkan
Perilaku dan petunjuk yang tidak terdefinisi
Inilah contoh besar. Ini menutupi dereference null pointer potensial:
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; } .... }
Peringatan PVS-Studio: V522 [CWE-690] Mungkin ada dereferensi dari penunjuk null potensial 'pCurrentHttpsResponse'. iot_https_client.c 1184
Blok
if terakhir berisi dereferensi bermasalah. Mari cari tahu apa yang terjadi di sini.
Fungsi dimulai dengan variabel
pCurrentHttpsResponse dan
pQItem diinisialisasi oleh nilai
NULL dan variabel
status diinisialisasi oleh nilai
IOT_HTTPS_OK , yang berarti semuanya benar.
Selanjutnya
pQItem diberi nilai, dikembalikan dari fungsi
IotDeQueue_PeekHead , yang mengembalikan pointer ke awal antrian yang ditautkan ganda.
Apa yang terjadi jika antrian kosong? Dalam hal ini, fungsi
IotDeQueue_PeekHead akan mengembalikan
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; }
Selanjutnya kondisi
pQItem == NULL akan menjadi benar dan aliran kontrol akan diteruskan oleh
goto ke bagian bawah fungsi. Pada saat ini, pointer
pCurrentHttpsResponse akan tetap nol, sementara
statusnya tidak akan sama dengan
IOT_HTTPS_OK . Pada akhirnya, kita akan sampai ke cabang
if yang sama, dan ... boom! Nah, Anda tahu tentang konsekuensi dari dereference semacam itu.
Baiklah Itu adalah contoh yang agak rumit. Sekarang saya sarankan agar Anda melihat dereferencing potensial yang sangat sederhana dan dapat dipahami:
int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; uint8_t ucSigComponentLength = pxMbedSignature[ 3 ];
Peringatan PVS-Studio: V595 [CWE-476] Pointer 'pxMbedSignature' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 52, 54. iot_pki_utils.c 52
Fungsi ini menerima pointer ke
uint8_t . Kedua petunjuk diperiksa untuk
NULL , yang merupakan praktik yang baik - situasi seperti itu harus segera dikerjakan.
Tapi inilah masalahnya: pada saat
pxMbedSignature dicentang, itu sudah akan direferensikan secara harfiah satu baris di atas. Ta-daa!
Contoh lain dari kode spekulatif:
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; }
Peringatan PVS-Studio:- V1004 [CWE-628] Pointer 'x51ByteHashOidBuffer' digunakan secara tidak aman setelah diverifikasi terhadap nullptr. Periksa baris: 275, 280. iot_pkcs11.c 280
- V1004 [CWE-628] Pointer 'x32ByteHashedMessage' digunakan secara tidak aman setelah diverifikasi terhadap nullptr. Periksa baris: 275, 281. iot_pkcs11.c 281
Penganalisis memperingatkan bahwa parameter fungsi yang merupakan pointer, tidak aman digunakan setelah mereka memeriksa
NULL . Memang, argumen diperiksa. Tetapi jika salah satu dari mereka bukan
NULL , tidak ada tindakan yang diambil kecuali untuk menulis di
xResult. Bagian kode ini mengatakan: "Ya, jadi argumennya ternyata buruk. Kami akan mencatatnya sekarang, dan Anda - teruskan, teruskan. "
Hasil:
NULL akan diteruskan ke
memcpy. Apa yang bisa terjadi? Di mana nilai-nilai akan disalin dan yang mana? Faktanya, menebak tidak akan membantu, karena standar
dengan jelas menyatakan bahwa panggilan seperti itu mengarah pada perilaku yang tidak terdefinisi (lihat bagian 1).
Ada contoh lain penanganan pointer yang salah dalam laporan analisa yang ditemukan di Amazon FreeRTOS, tetapi saya pikir, contoh yang diberikan cukup untuk menunjukkan kemampuan PVS-Studio dalam mendeteksi kesalahan tersebut. Mari kita lihat sesuatu yang baru.
BENAR! = 1
Ada beberapa kesalahan terkait dengan pola, yang, sayangnya, sering diabaikan.
Faktanya adalah bahwa tipe
bool (dari C ++) berbeda dari tipe
BOOL (umumnya digunakan dalam C). Yang pertama hanya bisa berisi nilai
benar atau
salah . Yang kedua adalah typedef dari tipe integer (
int ,
long , dan lainnya). Nilai
0 adalah "salah" untuk itu, dan nilai lain yang berbeda dari nol adalah "benar".
Karena tidak ada tipe Boolean bawaan di C, konstanta ini didefinisikan untuk kenyamanan:
#define FALSE 0 #define TRUE 1
Mari kita lihat contohnya.
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; }
Peringatan PVS-Studio:- V676 [CWE-253] Tidak benar membandingkan variabel tipe BOOL dengan TRUE. aws_entropy_hardware_poll.c 48
- V676 [CWE-253] Tidak benar membandingkan variabel tipe BOOL dengan TRUE. Ekspresi yang benar adalah: 'FALSE! = CryptGenRandom (hProv, len, output)'. aws_entropy_hardware_poll.c 51
Apakah Anda menemukan kesalahan? Jangan ragu, ada di sini :) Fungsi
CryptAcquireContextA dan
CryptGenRandom adalah fungsi standar dari header
wincrypt.h . Jika berhasil, mereka mengembalikan nilai bukan nol. Izinkan saya menekankan bahwa ini
bukan nol . Jadi, secara teoretis, nilai itu bisa berbeda dari nol:
1 ,
314 ,
42 ,
420 .
Rupanya, programmer yang menulis fungsi dari contoh, tidak memikirkan itu, dan pada akhirnya, nilai yang dihasilkan dibandingkan dengan satu.
Seberapa besar kemungkinan
TRUE == CryptGenRandom (....) kondisi tidak terpenuhi? Sulit dikatakan. Mungkin,
CryptGenRandom mungkin mengembalikan 1 lebih sering daripada nilai-nilai lain, tapi mungkin itu hanya mengembalikan 1. Kita tidak bisa mengetahui ini dengan pasti: implementasi fungsi kriptografi ini disembunyikan dari mata programmer fana :)
Penting untuk diingat bahwa perbandingan semacam itu berpotensi berbahaya. Alih-alih:
if (TRUE == GetBOOL())
Gunakan versi kode yang lebih aman:
if (FALSE != GetBOOL())
Masalah optimasi
Beberapa peringatan dari penganalisa terkait dengan struktur yang beroperasi secara perlahan. Sebagai contoh:
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... }
Peringatan PVS-Studio: V817 Lebih efisien untuk mencari karakter '/' daripada string. iot_demo_https_common.c 205
Ini pendek dan sederhana, bukan? Fungsi
strstr digunakan di sini untuk mencari hanya satu karakter, diteruskan dalam parameter sebagai string (itu dalam tanda kutip ganda).
Tempat ini berpotensi dioptimalkan dengan mengganti
strstr dengan
strchr :
int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... }
Dengan cara ini, pencarian akan bekerja sedikit lebih cepat. Suatu hal yang kecil, tapi menyenangkan.
Yah, optimasi seperti itu bagus, tetapi analisanya juga telah menemukan tempat lain, yang dapat dioptimalkan dengan cara yang jauh lebih terlihat:
void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } }
Peringatan PVS-Studio: V814 Penurunan kinerja. Fungsi 'strlen' disebut beberapa kali di dalam tubuh loop. aws_iot_ota_update_demo.c 235
Hmm ... Di dalam loop, dengan setiap
strlen iterasi disebut yang mengevaluasi panjang garis yang sama setiap kali. Bukan operasi yang paling efektif :)
Mari kita lihat definisi
clientcredentialIOT_THING_NAME :
#define clientcredentialIOT_THING_NAME ""
Pengguna diminta untuk memasukkan nama perangkat mereka di sini. Secara default, ini kosong, dan dalam hal ini, semuanya baik-baik saja. Bagaimana jika pengguna ingin memasukkan nama yang panjang dan indah di sana? Sebagai contoh, saya ingin menyebut gagasan saya "
Mesin Kopi yang Bergairah dan Canggih BarBarista-N061E Edisi Tertinggi ." Bisakah Anda bayangkan seperti apa kejutan saya jika mesin kopi cantik saya mulai bekerja sedikit lebih lambat setelah itu? Gangguan!
Untuk memperbaiki kesalahan, ada baiknya mengambil
strlen di luar body loop. Lagi pula, nama perangkat tidak berubah selama program bekerja. Oh,
constexpr dari C ++ akan cocok di sini ...
Oke, well, jangan menyepuh lily. Seperti kolega saya Andrey Karpov catat, kompiler modern tahu apa itu
strlen dan dia secara pribadi menyaksikan mereka menggunakan konstanta dalam kode biner jika mereka mendapatkan bahwa panjang garis tidak dapat berubah. Jadi ada kemungkinan besar bahwa dalam mode rilis rilis, alih-alih evaluasi panjang garis nyata, nilai pra-evaluasi akan digunakan. Namun, ini tidak selalu berhasil, jadi menulis kode seperti itu bukan praktik yang baik.
Beberapa kata tentang MISRA
Alat analisa PVS-Studio memiliki seperangkat aturan besar untuk memeriksa kode Anda untuk kepatuhan dengan standar MISRA C dan MISRA C. Apa standar ini?
MISRA adalah standar pengkodean untuk sistem tertanam yang sangat bertanggung jawab. Ini berisi seperangkat aturan ketat dan pedoman untuk menulis kode dan menyiapkan proses pengembangan. Aturan-aturan ini cukup banyak, dan mereka ditujukan tidak hanya untuk menghilangkan kesalahan serius, tetapi juga pada berbagai "bau kode". Ini juga bertujuan untuk menulis kode yang paling mudah dipahami dan dibaca.
Dengan demikian, mengikuti standar MISRA tidak hanya membantu menghindari kesalahan dan kerentanan, tetapi juga secara signifikan mengurangi kemungkinan kemunculan mereka dalam kode yang sudah ada.
MISRA digunakan dalam industri dirgantara, medis, otomotif dan militer, di mana kehidupan manusia bergantung pada kualitas perangkat lunak yang disematkan.
Tampaknya, pengembang FreeRTOS Amazon tahu tentang standar ini, dan sebagian besar mengikutinya. Pendekatan semacam itu benar-benar masuk akal: jika Anda menulis OS berbasis luas untuk sistem embedded, maka Anda harus memikirkan keamanan.
Namun, saya telah menemukan banyak pelanggaran terhadap standar MISRA. Saya tidak akan memberikan contoh aturan seperti "jangan gunakan serikat" atau "fungsi seharusnya hanya memiliki satu pengembalian di akhir tubuh" - sayangnya, mereka tidak spektakuler, seperti kebanyakan aturan MISRA. Saya lebih suka memberi Anda contoh-contoh pelanggaran yang berpotensi menimbulkan konsekuensi serius.
Mari kita mulai dengan makro:
#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 ) ) )
Peringatan PVS-Studio:- V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'ms' dari makro 'FreeRTOS_ms_to_tick'. FreeRTOS_IP.h 201
- V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'ulIn' dari makro 'SOCKETS_htonl'. iot_secure_sockets.h 512
- V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'x', 'c' dari makro 'LEFT_ROTATE'. iot_device_metrics.c 90
Ya, itulah yang Anda pikirkan. Parameter makro ini tidak diberi tanda kurung. Jika seseorang secara tidak sengaja menulis sesuatu seperti
val = LEFT_ROTATE(A[i] | 1, B);
"panggilan" makro semacam itu akan berkembang menjadi:
val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) );
Ingat prioritas operasi? Pertama, perubahan bitwise dilakukan, dan hanya setelah itu - bitwise "atau". Karena itu, logika program akan rusak. Contoh yang lebih sederhana: apa yang akan terjadi jika ekspresi "
x + y " dilewatkan di makro
FreeRTOS_ms_to_tick ? Salah satu tujuan utama MISRA adalah untuk mencegah situasi seperti itu.
Beberapa mungkin berpendapat, "Jika Anda memiliki programmer yang tidak tahu tentang ini, tidak ada standar yang dapat membantu Anda!" Saya tidak akan setuju dengan itu. Programmer juga orang, dan tidak peduli seberapa berpengalaman seseorang, mereka juga bisa lelah dan membuat kesalahan di akhir hari. Ini adalah salah satu alasan mengapa MISRA sangat merekomendasikan menggunakan alat analisis otomatis untuk menguji kepatuhan proyek.
Biarkan saya mengatasi pengembang Amazon FreeRTOS: PVS-Studio telah menemukan 12 makro yang lebih tidak aman, jadi Anda perlu berhati-hati dengan mereka :)
Pelanggaran MISRA lain yang menarik:
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)); }
Bisakah Anda menemukan bug sendiri?
Peringatan PVS-Studio: V2537 [MISRA C 2.7] Fungsi tidak boleh memiliki parameter yang tidak digunakan. Pertimbangkan memeriksa parameter: 'rc'. iot_demo_https_s3_upload_async.c 234
Lihatlah lebih dekat: parameter
rc tidak digunakan di mana saja di badan fungsi. Sementara komentar fungsi dengan jelas mengatakan bahwa parameter ini adalah kode pengembalian fungsi lain, dan bahwa itu dapat menandakan kesalahan. Mengapa parameter ini tidak ditangani dengan cara apa pun? Jelas ada yang salah di sini.
Namun, bahkan tanpa komentar seperti itu, parameter yang tidak digunakan sering menunjukkan logika program yang rusak. Kalau tidak, mengapa Anda membutuhkannya di tanda tangan fungsi?
Di sini saya telah memberikan fungsi kecil yang bagus untuk contoh di artikel. Selain itu, saya menemukan 10 parameter lain yang tidak digunakan. Banyak dari mereka digunakan dalam fungsi yang lebih besar, dan tidak mudah untuk dideteksi.
Yang mencurigakan, mereka belum ditemukan sebelumnya. Bagaimanapun, kompiler dengan mudah mendeteksi kasus-kasus seperti itu.
Kesimpulan
Ini tidak semua masalah yang ditemukan oleh penganalisa, tetapi artikel itu ternyata cukup besar. Saya berharap bahwa berkat itu, pengembang FreeRTOS amazon akan dapat memperbaiki beberapa kekurangan, dan bahkan mungkin ingin
mencoba PVS-Studio sendiri. Dengan cara ini akan lebih mudah untuk menyelidiki peringatan secara menyeluruh. Dan faktanya - bekerja dengan antarmuka yang nyaman jauh lebih mudah daripada melihat laporan teks.
Terima kasih telah membaca artikel kami! Sampai jumpa di publikasi berikutnya: D
PS Kebetulan artikel ini diterbitkan pada 31 Oktober. Selamat Halloween, cowok dan cewek!