Disallow VLAs in Matter code by adding -Wvla to our strict warnings c…

…onfig. We suppress the warning in two places, both tracked by existing issues.
project-chip · Sep 3, 2021 · 5d60f28 · 5d60f28
1 parent d716f27
commit 5d60f28
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 17 deletions.
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
@@ -200,6 +200,7 @@ config("strict_warnings") {
     "-Wextra",
     "-Wshadow",
     "-Wunreachable-code",
+    "-Wvla",
   ]
 
   cflags_cc = [ "-Wnon-virtual-dtor" ]

diff --git a/src/app/zap-templates/templates/app/CHIPClientCallbacks-src.zapt b/src/app/zap-templates/templates/app/CHIPClientCallbacks-src.zapt
@@ -353,7 +353,11 @@ void {{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}ListAttribu
 
     CHECK_MESSAGE_LENGTH_VOID(2);
     uint16_t count = Encoding::LittleEndian::Read16(message);
+    {{! TODO: This VLA is not OK.  See https://github.com/project-chip/connectedhomeip/issues/8558 }}
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wvla"
     {{chipType}} data[count];
+#pragma GCC diagnostic pop
     for (size_t i = 0; i < count; i++)
     {
         {{#if isStruct}}

diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp
@@ -555,9 +555,11 @@ static void TestAES_CCM_128DecryptInvalidCipherText(nlTestSuite * inSuite, void
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, 0, vector->aad, vector->aad_len, vector->tag, vector->tag_len, vector->key,
-                                             vector->key_len, vector->iv, vector->iv_len, out_pt);
+                                             vector->key_len, vector->iv, vector->iv_len, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }
@@ -575,9 +577,11 @@ static void TestAES_CCM_128DecryptInvalidKey(nlTestSuite * inSuite, void * inCon
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, vector->ct_len, vector->aad, vector->aad_len, vector->tag, vector->tag_len,
-                                             nullptr, 0, vector->iv, vector->iv_len, out_pt);
+                                             nullptr, 0, vector->iv, vector->iv_len, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }
@@ -595,9 +599,11 @@ static void TestAES_CCM_128DecryptInvalidIVLen(nlTestSuite * inSuite, void * inC
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, vector->ct_len, vector->aad, vector->aad_len, vector->tag, vector->tag_len,
-                                             vector->key, vector->key_len, vector->iv, 0, out_pt);
+                                             vector->key, vector->key_len, vector->iv, 0, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }

diff --git a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm
@@ -84,8 +84,8 @@ static BOOL isRunningTests(void)
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    uint16_t idStringLen = 16;
-    char issuerIdString[idStringLen];
+    char issuerIdString[16];
+    uint16_t idStringLen = sizeof(issuerIdString);
     if (CHIP_NO_ERROR != storage->SyncGetKeyValue(CHIP_COMMISSIONER_CA_ISSUER_ID, issuerIdString, idStringLen)) {
         mIssuerId = arc4random();
         CHIP_LOG_ERROR("Assigned %d certificate issuer ID to the commissioner", mIssuerId);

diff --git a/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt b/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt
@@ -88,11 +88,11 @@ public:
         CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCallbackBridge * callback = reinterpret_cast<CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCallbackBridge *>(context);
         if (callback && callback->mQueue)
         {
-            id values[count];
+            id array = [NSMutableArray arrayWithCapacity:count];
             for (uint16_t i = 0; i < count; i++)
             {
                 {{#if isStruct}}
-                values[i] = [[NSDictionary alloc] initWithObjectsAndKeys:
+                array[i] = [[NSDictionary alloc] initWithObjectsAndKeys:
                 {{#chip_attribute_list_entryTypes}}
                     {{#if (isString type)}}
                     {{#if (isOctetString type)}}
@@ -109,17 +109,16 @@ public:
                 {{else}}
                 {{#if (isString type)}}
                 {{#if (isOctetString type)}}
-                values[i] = [NSData dataWithBytes:entries[i].data() length:entries[i].size()];
+                array[i] = [NSData dataWithBytes:entries[i].data() length:entries[i].size()];
                 {{else}}
-                values[i] = [[NSString alloc] initWithBytes:entries[i]+{{asReadTypeLength type}} length:emberAf{{asReadType type}}Length(entries[i]) encoding:NSUTF8StringEncoding];
+                array[i] = [[NSString alloc] initWithBytes:entries[i]+{{asReadTypeLength type}} length:emberAf{{asReadType type}}Length(entries[i]) encoding:NSUTF8StringEncoding];
                 {{/if}}
                 {{else}}
-                values[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]];
+                array[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]];
                 {{/if}}
                 {{/if}}
             }
 
-            id array = [NSArray arrayWithObjects:values count:count];
             dispatch_async(callback->mQueue, ^{
                 callback->mHandler(nil, @{ @"value": array });
                 callback->Cancel();

diff --git a/src/setup_payload/ManualSetupPayloadGenerator.cpp b/src/setup_payload/ManualSetupPayloadGenerator.cpp
@@ -88,10 +88,11 @@ static uint32_t chunk3PayloadRepresentation(const SetupPayload & payload)
 }
 
 // TODO: issue #3663 - Unbounded stack in src/setup_payload
-#if !defined(__clang__)
 #pragma GCC diagnostic push
+#if !defined(__clang__)
 #pragma GCC diagnostic ignored "-Wstack-usage="
 #endif
+#pragma GCC diagnostic ignored "-Wvla"
 
 static std::string decimalStringWithPadding(uint32_t number, int minLength)
 {
@@ -100,9 +101,7 @@ static std::string decimalStringWithPadding(uint32_t number, int minLength)
     return std::string(buf);
 }
 
-#if !defined(__clang__)
 #pragma GCC diagnostic pop
-#endif
 
 CHIP_ERROR ManualSetupPayloadGenerator::payloadDecimalStringRepresentation(std::string & outDecimalString)
 {