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.

Sign up for GitHub

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

Jump to bottom

nucleo_f091rc: Linking issue since "align __data_ram/rom_start/end linker" (65a2de84a9d5c535167951bf1cf610c4f7967ea5) #38591

Closed
erwango opened this issue Sep 16, 2021 · 5 comments · Fixed by #38817
Closed

nucleo_f091rc: Linking issue since "align __data_ram/rom_start/end linker" (65a2de84a9d5c535167951bf1cf610c4f7967ea5) #38591

erwango opened this issue Sep 16, 2021 · 5 comments · Fixed by #38817
Assignees
@tejlmand
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore

Comments

@erwango
Copy link
Member

erwango commented Sep 16, 2021 •
edited
Loading

Describe the bug
Since merge of 65a2de8,
Some tests are not booting on nucleo_f091rc platform.

Reverting this commit gets the board booting again

Similar issue seen on : nucleo_l073rz

To Reproduce
Steps to reproduce the behavior:

  1. west build -b nucleo_f091rc tests/kernel/lifo/lifo_usage
  2. west flash -d build/nucleo_f091rc/
@erwango erwango added bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug labels Sep 16, 2021
@erwango
Copy link
Member Author

erwango commented Sep 16, 2021

@tejlmand Would you mind having a look?
(I can help in testing if needed)

@carlescufi carlescufi added the Regression Something, which was working, does not anymore label Sep 16, 2021
@erwango
Copy link
Member Author

erwango commented Sep 22, 2021 •
edited
Loading

@tejlmand I've made some investigation on this point.

In 65a2de8, you mention:

The following new symbols are introduced so that the data section is
aligned with other sections:

  • __data_end
  • __data_start
    value identical to __data_region_start but describes start of
    the section.

I can see this is the case on a target not impacted by the change (nucleo_f103rb).
However, on nucleo_f091rc, this is not true:

 *(SORT_BY_ALIGNMENT(.ramfunc.*))
                0x00000000200000bc                . = ALIGN (_region_min_align)
                0x00000000200000bc                _ramfunc_ram_end = .
                0x0000000000000000                _ramfunc_ram_size = (_ramfunc_ram_end - _ramfunc_ram_start)
                0x0000000008006ca4                _ramfunc_rom_start = LOADADDR (.ramfunc)
                0x00000000200000bc                __data_region_start = .

datas           0x00000000200000c0       0xb9 load address 0x0000000008006ca8
                0x00000000200000c0                __data_start = .
 *(SORT_BY_ALIGNMENT(.data))
 *(SORT_BY_ALIGNMENT(.data.*))
 .data.state.0  0x00000000200000c0        0x8 zephyr/libzephyr.a(heap-validate.c.obj)

If I'm reading thing correctly:

__data_region_start = 0x00000000200000bc
__data_start = 0x00000000200000c0

So the base assumption doesn't work.
No explanation on this fact so far, but this could be linked to the specific sram vector linker for this target: sram_vector_table.ld.

I'm continuing the analysis, but don't hesitate to get in touch if this rings a bell to you.

Edit: Impact of the specific sram vector:

                0x0000000020000000                _image_ram_start = .

.st_stm32f0x_vt
                0x0000000020000000       0xbc
                0x0000000020000000                _ram_vector_start = .
                0x00000000200000bc                . = (. + (_vector_end - _vector_start))
 *fill*         0x0000000020000000       0xbc 
                0x00000000200000bc                _ram_vector_end = .

.ramfunc        0x00000000200000bc        0x0 load address 0x0000000008006ca4
                0x00000000200000bc                . = ALIGN (_region_min_align)
                0x00000000200000bc                _ramfunc_ram_start = .

@erwango
Copy link
Member Author

erwango commented Sep 22, 2021

@JordanYates It seems you introduced sram_vector_table.ld in #34362.
It seems that disabling CONFIG_SRAM_VECTOR_TABLE on nucleo_f091rc solves current point.

First question, since you disabled CONFIG_SRAM_VECTOR_TABLE on a number of F0 boards, was there any reason to keep it on nucleo_f091rc ?
Then, if disabling CONFIG_SRAM_VECTOR_TABLE get things working, it does not solve the point that this option doesn't play well with 65a2de8. Would you mind having a quick look on the issue I pointed above ?

@JordanYates
Copy link
Collaborator

