Re: [PATCH] eal/arm: replace RTE_BUILD_BUG on non-constant

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 3 May 2024 10:46:05 +0100
Daniel Gregory <daniel.gregory@bytedance.com> wrote:

> On Thu, May 02, 2024 at 02:48:26PM -0700, Stephen Hemminger wrote:
> > There are already constant checks like this elsewhere in the file.  
> 
> Yes, but they're in macros, rather than inlined functions, so my
> understanding was that at compile time, macro expansion has put the
> memorder constant in the _Static_assert call as opposed to still being
> a function parameter in the inline definition.

Gcc and clang are smart enough that it is possible to use the internal
__builtin_constant_p() in the function. Some examples in DPDK:

static __rte_always_inline int
rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
			   unsigned int n, struct rte_mempool_cache *cache)
{
	int ret;
	unsigned int remaining;
	uint32_t index, len;
	void **cache_objs;

	/* No cache provided */
	if (unlikely(cache == NULL)) {
		remaining = n;
		goto driver_dequeue;
	}

	/* The cache is a stack, so copy will be in reverse order. */
	cache_objs = &cache->objs[cache->len];

	if (__extension__(__builtin_constant_p(n)) && n <= cache->len) {

It should be possible to use RTE_BUILD_BUG_ON() or static_assert here.

Changing a compile time check into a runtime check means that buggy programs
blow up much later and in a confusing manner. And it impacts all code that
is doing a spin lock, certainly one of the hottest paths in DPDK.