Inline assembly in Arm z_arch_switch_to_main_thread missing clobber list

[smithp35]

Describe the bug
I have been experimenting with compiling Zephyr with clang on Arm, I couldn't run the resulting binary with ninja run due to the stack getting corrupted and traced the problem back to this bit of inline assembly in ARM z_arch_switch_to_main_thread

        /*
	 * Set PSP to the highest address of the main stack
	 * before enabling interrupts and jumping to main.
	 */
	__asm__ volatile (
	"mov   r0,  %0     \n\t"   /* Store _main in R0 */
	"msr   PSP, %1     \n\t"   /* __set_PSP(start_of_main_stack) */
#if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE)
	"cpsie i           \n\t"   /* __enable_irq() */
#elif defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE)
	"cpsie if          \n\t"   /* __enable_irq(); __enable_fault_irq() */
	"mov   r1,  #0     \n\t"
	"msr   BASEPRI, r1 \n\t"   /* __set_BASEPRI(0) */
#else
#error Unknown ARM architecture
#endif /* CONFIG_ARMV6_M_ARMV8_M_BASELINE */
	"isb               \n\t"
	"movs r1, #0       \n\t"
	"movs r2, #0       \n\t"
	"movs r3, #0       \n\t"
	"bl z_thread_entry \n\t"   /* z_thread_entry(_main, 0, 0, 0); */
	:
	: "r" (_main), "r" (start_of_main_stack)
	);

Is missing the clobber list, I think at least r0 is needed here. When compiling the HelloWorld program with clang I get from the object:

MOVW     r0,#:LOWER16: _main_thread_s
MOVW     r1,#:LOWER16: bg_thread_main
MOVT     r0,#:UPPER16: _main_thread_s
MOVT     r1,#:UPPER16: bg_thread_main
STR      r0,[r5,#8]
; bg_thread_main is in r1
MOVW     r0,#:LOWER16: _main_stack
MOVT     r0,#:UPPER16: _main_stack
ADD      r0,r0,#0x400
; _main_stack in r0
MOV      r0,r1 ;;;; r0 clobbered by r1
MSR      PSP,r0
CPSIE    if
MOV      r1,#0
MSR      BASEPRI,r1
ISB
MOVS     r1,#0
MOVS     r2,#0
MOVS     r3,#0
BL       z_thread_entry

When compiled with GCC and some versions we happen not to use r0 for _main_stack but in principle this could happen with some future version. I think that at least r0 should be on the clobber list as this can be overwritten before the MSR PSP, r0.

To Reproduce
This is somewhat tricky as to use a clang cross compiler on Arm requires quite a few changes. I think that this should reproduce with the following:
Steps to reproduce the behavior:

1. cd zephyr/samples/hello_world
2. mkdir build; cd build
3. cmake -GNinja -DBOARD=qemu_cortex_m3 ..
4. ninja
5. rm ./zephyr/kernel/CMakeFiles/kernel.dir/init.c.obj
6. ninja -v (to get the arm-none-gcceabi command)
7. replace arm-none-gcceabi with clang --target=arm-none-eabi
8. Disassemble init.c.obj and look for MSR PSP, r0 in z_cstart
   Expected behavior
   I expect that if the compiler chooses to use r0 for _main_stack it isn't overwritten with MOV r0, %1

Impact
This is just a report of a latent problem in the source, I don't think it will affect anyone using GCC and the default flags right now.

Environment (please complete the following information):

• OS: Linux
• Toolchain A clang 8.0.0 toolchain (tags/RELEASE_800/final) using a GNU Tools for Arm Embedded Processors 8-2018-q4-major) 8.2.1 20181213 (release) [gcc-8-branch revision 267074] for the binutils and libraries.
• Commit SHA or Version used
  I rebased my patches to support clang on 442df97 committed on Thu Jun 6 06:31:43 2019 -0500 with first line "scripts/dts: Remove alias defines for labels"

[ioannisg]

Any update on this @galak ?

[ioannisg]

@smithp35 (if you are still around with this issue), I wonder, could we simply move the mov r0, _main, at the end, right before we use it at the bl instruction? Would that work?

Ah, no, the PSP is changing so we can't guarantee proper LDR from c expressions in stack.