STMicroelectronics / STM32CubeWB

Full Firmware Package for the STM32WB series: HAL+LL drivers, CMSIS, BSP, MW, plus a set of Projects (examples and demos) running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits).
https://www.st.com/en/embedded-software/stm32cubewb.html
Other
229 stars 139 forks source link

`__ASM`, `__asm` and `asm` #34

Closed atsju closed 1 year ago

atsju commented 3 years ago

I want to build the sources from STM32cubeIDE examples but without using STM32CubeIDE. During the process I came across an issue that lines like register char * stack_ptr asm("sp"); won't build with standard arm-none-eabi-gcc flag -std=C99. During my research I understood that STM32CubeIDE is using flag -std=GNU99. In order to build with -std=C99, asm needs to be replaced by __asm. As __asm works with both options maybe it would be great to update all examples. Even better would be to use __ASM instead with use of CMSIS_compiler.h to be even more generic.

Additional links for reference: Why the code doesn't build in my case GNU99 VS C99

I know this is a broad change and it affects all HALs.

RKOUSTM commented 3 years ago

Hi @atsju,

Thank you for your contribution. Your report will be forwarded to our development team. We will get back to you as soon as we have more details.

Thank you once more for this contribution.

With regards,

RKOUSTM commented 3 years ago

Hi @atsju,

Thank you for your reports. Actually, the ASM is defined asm in the CMSIS_compiler.h file like as follows:

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Drivers/CMSIS/Include/cmsis_compiler.h#L70-L72

In order to allow a better analysis of this problem, could you please give us more details about the issue.

With regards,

atsju commented 3 years ago

Actually, the ASM is defined asm in the CMSIS_compiler.h file like as follows:

Actually it is NOT. See here that you did not take correct file as I am using arm-none-eabi-gcc and you are linking me to some definition of your direct concurrent __TI_ARM__ : https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Drivers/CMSIS/Include/cmsis_compiler.h#L33

Let me know if you need additional information

RKOUSTM commented 3 years ago

Hi @atsju,

As you are using the __CC_ARM, the __asm is defined in the cmsis_armcc.h included file :

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Drivers/CMSIS/Include/cmsis_armcc.h#L57

With regards,

atsju commented 3 years ago

First to answer your question I use arm-none-eabi-gcc thus this is applied :

#elif defined ( __GNUC__ )
  #include "cmsis_gcc.h"

secondly let me re-explain the issue asm must be replaced because not defined when using flag -std=GNU99 with arm-none--eabi-gcc.

Now ST needs to solved and use __asm instead of asm. And because you are the experts at ST and whant the code to compile with as much compiler as possible you will use __ASM then the file cmsis_compiler.h is able to manage the define automagically. This way you do not need to ask customer about what compiler they use anymore.

Is this clear enough or do you want me to share with you on github an example based on ST code that will reproduce the issue ?

RKOUSTM commented 3 years ago

Hi @atsju,

Thank you for your report. Regarding the point for which you opened the issue, the cmsis_gcc.h file is actually using the __asm as follows:

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Drivers/CMSIS/Include/cmsis_gcc.h#L39-L42

However, the reported point is out of the scope of STMicroelectronics' activity as the concerned piece of software is a third party's one. These files come from Arm and we use them as is. Unfortunately we don't treat aspect related to Third_Party files in our GitHub repositories. As these files are out of the scope of STMicroelectronics, any modifications concerning these files are not implemented by our technical team.

Please allow me to close this issue now. Thank you again for your report and for your comprehension. We are looking forward to reading from you again.

With regards,

atsju commented 3 years ago

Sorry if I have been unclear but I do NOT request you to have third part code modified. Allow me to write it in different way because I'm not native English speaker and I feel we do not understand each other.

You shall in ST code use __asm or even better use __ASM instead of using asm.

In ST code replace asm by __ASM to be more portable/generic.

Make use of __ASM instead of asm.

ALABSTM commented 3 years ago

Hi @atsju,

If we understand correctly, you are pointing out the use of keyword asm in the following lines found exclusively within both the syscalls.c and the sysmem.c files of the different applications and examples:

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Projects/NUCLEO-WB15CC/Applications/FreeRTOS/FreeRTOS_Mutexes/STM32CubeIDE/Application/User/syscalls.c#L41

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Projects/P-NUCLEO-WB55.Nucleo/Applications/FreeRTOS/FreeRTOS_Mutexes/STM32CubeIDE/Application/User/sysmem.c#L30

https://github.com/STMicroelectronics/STM32CubeWB/blob/f8793c3e2ab6d2c84897405a6fcf529703185063/Projects/P-NUCLEO-WB55.Nucleo/Applications/FreeRTOS/FreeRTOS_Mutexes/STM32CubeIDE/Application/User/sysmem.c#L40

Your point is to replace keyword asm by keyword __asm to be fully compatible with the C99 standard.

Your request will be forwarded to our development teams. We will be back to you as soon as we have their feedback. Thank you for your patience and thank you again for having reported this point.

With regards,

ALABSTM commented 3 years ago

ST Internal Reference: 117210

ALABSTM commented 1 year ago

Hi @atsju,

Thank you again for having reported this point. It has been fixed in version 1.15.0. Now neither syscalls.c nor sysmem.c files use inline assembly anymore.

With regards,

atsju commented 1 year ago

Thank you for fixing. Could you please do the same for all affected families ? (hoping it won't take 1Y per family)

for example

https://github.com/STMicroelectronics/STM32CubeF0/blob/ee2ae4f522c26f26670d5f8873ce818122796f9f/Projects/STM32072B_EVAL/Examples/BSP/SW4STM32/syscalls.c#L28

could simply be

#include "cmsis_compiler.h"
register char * stack_ptr __ASM("sp"); 

Using cmsis_compiler.h will let it work with any compiler.

I have detected the exact same anomaly in F0 F1 F2 F3 F4 F7 G0 G4 L0 L1 L4 MP1 and WL families. maybe U5 and WBA in some limited ST files