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

linker: __data_region_start equal to __data_start #38817

Conversation

tejlmand
Copy link
Collaborator

@tejlmand tejlmand commented Sep 24, 2021

Fixes: #38591, #38207, #37861

The commit 65a2de8 aligned the data
linker symbol for sections and regions.

The data region symbol start has been placed outside the sections thus
being defined as the address of the region before alignment of the first
section in the data region, usually the datas section.

The symbol defining the start address of the data section is after
section alignment.
In most cases the address of the data region start and datas section
start will be identical, but not always.
The data region symbol is a new linker symbol and existing code has
been depending on the old data section start symbol.
Thus, the update to the use of the data region start symbol instead of
data ram start symbol thus results in a different address when the
section is aligned to a different address.

To ensure the original behavior in all cases, the data region start
address is now moved inside the data section.

Signed-off-by: Torsten Rasmussen Torsten.Rasmussen@nordicsemi.no


DNM to allow time to examine the ld linker script generator for possible related fixup.

@tejlmand tejlmand added bug The issue is a bug, or the PR is fixing a bug DNM This PR should not be merged (Do Not Merge) labels Sep 24, 2021
@tejlmand tejlmand added this to the v2.7.0 milestone Sep 24, 2021
@tejlmand tejlmand requested a review from a user September 24, 2021 09:55
@github-actions github-actions bot added area: API Changes to public APIs area: ARC ARC Architecture area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture area: RISCV RISCV Architecture (32-bit & 64-bit) labels Sep 24, 2021
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

I confirm it fixes #38591

@zephyrbot zephyrbot added the area: SPARC SPARC Architecture label Sep 24, 2021
@zephyrbot zephyrbot removed request for galak, dleach02 and a user September 24, 2021 12:29
@evgeniy-paltsev
Copy link
Collaborator

I was preparing similar PR, but you was faster :)

This also
fixes: #38207
fixes: #37861

Fixes: zephyrproject-rtos#38591, zephyrproject-rtos#38207, zephyrproject-rtos#37861

The commit 65a2de8 aligned the data
linker symbol for sections and regions.

The data region symbol start has been placed outside the sections thus
being defined as the address of the region before alignment of the first
section in the data region, usually the `datas` section.

The symbol defining the start address of the data section is after
section alignment.
In most cases the address of the data region start and datas section
start will be identical, but not always.
The data region symbol is a new linker symbol and existing code has
been depending on the old data section start symbol.
Thus, the update to the use of the data region start symbol instead of
data ram start symbol thus results in a different address when the
section is aligned to a different address.

To ensure the original behavior in all cases, the data region start
address is now moved inside the data section.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand tejlmand force-pushed the issues/38591_relocate_region_start_symbols branch from 0c48c5c to a2507ea Compare September 24, 2021 15:18
@tejlmand
Copy link
Collaborator Author

I was preparing similar PR, but you was faster :)

This also
fixes: #38207
fixes: #37861

Thanks, added to the commit message.

@tejlmand
Copy link
Collaborator Author

Two additional commits targeted the ld linker script generator has been added to ensure the data and text region start symbol addresses are defined identical to using the ld template files.

@tejlmand tejlmand removed the DNM This PR should not be merged (Do Not Merge) label Sep 24, 2021
The root cause of zephyrproject-rtos#38591 was region symbols being placed before the
section description for data region.

Some region start symbols are placed before section description, other
region start symbols are placed inside the first section in the region
and thus identical to the sections own first symbol.

To support both schemes with the linker generator, a new
`SYMBOL SECTION` argument has been added to the zephyr_linker_group()
function.

The ld_script.cmake linker script generator has been updated to support
the new argument so that generated ld linker script has identical
behavior to the templated ld linker scripts.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
The root cause of zephyrproject-rtos#38591 was region symbols being placed before the
section description for data region.

To support both schemes with the linker generator, a new
`SYMBOL SECTION` argument has been added to the zephyr_linker_group()
function.

This commit updates the arm/linker.cmake CMake linker file to use the
new `SYMBOL SECTION` argument for the data region group and text region
group so that those two groups now behave identical to the behavior when
using the cortex_m linker.ld template.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand tejlmand force-pushed the issues/38591_relocate_region_start_symbols branch from a2507ea to 72783f0 Compare September 24, 2021 15:25
@cfriedt cfriedt merged commit 734e528 into zephyrproject-rtos:main Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: ARC ARC Architecture area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture area: Build System area: RISCV RISCV Architecture (32-bit & 64-bit) area: SPARC SPARC Architecture bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
10 participants