code_relocation: Add NOKEEP option

[57300]

When using the code and data relocation feature, every relocated symbol would be marked with KEEP() in the generated linker script. Therefore, if any input files contained unused code, then it wouldn't be discarded by the linker, even when invoked with --gc-sections.

This can cause unexpected bloat, or other link-time issues stemming from some symbols being discarded and others not.

On the other hand, this behavior has been present since the feature's introduction, so it should remain default for the users who rely on it.

This patch introduces support for zephyr_code_relocate(... NOKEEP). This will suppress the generation of KEEP() statements for all symbols in a particular library or set of files.

Much like NOCOPY, the NOKEEP flag is passed to gen_relocate_app.py in string form. The script is now equipped to handle multiple such flags when passed from CMake as a semicolon-separated list, like so:

"SRAM2:NOCOPY;NOKEEP:/path/to/file1.c;/path/to/file2.c"

Documentation and tests are updated here as well.

[nordicjm]

Seeing very funny behaviour with this, starting with samples/application_development/code_relocation_nocopy and building without NOKEEP flags, then editing the ext_code.c file to have a new function and building, then editing the sram_code.c file to have a new function and building, it seems the second build gives the same as the first build which is wrong, then the third build suddenly increases in extflash size, flash size, ram size. Adding and removing NOKEEP to each file and building again doesn't always seem to change things, in fact you can get into a state whereby changing all 3 instances from without NOKEEP to having NOKEEP yields the exact same thing, which I would assume seems to be from a state cache?

[tejlmand]

Initial looks seems ok, but the observation made by @nordicjm should be investigated.

Only major notice is that it appears there originally was a specific reason for not using ; for anything else than the CMake list of files:
https://github.com/zephyrproject-rtos/zephyr/blob/900f06f54a26722f2d0ce6271b4618d47f852b91/cmake/modules/extensions.cmake#L1407-L1409

Therefore the following must be done:

• Ensure that introducing ; as flag list separator doesn't introduce unintended side-effects / breaks existing use-cases.
  Currently I don't see why this PR should break existing use-cases (even use-cases using genex), but I haven't looked at all existing code parts.
  Maybe a closer look at 10e85bf can provide extra details.
• Update referenced comment, as ; is now used in more than the file list.

[tejlmand]

just a thought, as I do notice this is aligned with the file list processing here: https://github.com/zephyrproject-rtos/zephyr/blob/900f06f54a26722f2d0ce6271b4618d47f852b91/scripts/build/gen_relocate_app.py#L544

but as we do have the function parse_input_string() wouldn't make more sense that such function does all conversion, that is, splitting content separated by ; into Python list (or array).

That way there is only one function handling input strings to internal representation.

Note, this comment is not blocking for approval.

[57300]

Good idea. Done.

[57300]

Thanks for your observations.

Seeing very funny behaviour with this, starting with samples/application_development/code_relocation_nocopy and building without NOKEEP flags, then editing the ext_code.c file to have a new function and building, then editing the sram_code.c file to have a new function and building, it seems the second build gives the same as the first build which is wrong, then the third build suddenly increases in extflash size, flash size, ram size.

@nordicjm I tried something similar, first on qemu_cortex_m3:

1. Added an empty void dummy1(void) to ext_code.c, rebuilt, and saw EXTFLASH increase by 4 bytes.
2. Added an empty void dummy2(void) to sram_code.c, rebuilt, and saw RAM increase by 8 bytes.

When I repeated this on nrf5340dk_nrf5340_cpuapp, I saw no change in RAM size, but dummy2 was still present in the ELF. That could be a matter of alignment or optimization.

So, I'm convinced that this is working OK. The relocation script should be re-run on every source code change, because it's set up to depend on "all" libraries:

zephyr/cmake/linker/ld/target_relocation.cmake

Line 35 in 5fc96ff

DEPENDS app kernel ${ZEPHYR_LIBS_PROPERTY}

Adding and removing NOKEEP to each file and building again doesn't always seem to change things, in fact you can get into a state whereby changing all 3 instances from without NOKEEP to having NOKEEP yields the exact same thing, which I would assume seems to be from a state cache?

That turns out to be a pre-existing issue, but I don't mind fixing it here. Check out the second commit.

[57300]

Only major notice is that it appears there originally was a specific reason for not using ; for anything else than the CMake list of files:

Thanks @tejlmand for pointing this out. I have updated the comment.

I believe this comment exists simply because the strings that are now separated by | used to be separated by ;.
PR #50791 added support for calling zephyr_code_relocate() with a list of FILES, or a genex producing one.
Previously, the function accepted only one file or glob at a time, so ; didn't need to be reserved for any lists.

So, that PR allowed for having one CMake list per string. With this PR, there are two lists per string, and they're separated by a colon. If anything can break from that, it would probably hinge on parse_input_string() in the Python script.

[tejlmand]

yes, my initial conclusion was also that your changes should not have unintended side-effects, but I didn't examined the existing code in details, as we know, the devil is in the details 😉 .

Thanks for carrying out more in-depth analysis and updating the comment.

[nordicjm]

Fix works, thanks.

[tejlmand]

lgtm