lib/os: Conditionally eliminate alloca/VLA usage

MISRA rules (see #9892) forbid alloca() and family, even though those
features can be valuable performance and memory size optimizations
useful to Zephyr.

Introduce a MISRA_SANE kconfig, which when true enables a gcc error
condition whenever a variable length array is used.

When enabled, the mempool code will use a theoretical-maximum array
size on the stack instead of one tailored to the current pool
configuration.

The rbtree code will do similarly, but because the theoretical maximum
is quite a bit larger (236 bytes on 32 bit platforms) the array is
placed into struct rbtree instead so it can live in static data (and
also so I don't have to go and retune all the test stack sizes!).
Current code only uses at most two of these (one in the scheduler when
SCHED_SCALABLE is selected, and one for dynamic kernel objects when
USERSPACE and DYNAMIC_OBJECTS are set).

This tunable is false by default, but is selected in a single test (a
subcase of tests/kernel/common) for coverage.  Note that the I2C and
SPI subsystems contain uncorrected VLAs, so a few platforms need to be
blacklisted with a filter.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>

CMakeLists.txt

@@ -187,6 +187,11 @@ if(NOT CONFIG_RTTI)
   )
 endif()
 
+if(CONFIG_MISRA_SANE)
+  zephyr_compile_options($<$<COMPILE_LANGUAGE:C>:-Werror=vla>)
+  zephyr_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Werror=vla>)
+endif()
+
 # @Intent: Obtain compiler specific flags for standard C includes
 toolchain_cc_nostdinc()
 

Kconfig.zephyr

@@ -402,4 +402,13 @@ config REBOOT
 	  Enable the sys_reboot() API. Enabling this can drag in other subsystems
 	  needed to perform a "safe" reboot (e.g. SYSTEM_CLOCK_DISABLE, to stop the
 	  system clock before issuing a reset).
+
+config MISRA_SANE
+	bool "MISRA standards compliance features"
+	help
+	  Causes the source code to build in "MISRA" mode, which
+	  disallows some otherwise-permitted features of the C
+	  standard for safety reasons.  Specifically variable length
+	  arrays are not permitted (and gcc will enforce this).
+
 endmenu

cmake/extra_flags.cmake

@@ -25,4 +25,3 @@ if(EXTRA_AFLAGS)
     zephyr_compile_options($<$<COMPILE_LANGUAGE:ASM>:${F}>)
   endforeach()
 endif()
-

include/misc/rb.h

@@ -49,6 +49,15 @@ struct rbnode {
 	struct rbnode *children[2];
 };
 
+/* Theoretical maximum depth of tree based on pointer size. If memory
+ * is filled with 2-pointer nodes, and the tree can be twice as a
+ * packed binary tree, plus root...  Works out to 59 entries for 32
+ * bit pointers and 121 at 64 bits.
+ */
+#define Z_TBITS(t) ((sizeof(t)) < 8 ? 2 : 3)
+#define Z_PBITS(t) (8 * sizeof(t))
+#define Z_MAX_RBTREE_DEPTH (2 * (Z_PBITS(int *) - Z_TBITS(int *) - 1) + 1)
+
 /**
  * @typedef rb_lessthan_t
  * @brief Red/black tree comparison predicate
@@ -68,6 +77,10 @@ struct rbtree {
 	struct rbnode *root;
 	rb_lessthan_t lessthan_fn;
 	int max_depth;
+#ifdef CONFIG_MISRA_SANE
+	struct rbnode *iter_stack[Z_MAX_RBTREE_DEPTH];
+	unsigned char iter_left[Z_MAX_RBTREE_DEPTH];
+#endif
 };
 
 typedef void (*rb_visit_t)(struct rbnode *node, void *cookie);
@@ -127,6 +140,7 @@ static inline void rb_walk(struct rbtree *tree, rb_visit_t visit_fn,
 {
 	_rb_walk(tree->root, visit_fn, cookie);
 }
+#endif
 
 struct _rb_foreach {
 	struct rbnode **stack;
@@ -134,14 +148,23 @@ struct _rb_foreach {
 	int top;
 };
 
+#ifdef CONFIG_MISRA_SANE
+#define _RB_FOREACH_INIT(tree, node) {					\
+	.stack   = &(tree)->iter_stack[0],				\
+	.is_left = &(tree)->iter_left[0],				\
+	.top     = -1							\
+}
+#else
 #define _RB_FOREACH_INIT(tree, node) {					\
 	.stack   = alloca((tree)->max_depth * sizeof(struct rbnode *)), \
 	.is_left = alloca((tree)->max_depth * sizeof(char)),		\
 	.top     = -1							\
 }
+#endif
 
 struct rbnode *_rb_foreach_next(struct rbtree *tree, struct _rb_foreach *f);
 
+#ifndef CONFIG_MISRA_SANE
 /**
  * @brief Walk a tree in-order without recursing
  *

lib/os/mempool.c

@@ -10,6 +10,12 @@
 #include <misc/mempool_base.h>
 #include <misc/mempool.h>
 
+#ifdef CONFIG_MISRA_SANE
+#define LVL_ARRAY_SZ(n) (8 * sizeof(void *) / 2)
+#else
+#define LVL_ARRAY_SZ(n) (n)
+#endif
+
 static bool level_empty(struct sys_mem_pool_base *p, int l)
 {
 	return sys_dlist_is_empty(&p->levels[l].free_list);
@@ -228,7 +234,7 @@ int _sys_mem_pool_block_alloc(struct sys_mem_pool_base *p, size_t size,
 	int i, from_l, alloc_l = -1, free_l = -1;
 	unsigned int key;
 	void *data = NULL;
-	size_t lsizes[p->n_levels];
+	size_t lsizes[LVL_ARRAY_SZ(p->n_levels)];
 
 	/* Walk down through levels, finding the one from which we
 	 * want to allocate and the smallest one with a free entry
@@ -298,7 +304,7 @@ int _sys_mem_pool_block_alloc(struct sys_mem_pool_base *p, size_t size,
 void _sys_mem_pool_block_free(struct sys_mem_pool_base *p, u32_t level,
 			      u32_t block)
 {
-	size_t lsizes[p->n_levels];
+	size_t lsizes[LVL_ARRAY_SZ(p->n_levels)];
 	int i;
 
 	/* As in _sys_mem_pool_block_alloc(), we build a table of level sizes

lib/os/rb.c

@@ -223,7 +223,11 @@ void rb_insert(struct rbtree *tree, struct rbnode *node)
 		return;
 	}
 
+#ifdef CONFIG_MISRA_SANE
+	struct rbnode **stack = &tree->iter_stack[0];
+#else
 	struct rbnode *stack[tree->max_depth + 1];
+#endif
 
 	int stacksz = find_and_stack(tree, node, stack);
 
@@ -357,7 +361,12 @@ static void fix_missing_black(struct rbnode **stack, int stacksz,
 
 void rb_remove(struct rbtree *tree, struct rbnode *node)
 {
-	struct rbnode *stack[tree->max_depth + 1], *tmp;
+	struct rbnode *tmp;
+#ifdef CONFIG_MISRA_SANE
+	struct rbnode **stack = &tree->iter_stack[0];
+#else
+	struct rbnode *stack[tree->max_depth + 1];
+#endif
 
 	int stacksz = find_and_stack(tree, node, stack);
 

tests/kernel/common/testcase.yaml

@@ -3,3 +3,11 @@ tests:
     tags: kernel
     min_flash: 33
     min_ram: 32
+  kernel.common.misra:
+    tags: kernel
+    min_flash: 33
+    min_ram: 32
+    # Some configurations are known-incompliant and won't build
+    filter: not ((CONFIG_I2C or CONFIG_SPI) and CONFIG_USERSPACE)
+    extra_configs:
+      - CONFIG_MISRA_SANE=y

tests/lib/rbtree/testcase.yaml

@@ -1,3 +1,4 @@
 tests:
   libraries.data_structures.rbtree:
     tags: rbtree
+    filter: not CONFIG_MISRA_SANE