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

Resources consumed by HardwareSerial even when not actually used #11597

Open
brycecherry75 opened this issue Jun 15, 2021 · 3 comments
Open

Resources consumed by HardwareSerial even when not actually used #11597

brycecherry75 opened this issue Jun 15, 2021 · 3 comments
Labels
Component: Core Related to the code for the standard Arduino API Type: Bug

Comments

@brycecherry75
Copy link

brycecherry75 commented Jun 15, 2021

My library contains serial communication function calls referring to HardwareSerial (not included at the level of my library) and when I include my aforementioned library, program memory and global registers related to HardwareSerial are used even though none of the functions in my library which specifically use serial communication are actually used.

Here is part of the .cpp file from my library in question:

void MyLibrary::MyFunction(uint32_t value) {
  if (Serial.available() > 0) { // this line will direct the compiler to include HardwareSerial and use resources even though MyFunction is not used
  }
@per1234 per1234 added Component: Core Related to the code for the standard Arduino API Type: Bug labels Jun 15, 2021
@matthijskooijman
Copy link
Collaborator

@brycecherry75 What core/board are you using?

For AVR, there is some code in place that ensures that serial objects are not included in the build unless they are referenced. For most other cores, this does not happen (so typically serial objects are always included, or sometimes all serial objects are included if one is referenced).

For the particular case you show, though, that the serial object is referenced from a function, but that function itself is unused, I can imagine that the compiler fails to handle this case. Serial objects in this context are a bit special, since in addition to an object that can be optimized as normal, there is an interrupt handler that is not called directly, but must be defined when it is enabled. To prevent the ISR from being optimized away, it gets the "used" attribute, preventing it from being optimized away. To ensure that the ISR and serial object are not always included in the build, even when they are unused, the AVR core uses some trickery that takes advantage of how the linker pulls objects from the core.a library file only when needed. In this case, I can imagine that, because there is a reference to Serial, the Serial object is pulled from core.a and included in the build, and only after that, the compiler realizes that MyFunction is unused and removes it (but because of the "used" attribute on the ISR, the serial object can no longer be removed).

In other words, if you are indeed using the AVR core and if removing the reference to Serial entirely prevents the code from being included, then I'm afraid that your case is not supported (and more importantly, cannot be easily supported).

Though thinking about this, one thing that could maybe work is if we could somehow replace the "used" attribute on the ISR with an explicit dependency from the Serial object and/or constructor to the ISR (i.e. make it as if the ISR is called from the serial object), so the optimizer can actually make a meaningful choice whether or not to include the ISR or not. I'm not sure if this is at all possible without actually calling the ISR (or taking its address), but maybe there is some C++ or gcc trick for this? I do not have time to look into this right now (or soon), though.

@matthijskooijman
Copy link
Collaborator

Though thinking about this, one thing that could maybe work is if we could somehow replace the "used" attribute on the ISR with an explicit dependency from the Serial object and/or constructor to the ISR (i.e. make it as if the ISR is called from the serial object), so the optimizer can actually make a meaningful choice whether or not to include the ISR or not. I'm not sure if this is at all possible without actually calling the ISR (or taking its address), but maybe there is some C++ or gcc trick for this? I do not have time to look into this right now (or soon), though.

I gave this a little more thought today. I think that you could add the explicit dependency with some volatile asm statement that takes the address of the ISR as an input operand. This will probably still generate an explicit load of a literal (the ISR address), but that seems acceptable overhead (maybe you could even mask off all but the lowest bits of the address to ensure the load is just a single short instruction, but I'm not sure if the linker supports this). Assigning the address to a volatile pointer variable could be an alternative, but I think this will be optimized away if the variable is not otherwise used.

I was thinking that this dependency should be added from the HardwareSerial::begin function (or whichever function actually enables the IRQ), since that would be the most semantically correct dependency (and even allow referencing Serial methods without actually including the ISR until you also call begin()). However, the begin function is of course shared between all HardwareSerial instances, so you wouldn't know which ISR to depend on. You could solve this by creating a subclass for each instance and overriding begin() to add this dependency (since when calling i.e. Serial1.begin() the compiler will know the actual (sub)class of Serial1 and can call a specific begin method), but that still breaks when generic code accepts a HardwareSerial* and calls begin on that, which would bypass the subclass' begin methods. Maybe this could be solved by overriding the cast-to-HardwareSerial* operator (and probably also for references) in each subclass and adding the dependency also to this operator (so you get the Serial1 ISR when you either call e.g. Serial1.begin(), or when you convert Serial1 to a `HardwareSerial*, but I'm not actually sure this would work and it will likely be fragile.

Simpler is to just add the dependency in the constructor, since that will also be called by the compiler but can also be optimized away when all references to the serial object are optimize away. Note that this still requires a separate subclass for each instance, but that's ok. The downside is that if you somehow reference a Serialx object, but do not actually call begin() on it, the ISR will still be included, but I guess that's acceptable.

@matthijskooijman
Copy link
Collaborator

I did a quick experiment, and it turns out that just removing the used attribute will not actually cause it to be removed from the link. Looking at the AVR ISR definition, it also adds the externally_visible attribute, which seems to prevent LTO from discarding the symbol, probably because it assumes that there might be unseen references to it (e.g. from assembler files). I've tried removing the externally_visible attribute, which indeed causes the symbol to be discarded by LTO, but somehow it seems that the linker still tries to reference the ISR (instead of falling back to the weakly defined default handler in e.g. crtatmega2560.o) and then raises:

`__vector_25' referenced in section `.vectors' of 7.3.0-atmel3.6.1-arduino7/bin/../lib/gcc/avr/7.3.0/../../../../avr/lib/avr6/crtatmega2560.o: defined in discarded section `.text' of /tmp/ccTwK7Nu.o (symbol from plugin)

I'm not sure what happens exactly, maybe the linker already decided to override the weak version with the strong version before LTO, or maybe it still considers the strong version as a valid option even though it is discarded by LTO?

Anyway, I'm out of time for this project (I thought I could maybe quickly whip something up, but it doesn't seem to be that easy...).

Here's the files I've been experimenting with (using plain avr-gcc to keep things simple).

main.cpp:

#include <avr/io.h>
#include <avr/interrupt.h>

//extern "C" void USART0_RX_vect() __attribute__((signal, externally_visible, used));
extern "C" void USART0_RX_vect() __attribute__((signal));
void USART0_RX_vect() {
}

extern "C" void foo() __attribute__(());
void foo() {
}

int main() {
        return 0;
}

Makefile:

#CC=/usr/bin/avr-gcc
CC=~/.arduino15/packages/arduino/tools/avr-gcc/7.3.0-atmel3.6.1-arduino7/bin/avr-gcc

test: main.cpp Makefile
        $(CC) -v -flto -Os -g -mmcu=atmega2560 $< -o $@

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Related to the code for the standard Arduino API Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants