-
-
Notifications
You must be signed in to change notification settings - Fork 722
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
Statup code: Fix zeroing of section .bss, improve initialization of .bss & .data sections #712
Open
merethan
wants to merge
3
commits into
arduino:master
Choose a base branch
from
merethan:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…defined rather than .data start & end being defined (this was likely a copy-pasta error when re-purposing code from the .data section above it to make the .bss piece). Also, the pointer jumps forward 4 bytes at a time now rather than one byte, given that pSrc and pDest are pointers to a uint32_t, meaning memory is written 4 bytes at a time. A bunch of processor architectures (especially the simple ones) can only write int32 values to a memory address aligned at a 4-byte boundary, and int16 values aligned at a 2-byte boundary. If one writes C/C++ code not taking this into account, the compiler is forced to generate all sorts of shifts and tricks to make that code work, which is very suboptimal code by every measure. (In this case suboptimal means having every byte but the first and last three written 4 or more times over and some extra space in flash memory for the tricks.) If one were to do have written the init of .data and .bss in assembly (meaning the compiler not saves your ass by inserting shifts & tricks to your doing it per-byte) this would have been a HardFault on most Cortex CPUs due to the alignment requirement.
Memory usage change @ bdd0b63
Click for full report table
|
…ino.cc> on Tue Oct 11 18:36:08 2016 (ce89e7e). Why: Construction of global objects is something which needs be done prior running main(). It has been like this since the conception of Unix and the C language that was created to write Unix in and since then standardized in the Portable Operating System Interface (POSIX): Initialization of memory is done by entry point _start() which does stuff like loading commandline arguments from the stack and runs main() with it, and calls _exit() on return of main(). The function _exit() in the old-time days of mainframes was responsible for flushing buffers and relinquish all other resources (memory, sockets etc.) such the next user/program can use them (nowadays operating systems are smarter and got your back if programs fail to do this properly). On embedded systems (such as those running Arduino) there is no program loader or operating system, meaning these duties are to be done by what is commonly referred to as the Reset Handler, being the entry point of the program like _start() is on a POSIX-compliant machine. By moving the duties of _start() to main() itself a variety of problems may arise involving a variety of static initialization problems that are very hard to spot. And if not today there will be in the future because the current behavior is not expected. (It is anti-logical; things that are to be done prior main() should not be in main() itself.) If this breaks libraries, it means these unnamed "some libraries" are broken: Things like hardware init should be in begin(), not a class constructor. And most definitely should one class constructor never rely on another class constructor having done something already, like initializing hardware. This is because the compiler has no good control over what class is initialized first: All the compiler can do is specify an order of importance, which the linker then must be instructed to follow, which the C library then needs to run correctly in that order. None of the problems that occur due to one of those steps going not as expected are easy to spot or in any way obvious from C/C++ code. Or easy to fix for that matter. And none of this is formally specified and hence implementation-specific. Type "The Static Initialization Order Fiasco" into a search engine of your choice. Or, to cut this story down to the shortest possible way to put it: Constructors are for creating constructs in memory, nothing else. Procedural code does not belong there, it should be in begin(). Any library needing Cristian Maglie's hack is broken and should be fixed. Compromising Arduino core is not a fix.
Memory usage change @ e80e035
Click for full report table
|
Memory usage change @ 4331996
Click for full report table
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Zeroing of section .bss is now conditional on .bss start & end being defined rather than .data start & end being defined (this was likely a copy-pasta error when re-purposing code from the .data section above it to make the .bss piece). Also, the pointer jumps forward 4 bytes at a time now rather than one byte, given that pSrc and pDest are pointers to a uint32_t, meaning memory is written 4 bytes at a time. (Startup code by ARM written in assembly also does this.)
A bunch of processor architectures (especially the simple ones, like the M0 and M4) can only write int32 values to a memory address aligned at a 4-byte boundary, and int16 values aligned at a 2-byte boundary. If one writes C/C++ code not taking this into account, the compiler is forced to generate all sorts of shifts and tricks to make that code work, which is very sub-optimal code by every measure. (In this case sub-optimal means having every byte but the first and last three written 4 or more times over and some extra space in flash memory for the tricks.) If one were to do have written the initialization of .data and .bss as presented prior this patch in assembly (meaning the compiler not saves your ass by inserting shifts & tricks to your doing it per-byte) this would have been a HardFault on most Cortex CPUs due to the alignment requirement.