cpu/atmega_common: wrappers for memory management function to avoid preemption

[gschorcht]

Contribution description

This PR is an aproach to fix the problem described in #10842.

Memory management function like malloc, calloc, realloc and free must not be preempted when they operate on allocator structures. To avoid such a preemption, wrappers around these functions are used which simply disable all interrupts for the time of their execution.

This approach produces 78 additional bytes of code.

Testing procedure

Flash and run tests/malloc.

Issues/PRs references

Fixes #10842

[fjmolinas]

is this related to #8619?

[gschorcht]

Not really. PR #8619 implements the locking mechanism as used by newlibc. AVR doesn't use the newlibc but their own avr-libc.

[maribu]

OK, for testing I modified examples/hello-world as described by the following patch:

diff --git a/examples/hello-world/main.c b/examples/hello-world/main.c
index f51bf8c0a0..212e0f62f4 100644
--- a/examples/hello-world/main.c
+++ b/examples/hello-world/main.c
@@ -20,6 +20,7 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 
 int main(void)
 {
@@ -27,6 +28,14 @@ int main(void)
 
     printf("You are running RIOT on a(n) %s board.\n", RIOT_BOARD);
     printf("This board features a(n) %s MCU.\n", RIOT_MCU);
+    void *a = malloc(4);
+    printf("void *a = %p\n", a);
+    void *b = calloc(4, 4);
+    printf("void *b = %p\n", b);
+    a = realloc(a, 32);
+    printf("void *a = %p (after realloc)\n", a);
+    free(a);
+    free(b);
 
     return 0;
 }

The console output was

2019-08-13 12:34:17,334 - INFO # main(): This is RIOT! (Version: 2019.10-devel-375-gd84f7-gunnar)
2019-08-13 12:34:17,335 - INFO # Hello World!
2019-08-13 12:34:17,401 - INFO # You are running RIOT on a(n) arduino-nano board.
2019-08-13 12:34:17,434 - INFO # This board features a(n) atmega328p MCU.
2019-08-13 12:34:17,436 - INFO # void *a = 0x5e1
2019-08-13 12:34:17,468 - INFO # void *b = 0x5e7
2019-08-13 12:34:17,480 - INFO # void *a = 0x5f9 (after realloc)

So no regressions were introduced.

Finally, I disassembled the symbol main and got the following output:

00000124 <main>:
     124:       86 e2           ldi     r24, 0x26       ; 38
     126:       91 e0           ldi     r25, 0x01       ; 1
     128:       0e 94 8b 08     call    0x1116  ; 0x1116 <puts>
     12c:       83 e3           ldi     r24, 0x33       ; 51
     12e:       91 e0           ldi     r25, 0x01       ; 1
     130:       9f 93           push    r25
     132:       8f 93           push    r24
     134:       80 e4           ldi     r24, 0x40       ; 64
     136:       91 e0           ldi     r25, 0x01       ; 1
     138:       9f 93           push    r25
     13a:       8f 93           push    r24
     13c:       0e 94 75 08     call    0x10ea  ; 0x10ea <printf>
     140:       88 e6           ldi     r24, 0x68       ; 104
     142:       91 e0           ldi     r25, 0x01       ; 1
     144:       9f 93           push    r25
     146:       8f 93           push    r24
     148:       83 e7           ldi     r24, 0x73       ; 115
     14a:       91 e0           ldi     r25, 0x01       ; 1
     14c:       9f 93           push    r25
     14e:       8f 93           push    r24
     150:       0e 94 75 08     call    0x10ea  ; 0x10ea <printf>
     154:       84 e0           ldi     r24, 0x04       ; 4
     156:       90 e0           ldi     r25, 0x00       ; 0
     158:       0e 94 e7 00     call    0x1ce   ; 0x1ce <__wrap_malloc>
     15c:       8c 01           movw    r16, r24
     15e:       1f 93           push    r17
     160:       8f 93           push    r24
     162:       85 e9           ldi     r24, 0x95       ; 149
     164:       91 e0           ldi     r25, 0x01       ; 1
     166:       9f 93           push    r25
     168:       8f 93           push    r24
     16a:       0e 94 75 08     call    0x10ea  ; 0x10ea <printf>
     16e:       64 e0           ldi     r22, 0x04       ; 4
     170:       70 e0           ldi     r23, 0x00       ; 0
     172:       84 e0           ldi     r24, 0x04       ; 4
     174:       90 e0           ldi     r25, 0x00       ; 0
     176:       0e 94 0e 01     call    0x21c   ; 0x21c <__wrap_calloc>
     17a:       d8 2f           mov     r29, r24
     17c:       c9 2f           mov     r28, r25
     17e:       9f 93           push    r25
     180:       8f 93           push    r24
     182:       83 ea           ldi     r24, 0xA3       ; 163
     184:       91 e0           ldi     r25, 0x01       ; 1
     186:       9f 93           push    r25
     188:       8f 93           push    r24
     18a:       0e 94 75 08     call    0x10ea  ; 0x10ea <printf>
     18e:       60 e2           ldi     r22, 0x20       ; 32
     190:       70 e0           ldi     r23, 0x00       ; 0
     192:       c8 01           movw    r24, r16
     194:       0e 94 29 01     call    0x252   ; 0x252 <__wrap_realloc>
     198:       8c 01           movw    r16, r24
     19a:       1f 93           push    r17
     19c:       8f 93           push    r24
     19e:       81 eb           ldi     r24, 0xB1       ; 177
     1a0:       91 e0           ldi     r25, 0x01       ; 1
     1a2:       9f 93           push    r25
     1a4:       8f 93           push    r24
     1a6:       0e 94 75 08     call    0x10ea  ; 0x10ea <printf>
     1aa:       c8 01           movw    r24, r16
     1ac:       0e 94 fc 00     call    0x1f8   ; 0x1f8 <__wrap_free>
     1b0:       8d 2f           mov     r24, r29
     1b2:       9c 2f           mov     r25, r28
     1b4:       0e 94 fc 00     call    0x1f8   ; 0x1f8 <__wrap_free>
     1b8:       8d b7           in      r24, 0x3d       ; 61
     1ba:       9e b7           in      r25, 0x3e       ; 62
     1bc:       44 96           adiw    r24, 0x14       ; 20
     1be:       0f b6           in      r0, 0x3f        ; 63
     1c0:       f8 94           cli
     1c2:       9e bf           out     0x3e, r25       ; 62
     1c4:       0f be           out     0x3f, r0        ; 63
     1c6:       8d bf           out     0x3d, r24       ; 61
     1c8:       90 e0           ldi     r25, 0x00       ; 0
     1ca:       80 e0           ldi     r24, 0x00       ; 0
     1cc:       08 95           ret

So every call to malloc(), calloc(), realloc() and free() was indeed rerouted as intended.

[maribu]

ACK. Thanks for this very elegant fix!

[gschorcht]

@maribu Thanks for reviewing, testing and merging.