[v5,6/6] eal: replace out of bounds VLA with static_assert

Message ID 20240118165315.63959-7-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series use static_assert for build error reports |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Stephen Hemminger Jan. 18, 2024, 4:51 p.m. UTC
  Both Gcc, clang and MSVC have better way to do compile time
assertions rather than using out of bounds array access.
The old method would fail if -Wvla is enabled because compiler
can't determine size in that code.  Also, the use of new
_Static_assert will catch broken code that is passing non-constant
expression to RTE_BUILD_BUG_ON().

Add workaround for clang static_assert in switch,
and missing static_assert in older FreeBSD.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_common.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
  

Comments

Morten Brørup Jan. 18, 2024, 6:42 p.m. UTC | #1
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 18 January 2024 17.51
> 
> Both Gcc, clang and MSVC have better way to do compile time
> assertions rather than using out of bounds array access.
> The old method would fail if -Wvla is enabled because compiler
> can't determine size in that code.  Also, the use of new
> _Static_assert will catch broken code that is passing non-constant
> expression to RTE_BUILD_BUG_ON().
> 
> Add workaround for clang static_assert in switch,
> and missing static_assert in older FreeBSD.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
  
Ferruh Yigit Jan. 19, 2024, 1:10 p.m. UTC | #2
On 1/18/2024 6:42 PM, Morten Brørup wrote:
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Thursday, 18 January 2024 17.51
>>
>> Both Gcc, clang and MSVC have better way to do compile time
>> assertions rather than using out of bounds array access.
>> The old method would fail if -Wvla is enabled because compiler
>> can't determine size in that code.  Also, the use of new
>> _Static_assert will catch broken code that is passing non-constant
>> expression to RTE_BUILD_BUG_ON().
>>
>> Add workaround for clang static_assert in switch,
>> and missing static_assert in older FreeBSD.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>> ---
> 
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
  

Patch

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 33680e818bfb..aa066125b6cd 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -16,6 +16,7 @@ 
 extern "C" {
 #endif
 
+#include <assert.h>
 #include <stdint.h>
 #include <limits.h>
 
@@ -492,10 +493,18 @@  rte_is_aligned(const void * const __rte_restrict ptr, const unsigned int align)
 
 /*********** Macros for compile type checks ********/
 
+/* Workaround for toolchain issues with missing C11 macro in FreeBSD */
+#if !defined(static_assert) && !defined(__cplusplus)
+#define	static_assert	_Static_assert
+#endif
+
 /**
  * Triggers an error at compilation time if the condition is true.
+ *
+ * The do { } while(0) exists to workaround a bug in clang (#55821)
+ * where it would not handle _Static_assert in a switch case.
  */
-#define RTE_BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#define RTE_BUILD_BUG_ON(condition) do { static_assert(!(condition), #condition); } while (0)
 
 /*********** Cache line related macros ********/