diff --git a/Makefile.include b/Makefile.include index 9e242cd6b9b4..1d7b1c9febbe 100644 --- a/Makefile.include +++ b/Makefile.include @@ -423,6 +423,8 @@ include $(RIOTMAKE)/kconfig.mk FEATURES_REQUIRED += $(filter arch_%,$(FEATURES_PROVIDED)) # always select CPU core features FEATURES_REQUIRED += $(filter cpu_core_%,$(FEATURES_PROVIDED)) +# always "select" bug affecting the current build +FEATURES_REQUIRED += $(filter bug_%,$(FEATURES_PROVIDED)) # check if required features are provided and update $(FEATURES_USED) include $(RIOTMAKE)/features_check.inc.mk diff --git a/core/lib/include/assert.h b/core/lib/include/assert.h index 8891a95cc2bb..cda77e5999f6 100644 --- a/core/lib/include/assert.h +++ b/core/lib/include/assert.h @@ -44,7 +44,7 @@ extern "C" { * CFLAGS += -DDEBUG_ASSERT_VERBOSE * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ -#define DEBUG_ASSERT_VERBOSE +# define DEBUG_ASSERT_VERBOSE /** * @brief Activate breakpoints for @ref assert() when defined @@ -58,32 +58,39 @@ extern "C" { * execution is stopped directly in the debugger. Otherwise the execution stops * in an endless while loop. */ -#define DEBUG_ASSERT_BREAKPOINT +# define DEBUG_ASSERT_BREAKPOINT #else /* we should not include custom headers in standard headers */ -#define _likely(x) __builtin_expect((uintptr_t)(x), 1) +# define _likely(x) __builtin_expect((uintptr_t)(x), 1) #endif +#ifndef __NORETURN +# if defined(__GNUC__) || defined(DOXYGEN) /** - * @def __NORETURN - * @brief hidden (__) NORETURN definition + * @brief Same as @ref NORETURN * @internal * - * Duplicating the definitions of kernel_defines.h as these are unsuitable for - * system header files like the assert.h. - * kernel_defines.h would define symbols that are not reserved. + * We are not using @ref NORETURN from `compiler_hints.h` here to avoid RIOT + * dependencies for standard C headers */ -#ifndef __NORETURN -#ifdef __GNUC__ -#define __NORETURN __attribute__((noreturn)) -#else /*__GNUC__*/ -#define __NORETURN -#endif /*__GNUC__*/ -#endif /*__NORETURN*/ +# define __NORETURN __attribute__((noreturn)) +# else +# define __NORETURN +# endif +#endif + +/** + * @brief Internal function to trigger a panic with a failed assertion as + * identifying cause + * @internal + * @warning This is an internal function. API changes may happen without regard + * for out-of tree uses. + * + * The implementation will identify the cause of the panic as a blown assertion, + * e.g. via a log output. + */ +__NORETURN void _assert_panic(void); -#ifdef NDEBUG -#define assert(ignore)((void)0) -#elif defined(DEBUG_ASSERT_VERBOSE) /** * @brief Function to handle failed assertion * @@ -95,6 +102,10 @@ extern "C" { * @param[in] line The code line of @p file the assertion failed in */ __NORETURN void _assert_failure(const char *file, unsigned line); + +#ifdef NDEBUG +# define assert(ignore)((void)0) +#elif defined(DEBUG_ASSERT_VERBOSE) /** * @brief abort the program if assertion is false * @@ -132,27 +143,26 @@ __NORETURN void _assert_failure(const char *file, unsigned line); * * @see http://pubs.opengroup.org/onlinepubs/9699919799/functions/assert.html */ -#define assert(cond) (_likely(cond) ? (void)0 : _assert_failure(__FILE__, __LINE__)) +# define assert(cond) (_likely(cond) ? (void)0 : _assert_failure(__FILE__, __LINE__)) #else /* DEBUG_ASSERT_VERBOSE */ -__NORETURN void _assert_panic(void); -#define assert(cond) (_likely(cond) ? (void)0 : _assert_panic()) +# define assert(cond) (_likely(cond) ? (void)0 : _assert_panic()) #endif /* DEBUG_ASSERT_VERBOSE */ #if !defined __cplusplus -#if __STDC_VERSION__ >= 201112L +# if __STDC_VERSION__ >= 201112L /** * @brief c11 static_assert() macro */ -#define static_assert(...) _Static_assert(__VA_ARGS__) -#else +# define static_assert(...) _Static_assert(__VA_ARGS__) +# else /** * @brief static_assert for c-version < c11 * * Generates a division by zero compile error when cond is false */ -#define static_assert(cond, ...) \ - { enum { static_assert_failed_on_div_by_0 = 1 / (!!(cond)) }; } -#endif +# define static_assert(cond, ...) \ + { enum { static_assert_failed_on_div_by_0 = 1 / (!!(cond)) }; } +# endif #endif /** @@ -161,7 +171,7 @@ __NORETURN void _assert_panic(void); * If the assertion failed in an interrupt, the system will still panic. */ #ifndef DEBUG_ASSERT_NO_PANIC -#define DEBUG_ASSERT_NO_PANIC (1) +# define DEBUG_ASSERT_NO_PANIC (1) #endif #ifdef __cplusplus diff --git a/cpu/cortexm_common/Makefile.features b/cpu/cortexm_common/Makefile.features index c94905807fd4..1e6056dcf3c0 100644 --- a/cpu/cortexm_common/Makefile.features +++ b/cpu/cortexm_common/Makefile.features @@ -1,5 +1,6 @@ FEATURES_PROVIDED += arch_32bit FEATURES_PROVIDED += arch_arm +FEATURES_PROVIDED += bug_newlib_broken_stdio FEATURES_PROVIDED += cortexm_svc FEATURES_PROVIDED += cpp FEATURES_PROVIDED += cpu_check_address @@ -9,8 +10,8 @@ FEATURES_PROVIDED += libstdcpp FEATURES_PROVIDED += newlib FEATURES_PROVIDED += periph_flashpage_aux FEATURES_PROVIDED += periph_pm -FEATURES_PROVIDED += puf_sram FEATURES_PROVIDED += picolibc +FEATURES_PROVIDED += puf_sram FEATURES_PROVIDED += ssp # cortex-m33, cortex-m4f and cortex-m7 provide FPU support diff --git a/cpu/msp430/Makefile.features b/cpu/msp430/Makefile.features index 7107cccde90f..a272d3665da1 100644 --- a/cpu/msp430/Makefile.features +++ b/cpu/msp430/Makefile.features @@ -14,6 +14,7 @@ endif FEATURES_PROVIDED += arch_16bit FEATURES_PROVIDED += arch_msp430 +FEATURES_PROVIDED += bug_newlib_broken_stdio FEATURES_PROVIDED += cpu_$(CPU_FAM) FEATURES_PROVIDED += dbgpin FEATURES_PROVIDED += newlib diff --git a/cpu/riscv_common/Makefile.features b/cpu/riscv_common/Makefile.features index 46f9f267b4c9..f48285ffeee5 100644 --- a/cpu/riscv_common/Makefile.features +++ b/cpu/riscv_common/Makefile.features @@ -4,6 +4,7 @@ endif FEATURES_PROVIDED += arch_32bit FEATURES_PROVIDED += arch_riscv +FEATURES_PROVIDED += bug_newlib_broken_stdio FEATURES_PROVIDED += cpp FEATURES_PROVIDED += libstdcpp FEATURES_PROVIDED += newlib diff --git a/features.yaml b/features.yaml index 6174f93f4ba3..7113af63e6c5 100644 --- a/features.yaml +++ b/features.yaml @@ -922,3 +922,17 @@ groups: open and solder bridges SB4, SB5 and SB6 are closed instead, SPI2 is connected to the STMmod+/Pmod connector. Request this feature to use SPI2 with this board configuration. + +- title: Bugs in hardware, toolchain, or binary blobs + help: This allows modelling bugs found in boards, so that workarounds can + be enabled for affected builds by inspecting "FEATURES_USED". Any + feature prefixed with "bug_" provided in a build will always be used, + regardless of build configuration. + features: + - name: bug_newlib_broken_stdio + help: newlib's stdio is not inherently thread-safe, but depends on the + reentrancy hooks to be provided for correct operation. When + reentrancy is not enabled, using newlib's stdio is racy, causing + random crashes at runtime. Use of this feature will cause an + alternative stdio implementation to be used if (and only if) newlib + is used as standard c lib. diff --git a/makefiles/features_existing.inc.mk b/makefiles/features_existing.inc.mk index de51f5cd7655..6420ccd089cd 100644 --- a/makefiles/features_existing.inc.mk +++ b/makefiles/features_existing.inc.mk @@ -44,6 +44,7 @@ FEATURES_EXISTING := \ ble_phy_coded \ board_bat_voltage \ bootloader_stm32 \ + bug_newlib_broken_stdio \ can_rx_mailbox \ cortexm_fpu \ cortexm_mpu \ diff --git a/makefiles/libc/newlib.mk b/makefiles/libc/newlib.mk index c34b3545d045..b0d729f43a15 100644 --- a/makefiles/libc/newlib.mk +++ b/makefiles/libc/newlib.mk @@ -100,3 +100,7 @@ ifeq (1,$(USE_NEWLIB_NANO)) INCLUDES := -isystem $(NEWLIB_NANO_INCLUDE_DIR) $(INCLUDES) endif endif + +# In case externally compiled code gets linked in, it may use newlib's assert +# implementation. We wrap that to RIOT's assert implementation for consistency. +LINKFLAGS += -Wl,-wrap=__assert_func diff --git a/pkg/mpaland-printf/Makefile.dep b/pkg/mpaland-printf/Makefile.dep index dcb79a696c48..ee8fa39902be 100644 --- a/pkg/mpaland-printf/Makefile.dep +++ b/pkg/mpaland-printf/Makefile.dep @@ -3,8 +3,12 @@ # is not compatible with AVR MCUs. FEATURES_BLACKLIST += arch_avr8 -# On 32 bit and 64 bit systems support for long long is so cheap, that we -# can enable it by default. Users can still disable it, though. -ifneq (,$(filter arch_32bit arch_64bit,$(FEATURES_USED))) - DEFAULT_MODULE += printf_long_long +# On ESP it is not easily possible to replace newlib's stdio, as there is only +# API compatibility and not ABI compatibility between mpaland-printf and newlib's +# printf and the ESP SDK contains binary blobs. +FEATURES_BLACKLIST += arch_esp + +# On 64 bit systems, we depend on long long support to be able to support `%p` +ifneq (,$(filter arch_64bit,$(FEATURES_USED))) + USEMODULE += printf_long_long endif diff --git a/pkg/mpaland-printf/Makefile.include b/pkg/mpaland-printf/Makefile.include index e1d77aaeae2d..5a11eebce648 100644 --- a/pkg/mpaland-printf/Makefile.include +++ b/pkg/mpaland-printf/Makefile.include @@ -19,6 +19,10 @@ LINKFLAGS += -Wl,-wrap=puts # stdio, we will catch any instance of both mpaland-printf and newlib's stdio # being both linked in. LINKFLAGS += -Wl,-wrap=_printf_common +LINKFLAGS += -Wl,-wrap=_fprintf_r +LINKFLAGS += -Wl,-wrap=_printf_r +LINKFLAGS += -Wl,-wrap=iprintf +LINKFLAGS += -Wl,-wrap=fiprintf # Workaround for bug in the newlib headers shipped with e.g. Ubuntu 24.04 LTS # not defining PRId64 / PRIu64 / PRIx64 / PRIo64 in due to an issue diff --git a/sys/Makefile.dep b/sys/Makefile.dep index dd583117fa04..381a2d4bd764 100644 --- a/sys/Makefile.dep +++ b/sys/Makefile.dep @@ -229,6 +229,13 @@ ifneq (,$(filter newlib,$(USEMODULE))) endif endif endif + ifneq (,$(filter bug_newlib_broken_stdio,$(FEATURES_USED))) + # work around broken newlib stdio by using alternative stdio implementation, + # unless stdio_null is used, which is inherently thread-safe :) + ifeq (,$(filter stdio_null,$(USEMODULE))) + USEPKG += mpaland-printf + endif + endif endif ifneq (,$(filter posix_select,$(USEMODULE))) diff --git a/sys/newlib_syscalls_default/syscalls.c b/sys/newlib_syscalls_default/syscalls.c index a19d706647dc..4c6f2a623863 100644 --- a/sys/newlib_syscalls_default/syscalls.c +++ b/sys/newlib_syscalls_default/syscalls.c @@ -34,6 +34,7 @@ #include #include +#include "assert.h" #include "log.h" #include "modules.h" #include "periph/pm.h" @@ -144,23 +145,7 @@ static const struct heap heaps[NUM_HEAPS] = { */ void _init(void) { - /* Definition copied from newlib/libc/stdio/local.h */ - extern void __sinit (struct _reent *); - - /* When running multiple threads: Initialize reentrant structure before the - * scheduler starts. This normally happens upon the first stdio function - * called. However, if no boot message happens this can result in two - * concurrent "first calls" to stdio in data corruption, if no locking is - * used. Except for ESP (which is using its own syscalls.c anyway), this - * currently is the case in RIOT. */ - if (MAXTHREADS > 1) { - /* Also, make an exception for riotboot, which does not use stdio - * at all. This would pull in stdio and increase .text size - * significantly there */ - if (!IS_USED(MODULE_RIOTBOOT)) { - __sinit(_REENT); - } - } + /* nothing to do here */ } /** @@ -662,3 +647,20 @@ clock_t _times_r(struct _reent *ptr, struct tms *ptms) return (-1); } + +/** + * @brief Wrapper for newlib's assert implementation + */ +NORETURN +void __wrap___assert_func (const char *file, int line, const char *func, const char *expr) +{ + (void)file; + (void)line; + (void)func; + (void)expr; +#ifdef DEBUG_ASSERT_VERBOSE + _assert_failure(file, line); +#else + _assert_panic(); +#endif +} diff --git a/tests/sys/snprintf/Makefile b/tests/sys/snprintf/Makefile index d5e587b491b3..db6f70cd7577 100644 --- a/tests/sys/snprintf/Makefile +++ b/tests/sys/snprintf/Makefile @@ -4,4 +4,9 @@ include ../Makefile.sys_common # to compile due to PRI*64 macros not being defined FEATURES_BLACKLIST := arch_avr8 +# This enables support for printing 64 bit numbers. You can comment this out +# to shrink the test app. The test will skip the corresponding test +# automatically if printf_long_long is not used. +USEMODULE += printf_long_long + include $(RIOTBASE)/Makefile.include diff --git a/tests/sys/snprintf/main.c b/tests/sys/snprintf/main.c index bf9b33886779..29a61f25863c 100644 --- a/tests/sys/snprintf/main.c +++ b/tests/sys/snprintf/main.c @@ -27,6 +27,8 @@ #include #include +#include "modules.h" + static bool failed = false; static void check(const char *expected, const char *got, int retval) @@ -277,7 +279,12 @@ int main(void) test_int8(); test_int16(); test_int32(); - test_int64(); + if (IS_USED(MODULE_PRINTF_LONG_LONG)) { + test_int64(); + } + else { + puts("WARNING: Module printf_long_long not used, skipping test for 64 bit"); + } test_size(); test_flags_widths();