search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.63k stars 6.5k forks source link

Cortex-M non-XIP binary includes bss/noinit sections #72307

Open gramsay0 opened 5 months ago

gramsay0 commented 5 months ago

This results in a larger binary than is actually necessary, which impacts bootloaders and firmware upgrade.

I tested with a Cortex-M board (nucleo_f756zg), although the same might occur on other architectures etc too.

To Reproduce

Off-topic...

This fails with a pretty horrific error message:

/tmp/ccseYthy.s: Assembler messages:
/tmp/ccseYthy.s:49: Error: missing expression
/tmp/ccseYthy.s:55: Error: missing expression

😕

I stumbled on this discussion https://github.com/zephyrproject-rtos/zephyr/discussions/42459 which IMO incorrectly states this as "not a bug"...

Some boards/SoCs do this:

if !XIP
config FLASH_SIZE
    default 0
config FLASH_BASE_ADDRESS
    default 0
endif

which probably should just be the default in zephyr/arch/Kconfig if !XIP

Back on topic...

Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align EXIDX 0x00341c 0x20013348 0x20013348 0x00008 0x00008 R 0x4 LOAD 0x0000d4 0x20010000 0x20010000 0x03cd0 0x03ce0 RWE 0x4 LOAD 0x003de0 0x20013ce0 0x20013ce0 0x000a2 0x01220 RW 0x40 LOAD 0x003e82 0x20014f00 0x20014f00 0x00004 0x00004 R 0x1 TLS 0x003938 0x20013864 0x20013864 0x00000 0x00004 R 0x4

Section to Segment mapping: Segment Sections... 00 .ARM.exidx 01 rom_start text .ARM.exidx initlevel device_area sw_isr_table rodata .ramfunc 02 datas device_states bss noinit 03 .last_section 04 tbss 


Note that `.last_section` is after `bss noinit`.

After digging through the linker script I found `CONFIG_LINKER_LAST_SECTION_ID` which can be disabled to remove this section.

* append `CONFIG_LINKER_LAST_SECTION_ID=n` to "samples/hello_world/prj.conf"
* build `west build -b nucleo_f756zg zephyr/samples/hello_world`
* check the binary size: `ls -l build/zephyr/zephyr.bin` -> 15746

Now the binary is 4482 bytes smaller, as bss/noinit are correctly excluded from the binary.

---

Now try another sample for iterable sections.

* append the previous learnings to "tests/misc/iterable_sections/prj.conf"

CONFIG_XIP=n CONFIG_FLASH_SIZE=0 CONFIG_FLASH_BASE_ADDRESS=0 CONFIG_LINKER_LAST_SECTION_ID=n

* build `west build -b nucleo_f756zg zephyr/tests/misc/iterable_sections`
* check the binary size: `ls -l build/zephyr/zephyr.bin` -> 36540
* dump the elf segments:

/opt/zephyr-sdk-0.16.4/arm-zephyr-eabi/bin/arm-zephyr-eabi-readelf build/zephyr/zephyr.elf --segments

Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align EXIDX 0x005394 0x200152c0 0x200152c0 0x00008 0x00008 R 0x4 LOAD 0x0000d4 0x20010000 0x20010000 0x076b0 0x076c0 RWE 0x4 LOAD 0x0077c0 0x200176c0 0x200176c0 0x000e0 0x017c0 RW 0x40 LOAD 0x0078a0 0x20018e80 0x20018e80 0x0003c 0x0003c R 0x4 TLS 0x005914 0x20015840 0x20015840 0x00000 0x00004 R 0x4

Section to Segment mapping: Segment Sections... 00 .ARM.exidx 01 rom_start text .ARM.exidx initlevel device_area sw_isr_table ztest log_const_area rodata .ramfunc 02 datas device_states test_ram_area test_ram2_area test_ram_named_area test_ram_numeric_area ramn_alt_area bss noinit 03 test_rom_area test_rom2_area test_rom_named_area test_rom_numeric_area romn_alt_area 04 tbss



Note that all the `test_rom_*` sections are after after `bss noinit`.

The reason is that `zephyr_linker_sources(SECTIONS ...)` (as recommended in the iterable sections documentation) is after bss/noinit. IMO there should be a `RODATA_SECTIONS` for things like this.

* for testing purposes just move `#include <snippets-sections.ld>` up in the linker file ("include/zephyr/arch/arm/cortex_m/scripts/linker.ld") to above bss/noinit
* build `west build -b nucleo_f756zg zephyr/tests/misc/iterable_sections`
* check the binary size: `ls -l build/zephyr/zephyr.bin` -> 30684

Now the binary is 5856 bytes smaller, as bss/noinit are correctly excluded from the binary.

**Expected behavior**
* Setting `CONFIG_XIP=n` builds without error
* bss/noinit are not included in the binary by default
* bss/noinit are not included in the binary when using iterable sections

**Impact**
Minor

**Environment**
 - OS: Linux
 - Toolchain: Zephyr SDK 0.16.4
 - Commit SHA: 55c8a1650235cbf26145160b149a7f6bce2954eb (current main)

---

BTW I'm happy to help with fixes etc if people agree that this should be tidied up
erwango commented 5 months ago

[...] which probably should just be the default in zephyr/arch/Kconfig if !XIP

Agree. This would help cleaning a bit the various non XIP configurations that can be found in tree and would help to get non-XIP just working.

pdgendt commented 5 months ago

Can https://github.com/zephyrproject-rtos/zephyr/pull/71911 be useful here?

gramsay0 commented 5 months ago

Can #71911 be useful here?

@pdgendt yes, that would help solve the iterable sections part of this issue.

Although, I think the in-tree uses and documentation should also be updated:

using the appropriate section identifier, DATA_SECTIONS for RAM structures and ~SECTIONS~ ROM_SECTIONS for ROM ones.

nashif commented 4 months ago

any update?

gramsay0 commented 3 months ago

I opened a couple of PRs to propose fixes

With respect to the confusing error message e.g:

west build -b nucleo_f756zg zephyr/samples/hello_world -DCONFIG_XIP=n
...
/tmp/cc2CISlB.s: Assembler messages:
/tmp/cc2CISlB.s:49: Error: missing expression
/tmp/cc2CISlB.s:55: Error: missing expression

I noticed that if you run cmake again you get a more sensible error message:

west build -b nucleo_f756zg zephyr/samples/hello_world -DCONFIG_XIP=n --cmake
...
warning: the value '' is invalid for FLASH_SIZE (defined at ...), which has type int -- assignment ignored
warning: the value '' is invalid for FLASH_BASE_ADDRESS (defined at ...), which has type hex -- assignment ignored
error: Aborting due to Kconfig warnings

I asked about this on discord but didn't get a reply https://discord.com/channels/720317445772017664/906521547672522752/1248019172798107648

github-actions[bot] commented 1 month ago

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.