Skip to content

app shared memory rules in CMakeLists.txt breaks incremental builds #13001

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.

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

Closed
andrewboie opened this issue Feb 2, 2019 · 4 comments · Fixed by #13182
Closed

app shared memory rules in CMakeLists.txt breaks incremental builds #13001

andrewboie opened this issue Feb 2, 2019 · 4 comments · Fixed by #13182
Assignees
Labels
area: Build System area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@andrewboie
Copy link
Contributor

andrewboie commented Feb 2, 2019

Describe the bug
If CONFIG_APP_SHARED_MEM is enabled, there is a build step to scan all the compiled .o files to generate a linker script containing application shared memory sections:

[ 77%] Built target kernel
Scanning dependencies of target app_smem_linker
Generating app_smem linker section
[ 77%] Built target app_smem_linker

Unfortuantely, this is messing with incremental builds. If I run 'make', and then run 'make' again, I would expect the build to finish immediately. Instead, every build step after app_smem_linker is repeated:

[  1%] Built target syscall_macros_h_target
[  5%] Built target driver_validation_h_target
[  5%] Built target kobj_types_h_target
[  5%] Built target syscall_list_h_target
[  6%] Built target offsets
[  7%] Built target offsets_h
[  8%] Built target app
[ 23%] Built target zephyr
[ 36%] Built target arch__arm__core
[ 41%] Built target lib__libc__minimal
[ 58%] Built target kernel
[ 66%] Built target arch__arm__core__cortex_m__mpu
[ 66%] Built target arch__arm__core__cortex_m
[ 67%] Built target boards__arm__mps2_an385
[ 69%] Built target subsys__app_memory
[ 71%] Built target drivers__gpio
[ 74%] Built target drivers__i2c
[ 77%] Built target tests__ztest
[ 77%] Built target drivers__serial
Generating app_smem linker section
[ 77%] Built target app_smem_linker
Scanning dependencies of target linker_priv_stacks_script
[ 78%] Generating linker_priv_stacks.cmd
[ 78%] Built target linker_priv_stacks_script
[ 80%] Linking C executable priv_stacks_prebuilt.elf
[ 81%] Built target priv_stacks_prebuilt
Scanning dependencies of target linker_script_target
[ 82%] Generating priv_stacks_hash.gperf
[ 82%] Generating linker.cmd
[ 82%] Built target linker_script_target
[ 82%] Built target priv_stacks
[ 83%] Generating priv_stacks_hash_preprocessed.c
[ 83%] Built target priv_stacks_output_src_pre
[ 84%] Generating priv_stacks_hash.c
Scanning dependencies of target priv_stacks_output_lib
[ 85%] Building C object zephyr/CMakeFiles/priv_stacks_output_lib.dir/priv_stacks_hash.c.obj
[ 85%] Linking C static library libpriv_stacks_output_lib.a
[ 86%] Built target priv_stacks_output_lib
[ 87%] Generating priv_stacks_hash_renamed.o
[ 87%] Built target priv_stacks_output_obj_renamed
[ 88%] Linking C executable zephyr_prebuilt.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:       55532 B         4 MB      1.32%
            SRAM:         27 KB         4 MB      0.66%
        IDT_LIST:         200 B         2 KB      9.77%
[ 89%] Built target zephyr_prebuilt
Scanning dependencies of target linker_pass_final_script_target
[ 90%] Generating kobject_hash.gperf
[ 91%] Generating linker_pass_final.cmd
[ 91%] Built target linker_pass_final_script_target
[ 91%] Built target obj_list
[ 92%] Generating kobject_hash_preprocessed.c
[ 92%] Built target output_src_pre
[ 93%] Generating kobject_hash.c
Scanning dependencies of target output_lib
[ 94%] Building C object zephyr/CMakeFiles/output_lib.dir/kobject_hash.c.obj
[ 94%] Linking C static library liboutput_lib.a
[ 95%] Built target output_lib
[ 96%] Generating kobject_hash_renamed.o
[ 96%] Built target output_obj_renamed
[ 97%] Generating isr_tables.c
Scanning dependencies of target kernel_elf
[ 97%] Building C object zephyr/CMakeFiles/kernel_elf.dir/isr_tables.c.obj
[ 98%] Linking C executable zephyr.elf
Generating files from zephyr.elf for board: mps2_an385
[100%] Built target kernel_elf

I would expect the output of a second make to more resemble:

[  1%] Built target syscall_macros_h_target
[  4%] Built target kobj_types_h_target
[  5%] Built target syscall_list_h_target
[  6%] Built target driver_validation_h_target
[  8%] Built target offsets
[  9%] Built target offsets_h
[ 10%] Built target app
[ 12%] Built target linker_script_target
[ 18%] Built target arch__arm__core__cortex_m
[ 22%] Built target arch__arm__core__cortex_m__mpu
[ 39%] Built target zephyr
[ 53%] Built target arch__arm__core
[ 73%] Built target kernel
[ 75%] Built target boards__arm__mps2_an385
[ 78%] Built target drivers__gpio
[ 82%] Built target drivers__serial
[ 82%] Built target drivers__i2c
[ 92%] Built target lib__libc__minimal
[ 94%] Built target zephyr_prebuilt
[ 95%] Built target linker_pass_final_script_target
[100%] Built target kernel_elf

