From 3baaf3bae0325fabcea2af190f2439f2386d2a04 Mon Sep 17 00:00:00 2001 From: Grzegorz Swiderski Date: Fri, 12 Jan 2024 06:06:36 +0100 Subject: [PATCH 1/2] code_relocation: Add NOKEEP option 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. Signed-off-by: Grzegorz Swiderski --- cmake/modules/extensions.cmake | 20 ++++--- doc/kernel/code-relocation.rst | 16 ++++++ scripts/build/gen_relocate_app.py | 55 ++++++++++++------- .../code_relocation/CMakeLists.txt | 5 +- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/cmake/modules/extensions.cmake b/cmake/modules/extensions.cmake index 5a216fc1a7994..e2904066e7a26 100644 --- a/cmake/modules/extensions.cmake +++ b/cmake/modules/extensions.cmake @@ -1330,9 +1330,11 @@ endmacro() # The following optional arguments are supported: # - NOCOPY: this flag indicates that the file data does not need to be copied # at boot time (For example, for flash XIP). +# - NOKEEP: suppress the generation of KEEP() statements in the linker script, +# to allow any unused code in the given files/library to be discarded. # - PHDR [program_header]: add program header. Used on Xtensa platforms. function(zephyr_code_relocate) - set(options NOCOPY) + set(options NOCOPY NOKEEP) set(single_args LIBRARY LOCATION PHDR) set(multi_args FILES) cmake_parse_arguments(CODE_REL "${options}" "${single_args}" @@ -1392,21 +1394,25 @@ function(zephyr_code_relocate) endif() endif() if(NOT CODE_REL_NOCOPY) - set(copy_flag COPY) + set(flag_list COPY) else() - set(copy_flag NOCOPY) + set(flag_list NOCOPY) + endif() + if(CODE_REL_NOKEEP) + list(APPEND flag_list NOKEEP) endif() if(CODE_REL_PHDR) set(CODE_REL_LOCATION "${CODE_REL_LOCATION}\ :${CODE_REL_PHDR}") endif() - # We use the "|" character to separate code relocation directives instead - # of using CMake lists. This way, the ";" character can be reserved for - # generator expression file lists. + # We use the "|" character to separate code relocation directives, instead of + # using set_property(APPEND) to produce a ";"-separated CMake list. This way, + # each directive can embed multiple CMake lists, representing flags and files, + # the latter of which can come from generator expressions. get_property(code_rel_str TARGET code_data_relocation_target PROPERTY COMPILE_DEFINITIONS) set_property(TARGET code_data_relocation_target PROPERTY COMPILE_DEFINITIONS - "${code_rel_str}|${CODE_REL_LOCATION}:${copy_flag}:${file_list}") + "${code_rel_str}|${CODE_REL_LOCATION}:${flag_list}:${file_list}") endfunction() # Usage: diff --git a/doc/kernel/code-relocation.rst b/doc/kernel/code-relocation.rst index a9da18e98cd4a..599cdedccbd74 100644 --- a/doc/kernel/code-relocation.rst +++ b/doc/kernel/code-relocation.rst @@ -97,6 +97,22 @@ This section shows additional configuration options that can be set in zephyr_code_relocate(FILES ${sources} LOCATION SRAM) zephyr_code_relocate(FILES $ LOCATION SRAM) +NOKEEP flag +=========== + +By default, all relocated functions and variables will be marked with ``KEEP()`` +when generating ``linker_relocate.ld``. Therefore, if any input file happens to +contain unused symbols, then they will not be discarded by the linker, even when +it is invoked with ``--gc-sections``. If you'd like to override this behavior, +you can pass ``NOKEEP`` to your ``zephyr_code_relocate()`` call. + + .. code-block:: none + + zephyr_code_relocate(FILES src/file1.c LOCATION SRAM2_TEXT NOKEEP) + +The example above will help ensure that any unused code found in the .text +sections of ``file1.c`` will not stick to SRAM2. + NOCOPY flag =========== diff --git a/scripts/build/gen_relocate_app.py b/scripts/build/gen_relocate_app.py index 1d7783cd38a95..f1bcccf1df699 100644 --- a/scripts/build/gen_relocate_app.py +++ b/scripts/build/gen_relocate_app.py @@ -33,6 +33,8 @@ ignored. - COPY/NOCOPY defines whether the script should generate the relocation code in code_relocation.c or not +- NOKEEP will suppress the default behavior of marking every relocated symbol + with KEEP() in the generated linker script. Multiple regions can be appended together like SRAM2_DATA_BSS this will place data and bss inside SRAM2. @@ -94,12 +96,17 @@ def for_section_named(cls, name: str): class OutputSection(NamedTuple): obj_file_name: str section_name: str + keep: bool = True PRINT_TEMPLATE = """ KEEP(*{obj_file_name}({section_name})) """ +PRINT_TEMPLATE_NOKEEP = """ + *{obj_file_name}({section_name}) +""" + SECTION_LOAD_MEMORY_SEQ = """ __{0}_{1}_rom_start = LOADADDR(.{0}_{1}_reloc); """ @@ -274,10 +281,16 @@ def assign_to_correct_mem_region( if align_size: mpu_align[memory_region] = int(align_size) + keep_sections = '|NOKEEP' not in memory_region + memory_region = memory_region.replace('|NOKEEP', '') + output_sections = {} for used_kind in use_section_kinds: # Pass through section kinds that go into this memory region - output_sections[used_kind] = full_list_of_sections[used_kind] + output_sections[used_kind] = [ + section._replace(keep=keep_sections) + for section in full_list_of_sections[used_kind] + ] return {MemoryRegion(memory_region): output_sections} @@ -308,10 +321,12 @@ def section_kinds_from_memory_region(memory_region: str) -> 'Tuple[set[SectionKi def print_linker_sections(list_sections: 'list[OutputSection]'): - return ''.join(PRINT_TEMPLATE.format(obj_file_name=section.obj_file_name, - section_name=section.section_name) - for section in sorted(list_sections)) - + out = '' + for section in sorted(list_sections): + template = PRINT_TEMPLATE if section.keep else PRINT_TEMPLATE_NOKEEP + out += template.format(obj_file_name=section.obj_file_name, + section_name=section.section_name) + return out def add_phdr(memory_type, phdrs): return f'{memory_type} {phdrs[memory_type] if memory_type in phdrs else ""}' @@ -485,20 +500,23 @@ def get_obj_filename(searchpath, filename): return fullname -# Extracts all possible components for the input strin: -# [\ :program_header]:: -# Returns a 4-tuple with them: (mem_region, program_header, flag, file_name) +# Extracts all possible components for the input string: +# [\ :program_header]:[;...]:[;...] +# Returns a 4-tuple with them: (mem_region, program_header, flags, files) # If no `program_header` is defined, returns an empty string def parse_input_string(line): - line = line.replace(' :', ':') + # Be careful when splitting by : to avoid breaking absolute paths on Windows + mem_region, rest = line.split(':', 1) - flag_sep = ':NOCOPY:' if ':NOCOPY' in line else ':COPY:' - mem_region_phdr, copy_flag, file_name = line.partition(flag_sep) - copy_flag = copy_flag.replace(':', '') + phdr = '' + if mem_region.endswith(' '): + mem_region = mem_region.rstrip() + phdr, rest = rest.split(':', 1) - mem_region, _, phdr = mem_region_phdr.partition(':') + # Split lists by semicolons, in part to support generator expressions + flag_list, file_list = (lst.split(';') for lst in rest.split(':', 1)) - return mem_region, phdr, copy_flag, file_name + return mem_region, phdr, flag_list, file_list # Create a dict with key as memory type and files as a list of values. @@ -515,17 +533,15 @@ def create_dict_wrt_mem(): if ':' not in line: continue - mem_region, phdr, copy_flag, file_list = parse_input_string(line) + mem_region, phdr, flag_list, file_list = parse_input_string(line) # Handle any program header if phdr != '': phdrs[mem_region] = f':{phdr}' - # Split file names by semicolons, to support generator expressions - file_glob_list = file_list.split(';') file_name_list = [] # Use glob matching on each file in the list - for file_glob in file_glob_list: + for file_glob in file_list: glob_results = glob.glob(file_glob) if not glob_results: warnings.warn("File: "+file_glob+" Not found") @@ -534,14 +550,13 @@ def create_dict_wrt_mem(): warnings.warn("Regex in file lists is deprecated, please use file(GLOB) instead") file_name_list.extend(glob_results) if len(file_name_list) == 0: - warnings.warn("No files in string: "+file_list+" found") continue if mem_region == '': continue if args.verbose: print("Memory region ", mem_region, " Selected for files:", file_name_list) - mem_region = "|".join((mem_region, copy_flag)) + mem_region = "|".join((mem_region, *flag_list)) if mem_region in rel_dict: rel_dict[mem_region].extend(file_name_list) diff --git a/tests/application_development/code_relocation/CMakeLists.txt b/tests/application_development/code_relocation/CMakeLists.txt index 132c905895ed0..60350a6e2a51e 100644 --- a/tests/application_development/code_relocation/CMakeLists.txt +++ b/tests/application_development/code_relocation/CMakeLists.txt @@ -37,8 +37,9 @@ zephyr_code_relocate(FILES src/test_file3.c LOCATION SRAM2_TEXT) zephyr_code_relocate(FILES src/test_file3.c LOCATION RAM_DATA) zephyr_code_relocate(FILES src/test_file3.c LOCATION SRAM2_BSS) - -zephyr_code_relocate(FILES ${ZEPHYR_BASE}/kernel/sem.c ${RAM_PHDR} LOCATION RAM) +# Test NOKEEP support. Placing both KEEP and NOKEEP symbols in the same location +# (this and test_file2.c in RAM) should work fine. +zephyr_code_relocate(FILES ${ZEPHYR_BASE}/kernel/sem.c ${RAM_PHDR} LOCATION RAM NOKEEP) if (CONFIG_RELOCATE_TO_ITCM) zephyr_code_relocate(FILES ${ZEPHYR_BASE}/lib/libc/minimal/source/string/string.c From a2c76d7c46e06b5eac496dca97a94c4f1655d6e7 Mon Sep 17 00:00:00 2001 From: Grzegorz Swiderski Date: Sat, 13 Jan 2024 13:02:56 +0100 Subject: [PATCH 2/2] cmake: code_relocation: Re-run when relocation_dict.txt changes Fix an issue where updating a `zephyr_code_relocate()` call in CMake didn't trigger regeneration of `code_relocation.c` and linker scripts. The generation command was missing a dependency on the aforementioned input text file, where the outcome of each call is cached. Signed-off-by: Grzegorz Swiderski --- cmake/linker/ld/target_relocation.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/linker/ld/target_relocation.cmake b/cmake/linker/ld/target_relocation.cmake index c290a330484cd..a90941d04af9f 100644 --- a/cmake/linker/ld/target_relocation.cmake +++ b/cmake/linker/ld/target_relocation.cmake @@ -32,7 +32,7 @@ macro(toolchain_ld_relocation) -b ${MEM_RELOCATION_SRAM_BSS_LD} -c ${MEM_RELOCATION_CODE} --default_ram_region ${MEM_REGION_DEFAULT_RAM} - DEPENDS app kernel ${ZEPHYR_LIBS_PROPERTY} + DEPENDS app kernel ${ZEPHYR_LIBS_PROPERTY} ${DICT_FILE} ) add_library(code_relocation_source_lib STATIC ${MEM_RELOCATION_CODE})