Make init() run before C++ static initializers #5592
Conversation
|
This is a good idea, and essentially the same thing I do in my SmallSetup lib. However, for a permanent option I think this needs to be extended to the other cores (SAM, SAMD at least) to keep a level of consistency. Otherwise we're making the requirements of the high-level API dependent on the low-level features. This will make the abstraction less.. abstract. One option I have thought about is making the init() function re-entrant. So all classes that need it can call it from their constructor, but it'll only run once. This will make it easy to port between hardware and will solve the problem addressed in this issue. Using a static bool to store the state of the init() function and by disabling interrupts, it should be safe to call where ever it may be required. |
|
Can't you just rename the existing init() to something else, which you call yourself as you see fit, and let init() be empty? |
|
@Chris--A: I agree, this has little value unless ported to all cores. I'll look at doing this once I get some more feedback on whether this is a good idea.
That seems like a bit of a workaround, and requires more thinking on behalf of the user. Also, since static initialization order is undefined, that can put you in a position where a class that forgets to call @bengtmartensson: I'm not sure what you're asking for here. This PR aims to make it guaranteed that all user code runs after |
Am I right in thinking the only other core is |
|
@eric-wieser When should interrupts be enabled? I suspect that that after the static initializations is a good and safe place?? |
|
@mikaelpatel: Very good point. It depends. If constructors hook up their own interrupt handlers with On the other hand, if any interrupts are directly hooked into the linker with assembler attributes ( In any case, |
|
@eric-wieser I saw that the new init had sei and this got me thinking of possible ripple effects. Otherwise I like the idea of forcing init before static constructors and will be adding this to Cosa where I have looked for a good solution; https://github.com/mikaelpatel/Cosa |
Oh damn. That's gotta move. |
|
Might be worth mentioning ARM Cortex-M's PRIMASK defaults to clear at boot time, which allows interrupts by default. This is different from AVR, where the global interrupt enable GIE bit defaults to zero, which inhibits all interrupts by default, until it's set by software. On AVR, the GIE bit serves at least 3 functions:
On ARM Cortex-M chips, generally #1 is done by a set of registers within the interrupt controller, where each bit corresponds to 1 of the interrupts. PRIMASK is meant for #2. The Cortex-M interrupt controller implements a nested priority scheme in hardware, which is separate from these other two register mechanisms. Many people who are very familiar with the GIE bit in AVR often lump these 3 very distinct usages together, since the simple hardware in AVR (and PIC) chips provides just 1 global bit. When designing for ARM Cortex-M chips, it's a common mistake to think of PRIMASK this way. It's really only meant for case #2, and by default it allows as interrupts from the moment the ARM core resets. |
facchinm
added
Architecture: AVR
|
Partially moved to arduino/ArduinoCore-sam#4, the patch will need to be rebased once SAM core gets detached |
|
I'd love to create a build with this PR but the SAM part must be removed so solve the conflict. @eric-wieser could you rebase it on master? Thanks |
Before this change, the contents of `init()` run after `.init6`, which is when static initializers run. However, this is not desirable: * init() does not need any C++ classes to be active * C++ static initialization sometimes _does_ require that the hardware be pre-configured! This makes it possible to make calls like `Serial.begin()` inside constructors of global variables.
|
Done |
|
@ArduinoBot build this please |
|
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-5592-BUILD-691-linux32.tar.xz ℹ️ The |
|
Is there anything that can be done to move this along? |
|
Closing here; if someone wants to adopt this PR, please reopen on ArduinoCore-avr repo suing this link https://github.com/arduino/ArduinoCore-avr/compare/pr_5592?expand=1 |
Before this change, the contents of
init()run after.init6which is when static initializers run.However, this is not desirable:
This makes it possible to make calls like
Serial.begin()inside constructors of global variables.