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

Fix linker scripts to define _end correctly #57727

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

keith-packard
Copy link
Collaborator

_end must point past the last address of statically allocated RAM in the application. Adjust the linker scripts to use a new fragment which does this. Adjust the cmake linker script generator to do the same.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this. The proposed solution looks reasonable and should hopefully improve the consistency across different archs and SoCs.

include/zephyr/linker/common-ram-end.ld Outdated Show resolved Hide resolved
include/zephyr/linker/common-ram-end.ld Outdated Show resolved Hide resolved
@stephanosio stephanosio assigned tejlmand and unassigned fkokosinski May 10, 2023
@keith-packard keith-packard force-pushed the define-_end branch 3 times, most recently from 4dfefd1 to 3bd49e0 Compare May 10, 2023 06:19
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Can you please fix the compliance check failure regarding the first commit message?

1: UC3 Commit title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "Fix linker scripts to define _end after all static RAM data"

tejlmand
tejlmand previously approved these changes May 10, 2023
galak
galak previously requested changes May 10, 2023
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the new symbol differ from _kernel_ram_end?

@stephanosio
Copy link
Member

How does the new symbol differ from _kernel_ram_end?

What "new" symbol?

@keith-packard
Copy link
Collaborator Author

How does the new symbol differ from _kernel_ram_end?

Aside from the fact that _end is already defined in most of the linker scripts (just incorrectly), _kernel_ram_end points at the end of memory, not the end of memory that is statically assigned by the linker.

@keith-packard
Copy link
Collaborator Author

@stephanosio -- could this be considered for 3.4? It's a fatal error when linking with picolibc using the default common malloc configuration settings on smaller targets for most architectures.

jhedberg
jhedberg previously approved these changes Jun 2, 2023
andyross
andyross previously approved these changes Jun 2, 2023
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticing this. Looks great to me.

@stephanosio stephanosio added the bug The issue is a bug, or the PR is fixing a bug label Jun 3, 2023
@stephanosio stephanosio added this to the v3.4.0 milestone Jun 3, 2023
@stephanosio
Copy link
Member

@galak PTAL

@nashif nashif dismissed galak’s stale review June 16, 2023 12:49

please revisit and address questions/comments after your review.

@nashif nashif added the backport v3.4-branch Backport to the v3.4-branch label Jun 17, 2023
@fabiobaltieri
Copy link
Member

@keith-packard can you rebase please?

…M data

The Zephyr linker scripts have inconsistent ordering of various chunks of
data which lands in RAM at runtime. This leads to the value of _end not
being consistently defined as the maximum address of static variables used
in the application.

Create a helper linker fragment, zephyr/linker/ram-end.ld, which can be
included after the last possible definition of RAM data, that consistently
sets _image_ram_end, _end and z_mapped_end.

Signed-off-by: Keith Packard <[email protected]>
The generated scripts don't include a definition for any symbol indicating
the end of statically allocated memory (such as "_end"). Add a shared cmake
fragment, ram-end.cmake, which contains the necessary instructions to
define _end and z_mapped_end consistently to align with the other sample
linker scripts.

Signed-off-by: Keith Packard <[email protected]>
@fabiobaltieri fabiobaltieri merged commit bbec614 into zephyrproject-rtos:main Jun 28, 2023
duda-patryk added a commit to Semihalf/zephyr that referenced this pull request Jun 30, 2023
Currently, this section has LOAD flag which means that it's part of
image layout. Attempt to generate binary file with --gap-fill results in
creating a very big file, because LMA of the section is RAM address
(e.g. 0x20000000 for STM32F4) instead of FLASH (e.g. 0x8000000 for
STM32F4).

This problem doesn't appear when linking using GNU LD, because it
removes the section in garbage collect process. However, LLVM LLD
doesn't garbage collect sections that define used symbols, so the
section is present in final ELF image.

There is no need to load this section (it has 0 size), so we can safely
add NOLOAD attribute to the section.

Fixes: zephyrproject-rtos#57727

Signed-off-by: Patryk Duda <[email protected]>
fabiobaltieri pushed a commit that referenced this pull request Jul 5, 2023
Currently, this section has LOAD flag which means that it's part of
image layout. Attempt to generate binary file with --gap-fill results in
creating a very big file, because LMA of the section is RAM address
(e.g. 0x20000000 for STM32F4) instead of FLASH (e.g. 0x8000000 for
STM32F4).

This problem doesn't appear when linking using GNU LD, because it
removes the section in garbage collect process. However, LLVM LLD
doesn't garbage collect sections that define used symbols, so the
section is present in final ELF image.

There is no need to load this section (it has 0 size), so we can safely
add NOLOAD attribute to the section.

Fixes: #57727

Signed-off-by: Patryk Duda <[email protected]>
cfriedt pushed a commit to cfriedt/zephyr that referenced this pull request Jul 5, 2023
Currently, this section has LOAD flag which means that it's part of
image layout. Attempt to generate binary file with --gap-fill results in
creating a very big file, because LMA of the section is RAM address
(e.g. 0x20000000 for STM32F4) instead of FLASH (e.g. 0x8000000 for
STM32F4).

This problem doesn't appear when linking using GNU LD, because it
removes the section in garbage collect process. However, LLVM LLD
doesn't garbage collect sections that define used symbols, so the
section is present in final ELF image.

There is no need to load this section (it has 0 size), so we can safely
add NOLOAD attribute to the section.

Fixes: zephyrproject-rtos#57727

Signed-off-by: Patryk Duda <[email protected]>
cfriedt pushed a commit to cfriedt/zephyr that referenced this pull request Jul 6, 2023
Currently, this section has LOAD flag which means that it's part of
image layout. Attempt to generate binary file with --gap-fill results in
creating a very big file, because LMA of the section is RAM address
(e.g. 0x20000000 for STM32F4) instead of FLASH (e.g. 0x8000000 for
STM32F4).

This problem doesn't appear when linking using GNU LD, because it
removes the section in garbage collect process. However, LLVM LLD
doesn't garbage collect sections that define used symbols, so the
section is present in final ELF image.

There is no need to load this section (it has 0 size), so we can safely
add NOLOAD attribute to the section.

Fixes: zephyrproject-rtos#57727

Signed-off-by: Patryk Duda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport v3.4-branch Backport to the v3.4-branch bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.