Possible bugs in `__STREXW` and `__LDREXW` in cmsis_armcc.h and cmsis_armclang.h
henrla opened this issue · 5 comments
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.
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
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
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
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.
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