Re: [PATCH v7 19/21] test: remove use of VLAs for Windows built code in bitset tests

[Andre Muezerie <andremue@linux.microsoft.com>]

On Tue, Nov 19, 2024 at 11:16:04AM +0100, Mattias Rönnblom wrote:
> On 2024-11-12 03:02, Andre Muezerie wrote:
> >1) MSVC does not support VLAs. Use standard fixed C arrays of
> >maximum size required instead.
> >
> >2) ../usr/lib/gcc/x86_64-redhat-linux/13/include/emmintrin.h:742:8:
> >     error: array subscript 9 is outside array bounds of 'uint64_t[16]'
> >     {aka 'long unsigned int[16]'} [-Werror=array-bounds=]
> >    3695 742 | *__P = __B;
> >
> >Compile with -Wno-array-bounds to avoid false positives when
> >using gcc version 11 or newer (gcc compiler bug/limitation).
> >
> >Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> >---
> >  app/test/meson.build            |  5 +++
> >  app/test/test_bitset.c          | 55 +++++++++++++++++++--------------
> >  app/test/test_lcore_var_perf.c  |  2 +-
> >  app/test/test_reassembly_perf.c |  4 +--
> >  4 files changed, 39 insertions(+), 27 deletions(-)
> >
> >diff --git a/app/test/meson.build b/app/test/meson.build
> >index 40f22a54d5..e7419a611b 100644
> >--- a/app/test/meson.build
> >+++ b/app/test/meson.build
> >@@ -249,6 +249,11 @@ foreach d:optional_deps
> >      endif
> >  endforeach
> >+# Bugzilla ID: 678
> 
> Mention which bugzilla.

I'll remove this bug ID and add an explanation instead. As suggested in other email,
I'll also reduce the scope when disabling these errors.
For reference, this issue we are facing is similar to the bug 678 mentioned:
https://bugs.dpdk.org/show_bug.cgi?id=678

> 
> >+if (toolchain == 'gcc' and cc.version().version_compare('>=11.0.0'))
> >+    cflags += '-Wno-array-bounds'
> >+endif
> >+
> >  if cc.has_argument('-Wno-format-truncation')
> >      cflags += '-Wno-format-truncation'
> >  endif
> >diff --git a/app/test/test_bitset.c b/app/test/test_bitset.c
> >index 50b8bf0da4..f58c491d8f 100644
> >--- a/app/test/test_bitset.c
> >+++ b/app/test/test_bitset.c
> >@@ -99,11 +99,13 @@ typedef void clear_fun(uint64_t *bitset, size_t bit_num);
> >  typedef void assign_fun(uint64_t *bitset, size_t bit_num, bool value);
> >  typedef void flip_fun(uint64_t *bitset, size_t bit_num);
> >+#define RAND_SET_MAX_SIZE (1000)
> >+
> 
> Remove redundant parenthesis on all in #define in this file, please.
> (A bad habit on mine.)
> 

Sure, I'll update this in the new series.