The script works by scanning all the .obj files in the build output directory looking for section names. The script is being unconditionally run:

if(CONFIG_APP_SHARED_MEM AND CONFIG_USERSPACE)
  set(APP_SMEM_LD "${PROJECT_BINARY_DIR}/include/generated/app_smem.ld")
  set(OBJ_FILE_DIR "${PROJECT_BINARY_DIR}/../")

  add_custom_target(
    ${APP_SMEM_DEP} ALL
    DEPENDS app
    kernel ${ZEPHYR_LIBS_PROPERTY}
    )

  if(CONFIG_NEWLIB_LIBC)
    set(NEWLIB_PART -l libc.a z_newlib_partition)
  endif()
  add_custom_command(
    TARGET ${APP_SMEM_DEP}
    COMMAND ${PYTHON_EXECUTABLE}
    ${ZEPHYR_BASE}/scripts/gen_app_partitions.py
    -d ${OBJ_FILE_DIR}
    -o ${APP_SMEM_LD}
    ${NEWLIB_PART}
    $<$<BOOL:${CMAKE_VERBOSE_MAKEFILE}>:--verbose>
    WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/
    COMMENT "Generating app_smem linker section"
    )
endif()

It should instead have as a dependency all the object files in the output dir, and only run if any are added/changed/removed.

To Reproduce
Steps to reproduce the behavior:

Build tests/kernel/mem_protect/userspace twice on mps2_an385 or qemu_x86

Or pull #12990 and build any test twice.

@andrewboie andrewboie added bug The issue is a bug, or the PR is fixing a bug area: Build System area: Memory Protection labels Feb 2, 2019
@andrewboie andrewboie changed the title app shared memory script breaks incremental builds app shared memory rules in CMakeLists.txt breaks incremental builds Feb 2, 2019
@SebastianBoe
Copy link
Collaborator

Build tests/kernel/mem_protect/userspace twice on mps2_an385 or qemu_x86

NB: Reproduced for mps2_an385, but not qemu_x86.

@andrewboie
Copy link
Contributor Author

NB: Reproduced for mps2_an385, but not qemu_x86.

You're right, in master branch qemu_x86 doesn't use the region script. Forthcoming changes #12990 it will.

@SebastianBoe
Copy link
Collaborator

I think I have a fix, I'd prefer to apply it to #12990 instead of master, is that OK @andrewboie ?

@SebastianBoe
Copy link
Collaborator

Nevermind ... 12990 is now merged ...

SebastianBoe added a commit to SebastianBoe/zephyr that referenced this issue Feb 8, 2019
This commit applies various refactorings and whitespace fixes in
preparation for the patch that fixes zephyrproject-rtos#13001.

It includes changing indentation from tabs to spaces and splitting
tokens across more lines to increase readability and minimize patch
sizes.

Signed-off-by: Sebastian Bøe <[email protected]>
SebastianBoe added a commit to SebastianBoe/zephyr that referenced this issue Feb 8, 2019
The dependencies relating to generating LD scripts are declared
incorrectly. This manifests itself as broken incremental builds as
shown in zephyrproject-rtos#13001.

This commit aligns the LD script generation with the Zephyr standard
for declaring custom commands.

For instance, custom commands should generate files that are wrapped
by targets. The custom commands should declare the dependencies, not
the targets.

Also, when using custom commands, the OUTPUT signature should be used,
not the TARGET signature, as app_smem was doing.

For details about how Zephyr uses custom commands see
https://samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands/

This fixes zephyrproject-rtos#13001.

Signed-off-by: Sebastian Bøe <[email protected]>
@nashif nashif added the priority: medium Medium impact/importance bug label Feb 8, 2019
galak pushed a commit that referenced this issue Feb 8, 2019
This commit applies various refactorings and whitespace fixes in
preparation for the patch that fixes #13001.

It includes changing indentation from tabs to spaces and splitting
tokens across more lines to increase readability and minimize patch
sizes.

Signed-off-by: Sebastian Bøe <[email protected]>
galak pushed a commit that referenced this issue Feb 8, 2019
The dependencies relating to generating LD scripts are declared
incorrectly. This manifests itself as broken incremental builds as
shown in #13001.

This commit aligns the LD script generation with the Zephyr standard
for declaring custom commands.

For instance, custom commands should generate files that are wrapped
by targets. The custom commands should declare the dependencies, not
the targets.

Also, when using custom commands, the OUTPUT signature should be used,
not the TARGET signature, as app_smem was doing.

For details about how Zephyr uses custom commands see
https://samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands/

This fixes #13001.

Signed-off-by: Sebastian Bøe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants