arm: ld: place .bss at the end #53262
Conversation
This is to avoid the binary padding caused by placing sections after the .bss noload gap, which causes the binary to be unnecessarily large. By placing the noload .bss at the end, we do not have to include it in the binary. Signed-off-by: Théophile Ranquet <[email protected]>
zephyrbot
requested review from
MaureenHelm,
bbolen,
carlocaione,
galak,
microbuilder and
povergoing
There was a problem hiding this comment.
We did basically the same thing for Cortex-M a while back. At the time, doing so broke userspace support, not sure if that is still the case -- see #21747.
|
@microbuilder can you take a look at this or reassign? |
|
@ibirnbaum @bbolen Can you please take a look? |
Sorry, was waiting for feedback on comment from Stephanos, and fell off my radar since it wasn't tagged for 3.3. |
|
@MaureenHelm @stephanosio Can this wait until after the 3.3 release? Trying to sort out a workable solution to the TF-M release blocker issue. |
Yes, this is not for 3.3. |
|
As it happens, without remembering the existence of this PR, I ran into the same issue a few days ago after being generous with several heap settings and discovering that the resulting .bin image was unreasonably large. I'll cross-check with my modifications later today at the office, I think the padding issue wasn't just limited to .bss, but also to .noinit. It should also be checked if __data_region_end is somehow relevant for the MMU setup, as with the proposed change, the .bss section is now within the area marked with _image_ram_end / _end / z_mapped_end, but beyond whatever is supposed to be indicated with __data_region_end. |
There was a problem hiding this comment.
Moving just the .bss section to the end of the image doesn't fully resolve the padding issue, as memory areas such as thread stacks or heaps are placed in the .noinit section, which, at the time being, is located just behind .bss and therefore in the middle of the image.
A quick test with an application which reserves heaps with an approximate total size of 30 MB resulted in the following image sizes:
- Linker command file as-is: 32.462.776
- .bss moved to the end of the image: 22.977.152
- both .bss and .noinit moved to the end of the image: 1.789.480
Therefore, the include statement for common-noinit.ld must be moved as well in order to fully resolve the padding issue.
With regards to the __data_region_end marker however, the behaviour is likely correct. This marker is used during startup in XIP operation mode when the initialized data sections are copied from flash to RAM. If the .bss and .noinit sections aren't contained in the flash, there's no point in copying random data from flash to RAM. Instead, .bss will be zeroed during early boot and .noinit is just left untouched.
|
@yroeht Have time to reply to the feedback from @ibirnbaum ? |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
@povergoing Assigning this to you, hoping that you and @SgrrZhf can address this as part of #60031 since the current PR has gone unresponsive. |
|
@microbuilder @povergoing As this is a relatively small and quick fix, I'd suggest that I open a separate PR later tonight which replaces this one. We can easily merge this change before tackling 'the big one'. |
Sure, and I saw that @ibirnbaum has already raised a fix for this.
Agreed, really appreciate it. |
This is a follow up to zephyrproject-rtos#53262, which still lacked the adjustment of the .noinit section's position within the binary by the time the PR went stale. Adjust the linker command file so that the .bss and .noinit sections are placed at the end of the resulting binary. Until now, those sections have been located somewhere in the middle of the binary, so that the inclusion of structures like statically defined heaps or large zero- initialized arrays reflected 1:1 in the resulting binary's size. Even for a stripped binary, such data was included in full as the linker couldn't omit it due to subsequent sections within the binary. This fix has been tested with a 32 MB statically allocated heap and a 32 MB uint8 zero-initialized array. Both structures are clearly identifyable in the memory consumption statistics, however, the final binary's size is unaffected by their inclusion. Signed-off-by: Immo Birnbaum <[email protected]>
This is a follow up to #53262, which still lacked the adjustment of the .noinit section's position within the binary by the time the PR went stale. Adjust the linker command file so that the .bss and .noinit sections are placed at the end of the resulting binary. Until now, those sections have been located somewhere in the middle of the binary, so that the inclusion of structures like statically defined heaps or large zero- initialized arrays reflected 1:1 in the resulting binary's size. Even for a stripped binary, such data was included in full as the linker couldn't omit it due to subsequent sections within the binary. This fix has been tested with a 32 MB statically allocated heap and a 32 MB uint8 zero-initialized array. Both structures are clearly identifyable in the memory consumption statistics, however, the final binary's size is unaffected by their inclusion. Signed-off-by: Immo Birnbaum <[email protected]>
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
7 participants
This is to avoid the binary padding caused by placing sections after the .bss noload gap, which causes the binary to be unnecessarily large. By placing the noload .bss at the end, we do not have to include it in the binary.
Signed-off-by: Théophile Ranquet [email protected]