First question, since you disabled CONFIG_SRAM_VECTOR_TABLE on a number of F0 boards, was there any reason to keep it on nucleo_f091rc ?

The PR was cleaning up usage of CONFIG_IS_BOOTLOADER in order to get secondary effects (no SRAM vector table).
nucleo_f091rc was not modified because it was not using CONFIG_IS_BOOTLOADER to modify the vector table in the first place.

Then, if disabling CONFIG_SRAM_VECTOR_TABLE get things working, it does not solve the point that this option doesn't play well with 65a2de8. Would you mind having a quick look on the issue I pointed above?

As I don't use STM, I wasn't willing to introduce any behavior changes, so section names and symbols were not changed in my PR. Perhaps the act of moving the section declaration caused the issue, but that seems unlikely, and its using the standard zephyr_linker_sources_ifdef function, so it seems unlikely that this should cause issues. If the problem is just the naming, my PR predates the linked commit by 4 months.

@erwango
Copy link
Member Author

erwango commented Sep 23, 2021

First question, since you disabled CONFIG_SRAM_VECTOR_TABLE on a number of F0 boards, was there any reason to keep it on nucleo_f091rc ?

The PR was cleaning up usage of CONFIG_IS_BOOTLOADER in order to get secondary effects (no SRAM vector table).
nucleo_f091rc was not modified because it was not using CONFIG_IS_BOOTLOADER to modify the vector table in the first place.

ok

Then, if disabling CONFIG_SRAM_VECTOR_TABLE get things working, it does not solve the point that this option doesn't play well with 65a2de8. Would you mind having a quick look on the issue I pointed above?

As I don't use STM, I wasn't willing to introduce any behavior changes, so section names and symbols were not changed in my PR. Perhaps the act of moving the section declaration caused the issue, but that seems unlikely, and its using the standard zephyr_linker_sources_ifdef function, so it seems unlikely that this should cause issues. If the problem is just the naming, my PR predates the linked commit by 4 months.

Sure, your PR was change was introduced earlier than 65a2de8.
I was hoping maybe you could have some clue on the way to make both work together.

tejlmand added a commit to tejlmand/zephyr that referenced this issue Sep 24, 2021
@tejlmand
linker: __data_region_start equal to __data_start
0c48c5c 
Fixes: zephyrproject-rtos#38591

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 mentioned this issue Sep 24, 2021
Merged
@FRASTM FRASTM mentioned this issue Sep 24, 2021
Merged
tejlmand added a commit to tejlmand/zephyr that referenced this issue Sep 24, 2021
@tejlmand
linker: __data_region_start equal to __data_start
70c4322 
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 added a commit to tejlmand/zephyr that referenced this issue Sep 24, 2021
@tejlmand
cmake: linker generator: group symbol from section
574295b 
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>
tejlmand added a commit to tejlmand/zephyr that referenced this issue Sep 24, 2021
@tejlmand
cmake: linker: arm: utilize zephyr_linker_group(... SYMBOL SECTION)
72783f0 
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>
cfriedt pushed a commit that referenced this issue Sep 24, 2021
@tejlmand @cfriedt
linker: __data_region_start equal to __data_start
15e834a 
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>
cfriedt pushed a commit that referenced this issue Sep 24, 2021
@tejlmand @cfriedt
cmake: linker generator: group symbol from section
57ab034 
The root cause of #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>
cfriedt pushed a commit that referenced this issue Sep 24, 2021
@tejlmand @cfriedt
cmake: linker: arm: utilize zephyr_linker_group(... SYMBOL SECTION)
734e528 
The root cause of #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>
Rushybrook pushed a commit to Rushybrook/zephyr that referenced this issue Oct 21, 2021
@tejlmand @Rushybrook
linker: __data_region_start equal to __data_start
3eb37d5 
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>
Rushybrook pushed a commit to Rushybrook/zephyr that referenced this issue Oct 21, 2021
@tejlmand @Rushybrook
cmake: linker generator: group symbol from section
b4c2486 
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>
Rushybrook pushed a commit to Rushybrook/zephyr that referenced this issue Oct 21, 2021
@tejlmand @Rushybrook
cmake: linker: arm: utilize zephyr_linker_group(... SYMBOL SECTION)
6b48a9e 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tejlmand tejlmand

Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug Regression Something, which was working, does not anymore
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

linker: __data_region_start equal to __data_start tejlmand/zephyr
4 participants
@JordanYates @tejlmand @carlescufi @erwango