On Tue, 5 Dec 2023 at 17:10, Ed Robbins <edd.robbins@xxxxxxxxxxxxxx> wrote:
>
> 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.
>
I guess I should have attached that instead of pasting it... second time lucky
Ed
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