> >  static int
> >  test_set_clear_size(test_fun test_fun, set_fun set_fun, clear_fun clear_fun, size_t size)
> >  {
> >  	size_t i;
> >-	bool reference[size];
> >+	bool reference[RAND_SET_MAX_SIZE];
> >  	uint64_t *bitset;
> >  	rand_bool_ary(reference, size);
> >@@ -132,7 +134,6 @@ test_set_clear_size(test_fun test_fun, set_fun set_fun, clear_fun clear_fun, siz
> >  }
> >  #define RAND_ITERATIONS (10000)
> >-#define RAND_SET_MAX_SIZE (1000)
> >  static int
> >  test_set_clear_fun(test_fun test_fun, set_fun set_fun, clear_fun clear_fun)
> >@@ -168,7 +169,7 @@ test_flip_size(test_fun test_fun, assign_fun assign_fun, flip_fun flip_fun, size
> >  	rand_bitset(bitset, size);
> >  	for (i = 0; i < size; i++) {
> >-		RTE_BITSET_DECLARE(reference, size);
> >+		RTE_BITSET_DECLARE(reference, RAND_SET_MAX_SIZE);
> >  		rte_bitset_copy(reference, bitset, size);
> >@@ -288,7 +289,7 @@ static int
> >  test_find_size(size_t size, bool set)
> >  {
> >  	uint64_t *bitset;
> >-	bool reference[size];
> >+	bool reference[RAND_SET_MAX_SIZE];
> >  	size_t i;
> >  	bitset = alloc_bitset(size);
> >@@ -388,8 +389,8 @@ record_match(ssize_t match_idx, size_t size, int *calls)
> >  static int
> >  test_foreach_size(ssize_t size, bool may_wrap, bool set)
> >  {
> >-	bool reference[size];
> >-	int calls[size];
> >+	bool reference[RAND_SET_MAX_SIZE];
> >+	int calls[RAND_SET_MAX_SIZE];
> >  	uint64_t *bitset;
> >  	ssize_t i;
> >  	ssize_t start_bit;
> >@@ -633,17 +634,19 @@ test_define(void)
> >  typedef void bitset_op(uint64_t *dst, const uint64_t *a, const uint64_t *b, size_t bit_num);
> >  typedef bool bool_op(bool a, bool b);
> >+#define LOGIC_MAX_SET_SIZE 200
> >+
> >  static int
> >  test_logic_op(bitset_op bitset_op, bool_op bool_op)
> >  {
> >-	const size_t size = 1 + rte_rand_max(200);
> >-	RTE_BITSET_DECLARE(bitset_a, size);
> >-	RTE_BITSET_DECLARE(bitset_b, size);
> >-	RTE_BITSET_DECLARE(bitset_d, size);
> >+	const size_t size = 1 + rte_rand_max(LOGIC_MAX_SET_SIZE);
> >+	RTE_BITSET_DECLARE(bitset_a, LOGIC_MAX_SET_SIZE);
> >+	RTE_BITSET_DECLARE(bitset_b, LOGIC_MAX_SET_SIZE);
> >+	RTE_BITSET_DECLARE(bitset_d, LOGIC_MAX_SET_SIZE);
> >-	bool ary_a[size];
> >-	bool ary_b[size];
> >-	bool ary_d[size];
> >+	bool ary_a[LOGIC_MAX_SET_SIZE];
> >+	bool ary_b[LOGIC_MAX_SET_SIZE];
> >+	bool ary_d[LOGIC_MAX_SET_SIZE];
> >  	rand_bool_ary(ary_a, size);
> >  	rand_bool_ary(ary_b, size);
> >@@ -708,14 +711,14 @@ test_complement(void)
> >  	for (i = 0; i < RAND_ITERATIONS; i++) {
> >  		const size_t size = 1 + rte_rand_max(RAND_SET_MAX_SIZE - 1);
> >-		RTE_BITSET_DECLARE(src, size);
> >+		RTE_BITSET_DECLARE(src, RAND_SET_MAX_SIZE);
> >  		rand_bitset(src, size);
> >  		bool bit_idx = rte_rand_max(size);
> >  		bool bit_value = rte_bitset_test(src, bit_idx);
> >-		RTE_BITSET_DECLARE(dst, size);
> >+		RTE_BITSET_DECLARE(dst, RAND_SET_MAX_SIZE);
> >  		rte_bitset_complement(dst, src, size);
> >@@ -726,6 +729,8 @@ test_complement(void)
> >  	return TEST_SUCCESS;
> >  }
> >+#define SHIFT_SET_MAX_SIZE 500
> >+
> >  static int
> >  test_shift(bool right)
> >  {
> >@@ -734,12 +739,12 @@ test_shift(bool right)
> >  	const char *direction = right ? "right" : "left";
> >  	for (i = 0; i < 10000; i++) {
> >-		const int size = 1 + (int)rte_rand_max(500);
> >+		const int size = 1 + (int)rte_rand_max(SHIFT_SET_MAX_SIZE);
> >  		const int shift_count = (int)rte_rand_max(1.5 * size);
> >  		int src_idx;
> >-		RTE_BITSET_DECLARE(src, size);
> >-		RTE_BITSET_DECLARE(reference, size);
> >+		RTE_BITSET_DECLARE(src, SHIFT_SET_MAX_SIZE);
> >+		RTE_BITSET_DECLARE(reference, SHIFT_SET_MAX_SIZE);
> >  		rte_bitset_init(src, size);
> >  		rte_bitset_init(reference, size);
> >@@ -788,12 +793,14 @@ test_shift_left(void)
> >  	return test_shift(false);
> >  }
> >+#define EQUAL_SET_MAX_SIZE 100
> >+
> >  static int
> >  test_equal(void)
> >  {
> >-	const size_t size = 100;
> >-	RTE_BITSET_DECLARE(bitset_a, size);
> >-	RTE_BITSET_DECLARE(bitset_b, size);
> >+	const size_t size = EQUAL_SET_MAX_SIZE;
> >+	RTE_BITSET_DECLARE(bitset_a, EQUAL_SET_MAX_SIZE);
> >+	RTE_BITSET_DECLARE(bitset_b, EQUAL_SET_MAX_SIZE);
> >  	rand_buf(bitset_a, RTE_BITSET_SIZE(size));
> >  	rand_buf(bitset_b, RTE_BITSET_SIZE(size));
> >@@ -821,9 +828,9 @@ test_equal(void)
> >  static int
> >  test_copy(void)
> >  {
> >-	const size_t size = 100;
> >-	RTE_BITSET_DECLARE(bitset_a, size);
> >-	RTE_BITSET_DECLARE(bitset_b, size);
> >+	const size_t size = EQUAL_SET_MAX_SIZE;
> >+	RTE_BITSET_DECLARE(bitset_a, EQUAL_SET_MAX_SIZE);
> >+	RTE_BITSET_DECLARE(bitset_b, EQUAL_SET_MAX_SIZE);
> 
> This is not still a VLA?
> 
> If I recall correctly, in C, a const variable declared inside a
> function is not considered a compile-time constant expression. In
> C++ it is.
>

You're right about the const variable not being considered a compile-time
expression. That's why we are still using the macro when calling RTE_BITSET_DECLARE.

> >  	rand_buf(bitset_a, RTE_BITSET_SIZE(size));
> >  	rand_buf(bitset_b, RTE_BITSET_SIZE(size));
> >diff --git a/app/test/test_lcore_var_perf.c b/app/test/test_lcore_var_perf.c
> >index 41e29bbd49..5a74ce4808 100644
> >--- a/app/test/test_lcore_var_perf.c
> >+++ b/app/test/test_lcore_var_perf.c
> >@@ -185,7 +185,7 @@ test_lcore_var_access_n(unsigned int num_mods)
> >  	double tls_latency;
> >  	double lazy_tls_latency;
> >  	double lvar_latency;
> >-	unsigned int mods[num_mods];
> >+	unsigned int mods[MAX_MODS];
> >  	unsigned int i;
> >  	for (i = 0; i < num_mods; i++)
> >diff --git a/app/test/test_reassembly_perf.c b/app/test/test_reassembly_perf.c
> >index 69cf029468..ac225e2b53 100644
> >--- a/app/test/test_reassembly_perf.c
> >+++ b/app/test/test_reassembly_perf.c
> >@@ -577,7 +577,7 @@ ipv4_reassembly_interleaved_flows_perf(uint8_t nb_frags)
> >  		for (j = 0; j < 4; j++)
> >  			nb_frags += frag_per_flow[i + j];
> >-		struct rte_mbuf *buf_arr[nb_frags];
> >+		struct rte_mbuf *buf_arr[MAX_FRAGMENTS];
> >  		for (j = 0; j < 4; j++) {
> >  			join_array(buf_arr, mbufs[i + j], prev,
> >  				   frag_per_flow[i + j]);
> >@@ -788,7 +788,7 @@ ipv6_reassembly_interleaved_flows_perf(int8_t nb_frags)
> >  		for (j = 0; j < 4; j++)
> >  			nb_frags += frag_per_flow[i + j];
> >-		struct rte_mbuf *buf_arr[nb_frags];
> >+		struct rte_mbuf *buf_arr[MAX_FRAGMENTS];
> >  		for (j = 0; j < 4; j++) {
> >  			join_array(buf_arr, mbufs[i + j], prev,
> >  				   frag_per_flow[i + j]);