Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed direct use of r7 in inline asm, which caused GCC to abort as it is used as a frame pointer. #472

Closed
wants to merge 1 commit into from

Conversation

tai
Copy link

@tai tai commented Aug 31, 2014

I initially reported this on mbed.org forum (http://mbed.org/forum/bugs-suggestions/topic/5112/), and was instructed to re-send it as a pull request. So, here it is.

Patch Description

I'm currently doing offline development with the mbed SDK, and noticed it fails to build under certain condition with a following error:

# python build.py -t GCC_ARM -m LPC1114 -o debug-info -r
...
Building library RTX (LPC1114, GCC_ARM)
Compile: rt_CMSIS.c
[Error] rt_CMSIS.c@588: In function 'osKernelInitialize': r7 cannot be used in asm here
c:\src\mbed\libraries\rtos\rtx\rt_CMSIS.c: In function 'osKernelInitialize':
c:\src\mbed\libraries\rtos\rtx\rt_CMSIS.c:588:1: error: r7 cannot be used in asm here
 }
 ^

Culpit is the following code in rt_CMSIS.c:

#if (defined (__CORTEX_M0)) || defined (__CORTEX_M0PLUS)
#define SVC_Call(f)                                                            \
  __asm volatile                                                                 \
  (                                                                            \
    "ldr r7,="#f"\n\t"                                                         \
    "mov r12,r7\n\t"                                                           \
    "svc 0"                                                                    \
    :               "=r" (__r0), "=r" (__r1), "=r" (__r2), "=r" (__r3)         \
    :                "r" (__r0),  "r" (__r1),  "r" (__r2),  "r" (__r3)         \
    : "r7", "r12", "lr", "cc"                                                  \
  );
#else

Cortex-M0 LDR has a limitation that it cannot directly load value into upper register (r12), and that's why r7 is being used as a temporary register. However, that breaks GCC on debug build since GCC is also using r7 as frame pointer.

An easy fix would be to change r7 into something else in lower register, say r6. However, to avoid writing temporary register name in asm, my patch allows compiler to allocate available register.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 1, 2014

Thanks for this bugfix.

Can you please sign the contribution agreement, https://mbed.org/contributor_agreement/

@tai
Copy link
Author

tai commented Sep 1, 2014

Sure. Please do the pull as I accepted the contribution agreement just now.

@bogdanm
Copy link
Contributor

bogdanm commented Sep 2, 2014

Thanks for your contribution. I understand the problem, but I don't understand why we need a frame pointer in the first place. Isn't it easier/more efficient to tell gcc to omit the frame pointer?

https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html

@bogdanm
Copy link
Contributor

bogdanm commented Sep 11, 2014

I am going to simply use '-fomit-frame-pointer' for now (I've checked that this fixes the problem), since I don't see any problems with this at the moment and it seems to be a more efficient/safer fix; if this breaks something in the future, I'll re-consider this change.

@bogdanm bogdanm closed this Sep 11, 2014
bogdanm pushed a commit that referenced this pull request Sep 12, 2014
bogdanm pushed a commit that referenced this pull request Sep 12, 2014
yogpan01 pushed a commit to yogpan01/mbed that referenced this pull request Jul 21, 2016
[CircleCi] Adding --no-commit to the git merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants