| 2017-10-11 16:17:10 |
Miro Samek |
description |
I've been able to distill the problem to the following test code:
#include <stdint.h>
/*! Private QS data to keep track of the filters and the trace buffer. */
typedef struct {
uint8_t glbFilter[16]; /*!< global on/off QS filter */
void const *locFilter[16]; /*!< local QS filters */
//...
} QSPriv;
extern QSPriv QS_priv_;
extern void *QS_obj_;
void foo(void); /* prototype */
void bug_test(void) {
uint8_t i;
for(i = 0; i < 10; i++) {
if ((((uint_fast8_t)QS_priv_.glbFilter[(uint8_t)(123) >> 3]
& (uint_fast8_t)((uint_fast8_t)1 << ((uint8_t)(123) & (uint8_t)7)))
!= (uint_fast8_t)0)
&& ((QS_priv_.locFilter[3] == (void *)0)
|| (QS_priv_.locFilter[3] == (QS_obj_))))
{
uint32_t status;
/* save the PRIMASK into 'status' */
__asm volatile ("mrs %0,PRIMASK" : "=r" (status) :: );
/* set PRIMASK to disable interrupts */
__asm volatile ("cpsid i");
foo(); /* call a function */
/* restore PRIMASK from 'status' */
__asm volatile ("msr PRIMASK,%0" :: "r" (status) : );
}
}
}
The code compiled with GNU Tools for ARM Embedded Processors 6-2017-q2-update, 6.3.1 20170620 release on Windows 10 64-bit. The following command line was used:
C:\tools\GNU_ARM\bin\arm-none-eabi-gcc -c -O -mcpu=cortex-m0plus -mthumb -Wall bug.c -o bug.o
C:\tools\GNU_ARM\bin\arm-none-eabi-objdump -d bug.o > bug.dasm
Here is the generated disassembly:
Disassembly of section .text:
00000000 <bug_test>:
0: b5f8 push {r3, r4, r5, r6, r7, lr}
2: 240a movs r4, #10
4: 4d0c ldr r5, [pc, #48] ; (38 <bug_test+0x38>)
6: 002e movs r6, r5
8: 4f0c ldr r7, [pc, #48] ; (3c <bug_test+0x3c>)
a: e00a b.n 22 <bug_test+0x22>
c: b672 cpsid i
e: f7ff fffe bl 0 <foo>
12: f3ef 8310 mrs r3, PRIMASK
16: f383 8810 msr PRIMASK, r3
1a: 3c01 subs r4, #1
1c: b2e4 uxtb r4, r4
1e: 2c00 cmp r4, #0
20: d009 beq.n 36 <bug_test+0x36>
22: 7beb ldrb r3, [r5, #15]
24: 071b lsls r3, r3, #28
26: d5f8 bpl.n 1a <bug_test+0x1a>
28: 69f3 ldr r3, [r6, #28]
2a: 2b00 cmp r3, #0
2c: d0ee beq.n c <bug_test+0xc>
2e: 683a ldr r2, [r7, #0]
30: 4293 cmp r3, r2
32: d1f2 bne.n 1a <bug_test+0x1a>
34: e7ea b.n c <bug_test+0xc>
36: bdf8 pop {r3, r4, r5, r6, r7, pc}
...
The problem is the incorrect disabling of interrupts around the function call foo().
The *same* code compiled with optimization -O2 is as follows:
Disassembly of section .text:
00000000 <bug_test>:
0: b5f0 push {r4, r5, r6, r7, lr}
2: 46c6 mov lr, r8
4: 4b0f ldr r3, [pc, #60] ; (44 <bug_test+0x44>)
6: 240a movs r4, #10
8: 2608 movs r6, #8
a: 4698 mov r8, r3
c: b500 push {lr}
e: 4d0e ldr r5, [pc, #56] ; (48 <bug_test+0x48>)
10: 7beb ldrb r3, [r5, #15]
12: 421e tst r6, r3
14: d006 beq.n 24 <bug_test+0x24>
16: 69eb ldr r3, [r5, #28]
18: 2b00 cmp r3, #0
1a: d00a beq.n 32 <bug_test+0x32>
1c: 4642 mov r2, r8
1e: 6812 ldr r2, [r2, #0]
20: 4293 cmp r3, r2
22: d006 beq.n 32 <bug_test+0x32>
24: 3c01 subs r4, #1
26: b2e4 uxtb r4, r4
28: 2c00 cmp r4, #0
2a: d1f1 bne.n 10 <bug_test+0x10>
2c: bc04 pop {r2}
2e: 4690 mov r8, r2
30: bdf0 pop {r4, r5, r6, r7, pc}
32: f3ef 8710 mrs r7, PRIMASK
36: b672 cpsid i
38: f7ff fffe bl 0 <foo>
3c: f387 8810 msr PRIMASK, r7
40: e7f0 b.n 24 <bug_test+0x24>
42: 46c0 nop ; (mov r8, r8)
...
Which seems to be correct in terms of the critical section.
Miro Samek
state-machine.com |
The GCC for ARM generates incorrect code for disabling interrupts around a function call. The interrupts are disabled by first obtaining the PRIMASK value into an automatic variable and then setting PRIMASK. Interrupts are re-enabled by restoring the saved PRIMASK value. Inline assembly is used for these operations (why there are no intrinsic functions for this?).
The problem is the incorrect reordering of the instructions for disabling/enabling interrupts with optimization level -O.
I've been able to distill the problem to the following test code:
#include <stdint.h>
/*! Private QS data to keep track of the filters and the trace buffer. */
typedef struct {
uint8_t glbFilter[16]; /*!< global on/off QS filter */
void const *locFilter[16]; /*!< local QS filters */
//...
} QSPriv;
extern QSPriv QS_priv_;
extern void *QS_obj_;
void foo(void); /* prototype */
void bug_test(void) {
uint8_t i;
for(i = 0; i < 10; i++) {
if ((((uint_fast8_t)QS_priv_.glbFilter[(uint8_t)(123) >> 3]
& (uint_fast8_t)((uint_fast8_t)1 << ((uint8_t)(123) & (uint8_t)7)))
!= (uint_fast8_t)0)
&& ((QS_priv_.locFilter[3] == (void *)0)
|| (QS_priv_.locFilter[3] == (QS_obj_))))
{
uint32_t status;
/* save the PRIMASK into 'status' */
__asm volatile ("mrs %0,PRIMASK" : "=r" (status) :: );
/* set PRIMASK to disable interrupts */
__asm volatile ("cpsid i");
foo(); /* call a function */
/* restore PRIMASK from 'status' */
__asm volatile ("msr PRIMASK,%0" :: "r" (status) : );
}
}
}
The code compiled with GNU Tools for ARM Embedded Processors 6-2017-q2-update, 6.3.1 20170620 release on Windows 10 64-bit. The following command line was used:
C:\tools\GNU_ARM\bin\arm-none-eabi-gcc -c -O -mcpu=cortex-m0plus -mthumb -Wall bug.c -o bug.o
C:\tools\GNU_ARM\bin\arm-none-eabi-objdump -d bug.o > bug.dasm
Here is the generated disassembly:
Disassembly of section .text:
00000000 <bug_test>:
0: b5f8 push {r3, r4, r5, r6, r7, lr}
2: 240a movs r4, #10
4: 4d0c ldr r5, [pc, #48] ; (38 <bug_test+0x38>)
6: 002e movs r6, r5
8: 4f0c ldr r7, [pc, #48] ; (3c <bug_test+0x3c>)
a: e00a b.n 22 <bug_test+0x22>
c: b672 cpsid i
e: f7ff fffe bl 0 <foo>
12: f3ef 8310 mrs r3, PRIMASK
16: f383 8810 msr PRIMASK, r3
1a: 3c01 subs r4, #1
1c: b2e4 uxtb r4, r4
1e: 2c00 cmp r4, #0
20: d009 beq.n 36 <bug_test+0x36>
22: 7beb ldrb r3, [r5, #15]
24: 071b lsls r3, r3, #28
26: d5f8 bpl.n 1a <bug_test+0x1a>
28: 69f3 ldr r3, [r6, #28]
2a: 2b00 cmp r3, #0
2c: d0ee beq.n c <bug_test+0xc>
2e: 683a ldr r2, [r7, #0]
30: 4293 cmp r3, r2
32: d1f2 bne.n 1a <bug_test+0x1a>
34: e7ea b.n c <bug_test+0xc>
36: bdf8 pop {r3, r4, r5, r6, r7, pc}
...
The problem is the incorrect disabling of interrupts around the function call foo().
As I experimented with this code, the excessive type casting in the condition for the if statement seems to be implicated (the bug goes away if I remove some of this type casting). The type casting has been added in the first place to satisfy static analysis with PC-Lint for MISRA-C compliance.
The *same* code compiled with optimization -O2 is as follows:
Disassembly of section .text:
00000000 <bug_test>:
0: b5f0 push {r4, r5, r6, r7, lr}
2: 46c6 mov lr, r8
4: 4b0f ldr r3, [pc, #60] ; (44 <bug_test+0x44>)
6: 240a movs r4, #10
8: 2608 movs r6, #8
a: 4698 mov r8, r3
c: b500 push {lr}
e: 4d0e ldr r5, [pc, #56] ; (48 <bug_test+0x48>)
10: 7beb ldrb r3, [r5, #15]
12: 421e tst r6, r3
14: d006 beq.n 24 <bug_test+0x24>
16: 69eb ldr r3, [r5, #28]
18: 2b00 cmp r3, #0
1a: d00a beq.n 32 <bug_test+0x32>
1c: 4642 mov r2, r8
1e: 6812 ldr r2, [r2, #0]
20: 4293 cmp r3, r2
22: d006 beq.n 32 <bug_test+0x32>
24: 3c01 subs r4, #1
26: b2e4 uxtb r4, r4
28: 2c00 cmp r4, #0
2a: d1f1 bne.n 10 <bug_test+0x10>
2c: bc04 pop {r2}
2e: 4690 mov r8, r2
30: bdf0 pop {r4, r5, r6, r7, pc}
32: f3ef 8710 mrs r7, PRIMASK
36: b672 cpsid i
38: f7ff fffe bl 0 <foo>
3c: f387 8810 msr PRIMASK, r7
40: e7f0 b.n 24 <bug_test+0x24>
42: 46c0 nop ; (mov r8, r8)
...
Which seems to be correct in terms of the critical section.
Miro Samek
state-machine.com |
|
