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

Static initialization guards implementation #1586

Merged
merged 2 commits into from
Feb 13, 2016
Merged

Static initialization guards implementation #1586

merged 2 commits into from
Feb 13, 2016

Conversation

igrr
Copy link
Member

@igrr igrr commented Feb 5, 2016

This pull request adds implementation of libsupc++ functions which are used by gcc to protect local static initializers.

Interrupts are disabled for the duration of static initialization. This is not ideal, but i don't see a way to implement this without disabling interrupts. Imagine the following scenario:

int first_millis() {
  static int x = millis();
  return x; 
}

void isr_handler() {
  first_millis();
}

void setup() {
  Serial.begin(115200);
  attachInterrupt(0, &isr_handler, CHANGE);
  delay(10);
  Serial.println(millis() - first_millis());
}

void loop() {
}

Let's consider the case when first_millis is called from setup first, and GPIO interrupt happens at the point when millis is being executed. Execution context is switched to isr_handler, which calls first_millis again. C++ standard requires static initialization to be finished before execution is allowed to proceed further, but since we are now inside an interrupt, there is no way of doing that. ISR has higher priority and millis will proceed only when ISR is finished.

Therefore I think we have to live with interrupts disabled for the duration of static initialization.

This alignment prevents umm_malloc to detect buffer overruns which fall within padding introduced by new/new[]. Allocated memory will be aligned by design of umm_malloc, so we don't need to pad anything here.
Also fixed some formatting/newlines and removed unused header files.
@loopj
Copy link

loopj commented Feb 6, 2016

Looks good to me, just tested this and works great in my (previously broken) app.

@tuxedo0801
Copy link
Contributor

Looks like the issue is gone with this fix. Thanks a lot.

@hallard
Copy link
Contributor

hallard commented Feb 11, 2016

Guys,

I clearly understand this problem, so does this mean now best practices are not using static declaration ? sure we can use them but we're not able to initialize them.

so avoid this

void loop() {
  static unsigned long oldms = millis();
}

And do that ?

unsigned long oldms = millis();
void loop() {
  oldms = millis();
}

is that correct ?

igrr added a commit that referenced this pull request Feb 13, 2016
Static initialization guards implementation
@igrr igrr merged commit 0659b80 into master Feb 13, 2016
@Links2004
Copy link
Collaborator

this code change brakes some of my projects,
they not booting any more, only:

 ets Jan  8 2013,rst cause:2, boot mode:(3,6) 

load 0x4010f000, len 1264, room 16  
tail 0 
chksum 0x42 
csum 0x42 
~ld

the reason is the malloc change, the umm makes problems.

@brankos
Copy link

brankos commented Feb 14, 2016

Latest malloc changes cause Telnet example crash and reboot in loop

Links2004 added a commit to Links2004/Arduino that referenced this pull request Feb 14, 2016
@Links2004
Copy link
Collaborator

check #1630

@igrr
Copy link
Member Author

igrr commented Feb 14, 2016

Yep, I got interrupt locks wrong, testing a fix here.

@richardwilliamson
Copy link

Hello, I don't fully understand how git up works- can you advise whether this is now sorted? How can I go about updating the Arduino IDE to use the new code?

@igrr igrr deleted the cxa_guard branch March 3, 2016 06:39
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

Successfully merging this pull request may close these issues.

7 participants