CMSIS __set_MSP causes generation of undefined instruction when -Os

[Martin Velek]

Hello,

the arm-none-eabi-gcc generates undefined instruction in function IAP when compiled with Os. This function is part of the Bootloader application and enables to update the Cortex-m3 LPC1788 internal Flash. The __set_* functions come from the ARM CMSIS.
I need to switch the MSP as the current SP and adjust the value for safety reason before calling the IAP_internal_* functions. Compiler, Linker commands and Linker script are attached.

The IAP_internal_* functions must be in a special input section otherwise the linker will "garbage" them even with the "used" attribute.

#include <stdint.h>
void IAP(int flash_image) __attribute__ ((section (".rom.bootloader"),used, noreturn));
void IAP_internal_force_backup(void) __attribute__ ((section (".text.nogarbage"),used, noreturn));
void IAP_internal(void) __attribute__ ((section (".text.nogarbage"),used, noreturn));

void IAP_internal(void)
{
 while(1);
}

void IAP_internal_force_backup(void)
{
     while(1);
}

#define BOOTLOADER_ADDRESS ((uintptr_t)(0x00000000))
void IAP(int flash_image)
{
 if(0 == flash_image)
 {
  __set_MSP(*((uint32_t *) BOOTLOADER_ADDRESS));
  __set_CONTROL(0x0);
  IAP_internal_force_backup();
 }
 else
 {
  __set_MSP(*((uint32_t *) BOOTLOADER_ADDRESS));
  __set_CONTROL(0x0);
  IAP_internal();
 }
}

Disassembly of section .bootloader:

00003fc0 <IAP>:
    3fc0: b908 cbnz r0, 3fc6 <IAP+0x6>
    3fc2: 6803 ldr r3, [r0, #0]
    3fc4: e001 b.n 3fca <IAP+0xa>
    3fc6: 2300 movs r3, #0
    3fc8: 681b ldr r3, [r3, #0]
    3fca: deff udf #255 ; 0xff

[Andre Vieira (andre-simoesdiasvieira)]

Hi Martin,

It seems like you did not include the right CMSIS header file. The definition of __set_MSP, as well as other CMSIS functionality, depends on it. Without '-Wimplicit-function-declaration' GCC will silently accept any undefined intrinsic and any O2 or higher optimization will translate it to an undefined instruction.

Best Regards.

[Martin Velek (martin-velek)]

Hello,

negative. I have compiled it with the -Wimplicit-function-declaration and the result is the same.
Without the -Os, the compiler generates correct instructions.
00003fc0 <IAP>:
    3fc0: b508 push {r3, lr}
    3fc2: 2200 movs r2, #0
    3fc4: 4b0a ldr r3, [pc, #40] ; (3ff0 <IAP+0x30>)
    3fc6: 605a str r2, [r3, #4]
    3fc8: f3bf 8f4f dsb sy
    3fcc: f3bf 8f6f isb sy
    3fd0: b930 cbnz r0, 3fe0 <IAP+0x20>
    3fd2: f382 8814 msr CONTROL, r2
    3fd6: 6813 ldr r3, [r2, #0]
    3fd8: f383 8808 msr MSP, r3
    3fdc: f7fd fac6 bl 156c <IAP_internal_force_backup>
    3fe0: 2300 movs r3, #0
    3fe2: f383 8814 msr CONTROL, r3
    3fe6: 681b ldr r3, [r3, #0]
    3fe8: f383 8808 msr MSP, r3
    3fec: f7fd faae bl 154c <IAP_internal>
    3ff0: e000ed90 .word 0xe000ed90

I would bet it has something to do with the stack optimalisation. I agree, it is not polite to change stack address in a C function. Anyway, the compiler should at least issue a warning that undefined instruction is generated.

[Thomas Preud'homme (thomas-preudhomme)]

Hi Martin,

As I understand it, -Wimplicit-function-declaration will not prevent unknown intrinsic from being assimilated to an undefined instruction but just warn when that happens. So if you had a warning when compiling your code with that option, the solution should be to include CMSIS header file.

Best regards.

[Martin Velek (martin-velek)]

Hi,

I am sorry, also the -Werror-implicit-function-declaration flag does not generate any warning or error. That's my fault during stripping the code, I did not include the CMSIS header file in the posted code. Now the code below is a completed one. It should be possible to recreate the issue by now.

----------------------------------
#include <stdint.h>

__attribute__( ( always_inline ) ) static inline void __set_CONTROL(uint32_t control)
{
  __asm volatile ("MSR control, %0" : : "r" (control) : "memory");
}

__attribute__( ( always_inline ) ) static inline void __set_MSP(uint32_t topOfMainStack)
{
 __asm volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
}

void IAP(int flash_image) __attribute__ ((used, noreturn));
void IAP_internal_force_backup(void) __attribute__ ((used, noreturn));
void IAP_internal(void) __attribute__ ((used, noreturn));

void IAP_internal(void)
{
 while(1);
}

void IAP_internal_force_backup(void)
{
     while(1);
}

#define BOOTLOADER_ADDRESS ((uintptr_t)(0x00000000))
void IAP(int flash_image)
{
 if(0 == flash_image)
 {
  __set_MSP(*((uint32_t *) BOOTLOADER_ADDRESS));
  __set_CONTROL(0x0);
  IAP_internal_force_backup();
 }
 else
 {
  __set_MSP(*((uint32_t *) BOOTLOADER_ADDRESS));
  __set_CONTROL(0x0);
  IAP_internal();
 }
}

void _start(void)
{
 int flash_image;
 IAP(*((int *) 0x000447));
 return;
}

----------------------------------
Martin

[Andre Vieira (andre-simoesdiasvieira)]

Hi Martin,

We are now able to reproduce your issue. The problem here lies in the fact that you are casting value 0 to a pointer type, which according to the C99 standard yields a 'null pointer'. You then dereference this null pointer which yields undefined behavior.

The -fisolate-erroneous-paths-dereference, which now gets turned on for O>2, "detects paths that trigger erroneous or undefined behaviour due to dereferencing a null pointer. Isolate those paths from the main control flow and turn the statement with erroneous or undefined behavior into a trap."

The best option here is probably to avoid having the compiler see this as undefined behavior, that means you need to "obscure" the value of BOOTLOADER_ADDRESS. I suggest making it an uninitialized global variable, maybe even with the 'extern' keyword, and defining it as 0 in your linker script.

For this particular example turning off the aforementioned pass also 'works', but there might be other passes that recognize this as undefined behavior and do something you are not expecting them to.

Best Regards,

[Martin Velek (martin-velek)]

Hi,

thank You very much for the explanation, I was not aware of this. For those reading this ticket, dereferncing a NULL pointer, in my case, is a way how to get the initial stack pointer address. The LPC1788 MCU (cortex-m3 inside) has at offset 0 from address 0x00000000 a value of the stack pointer.

BR
Martin

[Martin Velek (martin-velek)]

This bug is related to the C99(11) standard and in actual fact it is not a bug but gcc correct behavior according to the standards.