From 7707060959f6a0433340b314092542374e9da2bb Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Wed, 27 Feb 2019 20:12:40 -0800 Subject: [PATCH] userspace: get rid of app section placeholders We used to leave byte-long placeholder symbols to ensure that empty application memory sections did not cause build errors that were very difficult to understand. Now we use some relatively portable inline assembly to generate a symbol, but don't take up any extra space. The malloc and libc partitions are now only instantiated if there is some data to put in them. Fixes: #13923 Signed-off-by: Andrew Boie --- include/app_memory/app_memdomain.h | 34 +++++++++++++++++++++++-- include/misc/libc-hooks.h | 11 ++++++++ kernel/userspace.c | 3 +++ lib/libc/minimal/source/stdlib/malloc.c | 5 +--- subsys/testsuite/ztest/src/ztest.c | 8 ++++-- 5 files changed, 53 insertions(+), 8 deletions(-) diff --git a/include/app_memory/app_memdomain.h b/include/app_memory/app_memdomain.h index acbd02e9558..363b695c298 100644 --- a/include/app_memory/app_memdomain.h +++ b/include/app_memory/app_memdomain.h @@ -67,6 +67,37 @@ struct z_app_region { #define Z_APP_BSS_START(id) data_smem_##id##_bss_start #define Z_APP_BSS_SIZE(id) data_smem_##id##_bss_size +/* If a partition is declared with K_APPMEM_PARTITION, but never has any + * data assigned to its contents, then no symbols with its prefix will end + * up in the symbol table. This prevents gen_app_partitions.py from detecting + * that the partition exists, and the linker symbols which specify partition + * bounds will not be generated, resulting in build errors. + * + * What this inline assembly code does is define a symbol with no data. + * This should work for all arches that produce ELF binaries, see + * https://sourceware.org/binutils/docs/as/Section.html + * + * We don't know what active flags/type of the pushed section were, so we are + * specific: "aw" indicates section is allocatable and writable, + * and "@progbits" indicates the section has data. + */ +#ifdef CONFIG_ARM +/* ARM has a quirk in that '@' denotes a comment, so we have to send + * %progbits to the assembler instead. + */ +#define Z_PROGBITS_SYM "\%" +#else +#define Z_PROGBITS_SYM "@" +#endif + +#define Z_APPMEM_PLACEHOLDER(name) \ + __asm__ ( \ + ".pushsection " STRINGIFY(K_APP_DMEM_SECTION(name)) \ + ",\"aw\"," Z_PROGBITS_SYM "progbits\n\t" \ + ".global " STRINGIFY(name) "_placeholder\n\t" \ + STRINGIFY(name) "_placeholder:\n\t" \ + ".popsection\n\t") + /** * @brief Define an application memory partition with linker support * @@ -94,8 +125,7 @@ struct z_app_region { .bss_start = &Z_APP_BSS_START(name), \ .bss_size = (size_t) &Z_APP_BSS_SIZE(name) \ }; \ - K_APP_BMEM(name) char name##_placeholder; - + Z_APPMEM_PLACEHOLDER(name); #else #define K_APP_BMEM(ptn) diff --git a/include/misc/libc-hooks.h b/include/misc/libc-hooks.h index 9d266dc24cb..544c0947905 100644 --- a/include/misc/libc-hooks.h +++ b/include/misc/libc-hooks.h @@ -37,12 +37,23 @@ __syscall size_t _zephyr_fwrite(const void *_MLIBC_RESTRICT ptr, size_t size, #endif /* CONFIG_NEWLIB_LIBC */ #ifdef CONFIG_USERSPACE +#if defined(CONFIG_NEWLIB_LIBC) || (CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE > 0) +#define Z_MALLOC_PARTITION_EXISTS 1 + /* Memory partition containing the libc malloc arena */ extern struct k_mem_partition z_malloc_partition; +#endif + +#if defined(CONFIG_NEWLIB_LIBC) || defined(CONFIG_STACK_CANARIES) +/* Minimal libc has no globals. We do put the stack canary global in the + * libc partition since it is not worth placing in a partition of its own. + */ +#define Z_LIBC_PARTITION_EXISTS 1 /* C library globals, except the malloc arena */ extern struct k_mem_partition z_libc_partition; #endif +#endif /* CONFIG_USERSPACE */ #include diff --git a/kernel/userspace.c b/kernel/userspace.c index 990d60e0b90..a744728f76e 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -18,8 +18,11 @@ #include #include #include +#include +#ifdef Z_LIBC_PARTITION_EXISTS K_APPMEM_PARTITION_DEFINE(z_libc_partition); +#endif #define LOG_LEVEL CONFIG_KERNEL_LOG_LEVEL #include diff --git a/lib/libc/minimal/source/stdlib/malloc.c b/lib/libc/minimal/source/stdlib/malloc.c index 4b69f5b7d95..f194f55a5a5 100644 --- a/lib/libc/minimal/source/stdlib/malloc.c +++ b/lib/libc/minimal/source/stdlib/malloc.c @@ -16,12 +16,9 @@ #include LOG_MODULE_DECLARE(os); -#ifdef CONFIG_USERSPACE -K_APPMEM_PARTITION_DEFINE(z_malloc_partition); -#endif - #if (CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE > 0) #ifdef CONFIG_USERSPACE +K_APPMEM_PARTITION_DEFINE(z_malloc_partition); #define POOL_SECTION K_APP_DMEM_SECTION(z_malloc_partition) #else #define POOL_SECTION .data diff --git a/subsys/testsuite/ztest/src/ztest.c b/subsys/testsuite/ztest/src/ztest.c index 6036a719f57..155ff1bf314 100644 --- a/subsys/testsuite/ztest/src/ztest.c +++ b/subsys/testsuite/ztest/src/ztest.c @@ -301,11 +301,15 @@ void main(void) { #ifdef CONFIG_USERSPACE struct k_mem_partition *parts[] = { - &ztest_mem_partition, +#ifdef Z_LIBC_PARTITION_EXISTS /* C library globals, stack canary storage, etc */ &z_libc_partition, +#endif +#ifdef Z_MALLOC_PARTITION_EXISTS /* Required for access to malloc arena */ - &z_malloc_partition + &z_malloc_partition, +#endif + &ztest_mem_partition }; /* Ztests just have one memory domain with one partition.