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

Arch arm cortex m text section fix #21874

Merged
merged 4 commits into from
Jan 23, 2020
Merged

Arch arm cortex m text section fix #21874

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b5767c7
arch: arm: cortex-m: linker.ld: do not indent preprocessor directives
ioannisg Jan 13, 2020
6107453
arch: arm: cortex-m: properly name ROM sections in the linker
ioannisg Jan 13, 2020
5875f6c
arch: arm: cortex-m: move openocd sections after _vector_end
ioannisg Jan 17, 2020
3eb1516
extensions.cmake: Replace TEXT_START with ROM_START
oyvindronningstad Jan 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/arch/arm/aarch32/cortex_m/scripts/linker.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ SECTIONS

_image_rom_start = ROM_ADDR;

SECTION_PROLOGUE(_TEXT_SECTION_NAME,,)
SECTION_PROLOGUE(rom_start,,)
{

/* Located in generated directory. This file is populated by the
Expand All @@ -151,7 +151,7 @@ SECTIONS

#endif /* CONFIG_CODE_DATA_RELOCATION */

SECTION_PROLOGUE(_TEXT_SECTION_NAME_2,,)
SECTION_PROLOGUE(_TEXT_SECTION_NAME,,)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we do the same fix for cortex-r ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do that on separate PR, hopefully by the cortex r contributors

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that _TEXT_SECTION_NAME_2 is used in scripts/sanitycheck for doing some size calculation. Should the script be updated to reflect these changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we left it there because we don't fix the cortex-r now. Will make sure I ask @stephanosio for the cortex r fix ::)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this break the size calculation? What consequences does this have?

Copy link
Member Author

Choose a reason for hiding this comment

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

@SebastianBoe I am not sure where "this" exactly refers to; i'll try to reply, anyways.

The current patch only renames the "TEXT_SECTION_NAME_2" section to "text", and the original "text" section to rom_start; note that that section contains the vector table.

The patch does not move any symbols between sections.

Where does size calculation take place?

Copy link
Collaborator

Choose a reason for hiding this comment

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

By this I meant this patch. Will see if I can find the relevant sanitycheck code.

Copy link
Collaborator

@SebastianBoe SebastianBoe Jan 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It appears that sanitycheck believes that _TEXT_SECTION_NAME_2 is in the rw area.

Which I assume is wrong, so this patch should not cause any regression, it might even fix
the calculation.

https://github.com/zephyrproject-rtos/zephyr/blob/master/scripts/sanitycheck#L1074

{
_image_text_start = .;

Expand Down