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

Possible bugs in __STREXW and __LDREXW in cmsis_armcc.h and cmsis_armclang.h #1623

Open
henrla opened this issue Apr 26, 2023 · 5 comments
Open

Comments

@henrla
Copy link

henrla commented Apr 26, 2023

CMSIS includes the functions __STREXW and __LDREXW.

cmsis_armcc.h
In cmsis_armcc.h, they are defined using intrinsics like this:

#if defined(__ARMCC_VERSION) && (__ARMCC_VERSION < 5060020)
  #define __STREXW(value, ptr)                                                 __strex(value, ptr)
#else
  #define __STREXW(value, ptr)   _Pragma("push") _Pragma("diag_suppress 3731") __strex(value, ptr)        _Pragma("pop")
#endif
#if defined(__ARMCC_VERSION) && (__ARMCC_VERSION < 5060020)
  #define __LDREXW(ptr)                                                        ((uint32_t ) __ldrex(ptr))
#else
  #define __LDREXW(ptr)          _Pragma("push") _Pragma("diag_suppress 3731") ((uint32_t ) __ldrex(ptr))  _Pragma("pop")
#endif

While in the arm documentation, the following is stated with regards to the two intrinsic calls:

"The compiler does not guarantee to preserve the state of the exclusive monitor. It might generate load and store instructions between the LDREX instruction generated for the __ldrex intrinsic and the STREX instruction generated for the __strex intrinsic. Because memory accesses can clear the exclusive monitor, code using the __ldrex and __strex intrinsics can have unexpected behavior. Where LDREX and STREX instructions are needed, ARM recommends using embedded assembly."

Reference: https://developer.arm.com/documentation/dui0472/k/Compiler-specific-Features/--ldrex-intrinsic

This statement casts doubt on whether __STREXW and __LDREXW works as intended for armcc. Is there a rationale or a specific use-case of the __strex and __ldrex intrinsics? Or are the intrinsics currently expected to have unexpected behavior?

cmsis_armclang.h
In cmsis_armclang.h, they are defined like this:

#define __STREXW        (uint32_t)__builtin_arm_strex

#define __LDREXW        (uint32_t)__builtin_arm_ldrex

While in the armclang documentation, the following is stated with regards to the two intrinsic calls:

"Note that the compiler does not guarantee it will not insert stores which clear the exclusive monitor in between an ldrex type operation and its paired strex. In practice this is only usually a risk when the extra store is on the same cache line as the variable being modified and Clang will only insert stack stores on its own, so it is best not to use these operations on variables with automatic storage duration."

Reference: https://clang.llvm.org/docs/LanguageExtensions.html

This statement casts doubt on whether __STREXW and __LDREXW works as intended for armclang. Is there a rationale or a specific use-case intention of the armclang intrinsics, or is unexpected behavior expected?

Assembly is used in the gcc implementation, so it should work as intended. My concern is with regards to the implementation for armcc and armclang.

Thank you.

@JonatanAntoni
Copy link
Member

Hi @henrla,

There should be no difference in exposure to undefined behaviour when using low-level instructions within a block of C/C++ code. My reading of the advice is to not rely on C/C++ code around exclusive access and rather implement the whole portion in assembly. As a reference you might want to take a look into the bunch of atomic_ helper functions implemented for RTX5, see https://github.com/ARM-software/CMSIS_5/blob/7b8c3f3b2a2a8f4a606eaa5c27fd38a056284373/CMSIS/RTOS2/RTX/Source/rtx_core_cm.h.

The downside of inline assembly is to be not compiler agnostic anymore. This might not be a big issue because Arm Compiler 6, IAR, and GCC, all accept inline GNU Assembly.

Another option would be to rely on standard C library which received atomic support in C11 and C++11. One just needs to be careful when using C/C++ atomics and compile for targets without exclusive access support. Compilation can fail at link stage due to missing low level implementations. One would need to provide implementations that fit the purpose, e.g., using interrupt locks or rtos mutexes to achieve atomicity.

Cheers,
Jonatan

@henrla
Copy link
Author

henrla commented Apr 26, 2023

Hi @JonatanAntoni,

Thank you for your swift reply.

If I understand correctly, you are interpreting the documentation of the intrinsics for exclusive load and store in arm and armclang, to also apply for the assembly instructions STREX and LDREX documented here https://developer.arm.com/documentation/dui0204/j/Cihbghef.

It was my understanding that the functions __STREXW and __LDREXW defined in CMSIS are intended for use in C/C++ code, but given your interpretation of the documentation and how __STREXW and __LDREXW are defined, this does not makes sense. Or am I misunderstanding something with regards to the usage of __STREXW and __LDREXW?

Thank you

Henrik

@JonatanAntoni
Copy link
Member

Hi @henrla,

They are basically intended to be used in C/C++ code. But if the overall code gives the expected behaviour goes far beyond CMSIS visibility. Hence, it's totally up to the user to check if the defines from CMSIS can be used successfully or if one needs to go for another solution.

In RTX5 we don't rely on the intrinsics but give the full implementation in inline assembly. RTX5 was developed for C99 and without considering C11 features. In EventRecorder we decided to go for C11 and <stdatomic.h>, see https://github.com/ARM-software/CMSIS-View/blob/main/EventRecorder/Source/EventRecorder.c#L257-L328. For Cortex-M0/M0+ devices (Armv6-M) which don't have exclusive access instruction we provide plain C code and use interrupt-locks to assure atomicity.

Cheers,
Jonatan

@henrla
Copy link
Author

henrla commented Apr 26, 2023

Thank you for your swift reply @JonatanAntoni.

If I want to use __STREXW and __LDREXW in C/C++ code (which is their intended use-case), how can I check if using these defines in my code gives overall expected behavior, if these defines themselves have unexpected behavior in C/C++ code?

To me, it sounds like their limitation should be documented in CMSIS.

@JonatanAntoni
Copy link
Member

Hi @henrla,

Sure, we can add the links you provided earlier into the documentation.

I guess, you'd need to inspect the disassembly to know if the compiler inserted additional load/store instructions which would break the exclusive access.

Could you enhance the docs and raise your proposal as a PR? That would be really helpful.

Cheers,
Jonatan

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

No branches or pull requests

2 participants