On Tue, 28 Nov 2023 at 18:00, David Brown <david.brown@xxxxxxxxxxxx> wrote: > > On 28/11/2023 10:51, Ed Robbins via Gcc-help wrote: > > On Tue, 28 Nov 2023 at 07:21, Matthias Pfaller <leo@xxxxxxxx> wrote: > >> > >> On 2023-11-27 16:16, Ed Robbins via Gcc-help wrote: > >>> Hello, > >>> I am using gcc-arm-none-eabi with a cortex M7 device, with caches > >>> (data/instruction) enabled. Nested function calls result in a usage fault > >>> because there is no clear cache call for this platform. > >>> > > >> I have lots of code with nested functions. When switching to gcc-12 I got random > >> crashes on my cortex-m7 targets. In order to get that working again I had to patch > >> gcc/config/arm/arm.h: > >> > > > Can I ask (either or both of you) why you are using are using nested > functions like this? This is possibly the first time I have heard of > anyone using them, certainly the first time in embedded development. > Even when I programmed in Pascal, where nested functions are part of the > language, I did not use them more than a couple of times. > > What benefit do you see in nested functions in C, compared to having > separate functions? Have you considered moving to C++ and using > lambdas, which are more flexible, standard, and can be very much more > efficient? > > This is, of course, straying from the topicality of this mailing list. > But I am curious, and I doubt if I am the only one who is. > > David > In the simplest case you may want to do something like: struct sometype* find_thing_with_value(struct list *things, uint32_t value) { bool match(void *thing) { return ((struct sometype*)thing)->field == value; } return (struct sometype*)list_find(things, &match); } In general though dependency inversion can be quite powerful. We use nested functions that do not access local variables fairly frequently in our codebase, mostly as a scoping mechanism. We would like to be able to access locals, and are aware of the implications, but only very recently decided to address the issue. I think that rather than switch to C++ lambdas we would be more inclined to move to clang blocks and stick with C. But there is no impetus for either move: Nested functions are just fine for what we need to do, and the syntax is very clean. We have thoughts about a HLL shift but it wouldn't be to C++. Security implications are not an issue for us because there is no way for an outsider to communicate with the devices, let alone get a stack overflow and code execution. I think that it would be sensible if there was a -mflush_func option for ARM targets, as for MIPS, which would resolve the caching issues that require a patched gcc build in this case. Thanks to Matthias for guidance. I have ended up implementing this a bit differently, as in the patch below, as it's then totally self-contained. I've tested by rebuilding the toolchain using the script from [1] and it is working well. Best regards, Ed [1] https://github.com/FreddieChopin/bleeding-edge-toolchain/tree/master >From 569177e29cdc777e84e5e8cf633ebef761e82f83 Mon Sep 17 00:00:00 2001 From: Ed Robbins <edd.robbins@xxxxxxxxx> Date: Tue, 5 Dec 2023 16:56:36 +0000 Subject: [PATCH] Define CLEAR_INSN_CACHE in arm.h and implement for v7em targets --- gcc/config/arm/arm.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index f479540812a..666d98611bc 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2518,4 +2518,37 @@ const char *arm_be8_option (int argc, const char **argv); representation for SHF_ARM_PURECODE in GCC. */ #define SECTION_ARM_PURECODE SECTION_MACH_DEP +#ifndef CLEAR_INSN_CACHE +/* When defined CLEAR_INSN_CACHE is called by __clear_cache in libgcc/libgcc2.c + It needs to always be _defined_, otherwise maybe_emit_call_builtin___clear_cache + from gcc/builtins.cc will not generate a call to __clear_cache, however we + only want it _implemented_ for the multilib version of libgcc.a built for + v7em targets. */ +#ifdef __ARM_ARCH_7EM__ +#define CLEAR_INSN_CACHE(BEG, END) \ + { \ + const void *scb_base = (const void*)0xe000e000; \ + const unsigned dccmvac_offset = 0x268; \ + const unsigned icimvau_offset = 0x258; \ + const unsigned cache_line_size = 32; \ + void *addr = (void*)((unsigned)BEG & ~(cache_line_size - 1)); \ + void *end = (void*)((unsigned)(END + cache_line_size - 1) & ~(cache_line_size - 1)); \ + __asm__ __volatile__("dsb" : : : "memory"); \ + while (addr < end) { \ + *(unsigned**)(scb_base + dccmvac_offset) = addr; \ + addr += cache_line_size; \ + } \ + __asm__ __volatile__("dsb; isb" : : : "memory"); \ + addr = (void*)((unsigned)BEG & ~(cache_line_size - 1)); \ + while (addr < end) { \ + *(unsigned**)(scb_base + icimvau_offset) = addr; \ + addr += cache_line_size; \ + } \ + __asm__ __volatile__("dsb; isb" : : : "memory"); \ + } +#else +#define CLEAR_INSN_CACHE(BEG, END) ; +#endif +#endif + #endif /* ! GCC_ARM_H */ -- 2.34.1