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

Macro/global struct conflicts from CMSIS #488

Open
henrikssn opened this issue Oct 5, 2020 · 3 comments
Open

Macro/global struct conflicts from CMSIS #488

henrikssn opened this issue Oct 5, 2020 · 3 comments

Comments

@henrikssn
Copy link
Contributor

I recently ran into issues where e.g. the declaration

MODM_ISR(RTC)

conflicts with a macro from CMSIS header: https://github.com/modm-io/cmsis-header-sam/blob/9f08123fc9c504cfe99928fb8f125e149c815e80/samd21a/include/samd21g18a.h#L395

Also, CMSIS defines objects named Rtc and RTC_ in global namespace which is very annoying as those names cannot be used for namespaced classes, template args, etc in any code that transitively depends on modm. This is also an issue for at least USB and EIC peripherals, there are probably more examples.

Does anyone have ideas on a mitigation? Could we integrate the CMSIS defs into modm-devices and (at least partially) get rid of CMSIS?

@salkinium
Copy link
Member

salkinium commented Oct 6, 2020

Does anyone have ideas on a mitigation?

In the short-to-medium term you can only really push/pop the offending macro. Of course not a very scalable solution.

#pragma push_macro("RTC")
#undef RTC

// use RTC

#pragma pop_macro("RTC")

Read on for the long term plan ;-P

the CMSIS defs

There are an official solution: CMSIS SVD files, which describe the memory map in detail in a machine readable way via XML: https://github.com/posborne/cmsis-svd

However, there's a very big issue at least for the STM32 SVD files: they are incomplete, wrong and differ from the official CMSIS STM32 header files and even the Reference Manuals by spelling mistakes, flipped bits, etc… the typical errors you'd expect from manual transcription!!!

This makes some sense, because the SVD files are typically only used the debugger to identify an address as a peripheral register and give the name and meaning of it, allow on-the-fly config etc…
Which also means they are never actually used in actual C/C++ source code, so the mistakes are not discovered by the "real world" CI aka. human engineers.

Note that because Rust, Ada, Go cannot use the vendor provided header files, they all use the SVD files to generate their language-specific headers, which are all pretty cool, but they need to manually fix these issues, which is not a great solution:
https://github.com/stm32-rs/stm32-rs

There are also some visualizers that use this data to present you an overview of peripherals and their registers:

(There used to be one memmap comparator built right into modm-devices, but I apparently removed most of it… 😬)

Could we integrate the CMSIS defs into modm-devices

I thought about this problem and decided that the only sensible database to use are the real CMSIS header files.
So I wrote a parser that converts the CMSIS header definitions into a Python dictionary. It compiles the header definitions into a GCC executable, which prints out the values as a python dictionary, which is exec() back into Python. (I didn't want to deal with macro expansion code and conditional defines).

It works really well, but it's output currently unused. Here is an out-of-date dump of the memory maps of all STM32 devices. (Excuse the device file format, it's not a great fit for this data).
I intended to integrate this data into modm-devices as an additional query for certain features, like "if this device have this bit, then I need to choose a different implementation", but I just never got there.

and (at least partially) get rid of CMSIS?

You could use the parsed header data to generate a new header file, which is properly namespaced. You could use this inside modm without issue, but to third-party and outside code, you will still have to provide the official header, which would then cause the same issues again.

It would make sense to integrate the parsing into modm-devices, so that we can also derive additional data from the header files into the device files, so that the header interpretation logic for common things isn't spread across a million module.lb files.

@henrikssn
Copy link
Contributor Author

Does anyone have ideas on a mitigation?

In the short-to-medium term you can only really push/pop the offending macro. Of course not a very scalable solution.

#pragma push_macro("RTC")
#undef RTC

// use RTC

#pragma pop_macro("RTC")

Great tip! Unfortunately this doesn't work for the typedefs... Any ideas how I can deal with those? I.e.

typedef struct {
// ...
} Usb;

Read on for the long term plan ;-P

Big thanks for the detailed answer! Well, this is not on the top of my list of annoying things, although the CMSIS designers must have done a quite good attempt of getting there. First typedef:ing Usb in global namespace, and then cover any workarounds with defining a macro for USB. Really!!??

but to third-party and outside code, you will still have to provide the official header

One solution to that is to not use third-party code, but that is quite a big hammer for something that is just annoying.

I guess I stick to staying clear from the CMSIS names in the short/medium term. We should also consider applying that practice in modm code base.

@salkinium
Copy link
Member

salkinium commented Oct 7, 2020

Any ideas how I can deal with those? I.e.

Nope, that just screws everyone over. ¯\_(ツ)_/¯

Perhaps we could hack it a little in <modm/platform/device.hpp>?

namespace modm::device
{
// We must avoid extern "C" to hoist the typedefs into a namespace
#pragma push_macro("__cplusplus")
#undef __cplusplus

#include "cmsis_header_sam_xyz.hpp"

#pragma pop_macro("__cplusplus")
}

But even then, once the user includes cmsis_header_sam_xyz.hpp manually in their code (for example to integrate third-party code) this will break everything again. There are also some subtle differences between the way that C and C++ define things in headers, so this might not even compile without extern "C".

We should also consider applying that practice in modm code base.

STM32 doesn't have this issue, because the typedefs are called USB_TypeDef etc and the define is then called USB. So we only ever have to deal with the annoying macro, not also the type.

This is a complete mess and I'm always a little jealous of languages like Rust, Ada, Go that can define their own interface, because they don't have a de-facto standard to deal with. We do have to deal with CMSIS very much (but most of the non-vendor specific sutff is quite sensible actually).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants