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

scripts: check_init_priorities: use the zephyr executable file [WAS: scripts: add dump_init_order.py] #62459

Merged
merged 3 commits into from
Sep 20, 2023
Merged

scripts: check_init_priorities: use the zephyr executable file [WAS: scripts: add dump_init_order.py] #62459

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
954f63b
scripts: check_init_priorities: use the Zephyr executable file
fabiobaltieri Sep 8, 2023
8be0bf1
cmake: add a new "initlevels" target to print the init sequence
fabiobaltieri Sep 11, 2023
4706d0e
cmake: skip build time priority checking with armclang
fabiobaltieri Sep 18, 2023
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
14 changes: 12 additions & 2 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1811,12 +1811,22 @@ if(CONFIG_CHECK_INIT_PRIORITIES)
list(APPEND
post_build_commands
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/build/check_init_priorities.py
--build-dir ${PROJECT_BINARY_DIR}/..
--edt-pickle ${EDT_PICKLE}
--elf-file=${ZEPHYR_BINARY_DIR}/${KERNEL_ELF_NAME}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Failing with armclang with following error:

Traceback (most recent call last):
  File "/projects/github/ncs/zephyr/scripts/build/check_init_priorities.py", line 394, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/projects/github/ncs/zephyr/scripts/build/check_init_priorities.py", line 376, in main
    validator = Validator(args.elf_file, args.edt_pickle, log)
  File "/projects/github/ncs/zephyr/scripts/build/check_init_priorities.py", line 253, in __init__
    self._obj = ZephyrInitLevels(elf_file_path)
  File "/projects/github/ncs/zephyr/scripts/build/check_init_priorities.py", line 110, in __init__
    self._load_level_addr()
  File "/projects/github/ncs/zephyr/scripts/build/check_init_priorities.py", line 145, in _load_level_addr
    raise ValueError(f"Missing init symbols, found: {self._init_level_addr}")
ValueError: Missing init symbols, found: {}
ninja: build stopped: subcommand failed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bummer. Would you be able to send an elf file over to me?

Copy link
Member

Choose a reason for hiding this comment

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

I'd honestly not block this PR because it doesn't work with armclang, but exclude the target if the toolchain being used is armclang. It can be investigated later by users with access to it. Restricting progress on OSS projects because of licensed software is not a good idea.

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 I was about to voice the same opinion over into #60031, we can't really depend on "ping someone who can carry out such test". Either the tool is generally available and we can do a CI integration or regressions will have to be fixed out of band.

That said, for this one since se managed to catch the regression I'm happy to look into it if you can give me something to work on.

Copy link
Member

Choose a reason for hiding this comment

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

does it work with public clang?

Copy link
Member Author

@fabiobaltieri fabiobaltieri Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

$ export ZEPHYR_TOOLCHAIN_VARIANT=llvm
$ west build -p -b native_posix_64 samples/subsys/display/lvgl
...
-- Found toolchain: host (clang/ld)
...
-- The C compiler identification is Clang 14.0.6
-- The CXX compiler identification is Clang 14.0.6
-- The ASM compiler identification is Clang with GNU-like command-line
...
[329/329] Linking C executable zephyr/zephyr.elf
$ ./scripts/build/check_init_priorities.py -v
INFO: check_init_priorities: build/zephyr/zephyr.elf
INFO: /gpio@800/sdl_gpio POST_KERNEL 3 > /gpio@800 POST_KERNEL 2
INFO: /keys POST_KERNEL 5 > /gpio@800 POST_KERNEL 2
INFO: /keys POST_KERNEL 5 > /gpio@800 POST_KERNEL 2
INFO: /lvgl_button_input APPLICATION 2 > /keys POST_KERNEL 5
INFO: /lvgl_pointer APPLICATION 3 > /input-sdl-touch POST_KERNEL 6
$ ./scripts/build/check_init_priorities.py -i
EARLY
PRE_KERNEL_1
  __init_statics_init_pre: statics_init(NULL)
  __init___device_dts_ord_18: np_uart_0_init(__device_dts_ord_18)
  __init_uart_console_init: uart_console_init(NULL)
  __init_posix_arch_console_init: posix_arch_console_init(NULL)
PRE_KERNEL_2
  __init_sys_clock_driver_init: sys_clock_driver_init(NULL)
POST_KERNEL
  __init_enable_logger: enable_logger(NULL)
  __init_enable_shell_uart: enable_shell_uart(NULL)
  __init___device_dts_ord_31: gpio_emul_init(__device_dts_ord_31)
  __init___device_dts_ord_32: gpio_sdl_init(__device_dts_ord_32)
  __init_k_sys_work_q_init: k_sys_work_q_init(NULL)
  __init___device_dts_ord_10: gpio_keys_init(__device_dts_ord_10)
  __init___device_dts_ord_12: sdl_init(__device_dts_ord_12)
APPLICATION
  __init___device_dts_ord_16: sdl_display_init(__device_dts_ord_16)
  __init_lvgl_init: lvgl_init(NULL)
  __init___device_dts_ord_11: lvgl_button_input_init(__device_dts_ord_11)
  __init___device_dts_ord_13: lvgl_pointer_input_init(__device_dts_ord_13)
SMP

Yeah looks alright to me, the armclang linker must be discarding or changing the symbols, maybe the the binary is stripped? Hard to say without a file...

https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/bintools/armclang/elfconvert_command.cmake#L8 there's these bits of config kicking around, I checked and stripping would cause that error (obviously...)

${fail_on_warning}
)
endif()

if(NOT CMAKE_C_COMPILER_ID STREQUAL "ARMClang")
Copy link
Collaborator

Choose a reason for hiding this comment

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

not fond, of it, but I see your reasoning here.
#62459 (comment)

and users could want to run the target even with the kconfig to n.
I don't think this specific feature is strong enough to have something along TOOLCHAIN_CREATES_GOOD_ELF or similar.

So accepted for now, and let's see if we can find a way with armclang to support the check init feature.

Copy link
Member Author

@fabiobaltieri fabiobaltieri Sep 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I'm sure it can be fixed, those stripping options in cmake/bintools/armclang/elfconvert_command.cmake looks suspicious to me. It'd be cool to figure a way of having a CI run with these compilers so we don't depend on individuals to test stuff though.

I'll file an issue about it, it's worth discussing.

add_custom_target(
initlevels
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/build/check_init_priorities.py
--elf-file=${ZEPHYR_BINARY_DIR}/${KERNEL_ELF_NAME}
--initlevels
DEPENDS ${logical_target_for_zephyr_elf}
USES_TERMINAL
)
endif()

# Generate and use MCUboot related artifacts as needed.
if(CONFIG_BOOTLOADER_MCUBOOT)
get_target_property(signing_script zephyr_property_target SIGNING_SCRIPT)
2 changes: 2 additions & 0 deletions Kconfig.zephyr
View file
Original file line number Diff line number Diff line change
@@ -747,6 +747,8 @@ config BUILD_OUTPUT_STRIP_PATHS
config CHECK_INIT_PRIORITIES
bool "Build time initialization priorities check"
default y
depends on !NATIVE_LIBRARY
depends on "$(ZEPHYR_TOOLCHAIN_VARIANT)" != "armclang"
help
Check the build for initialization priority issues by comparing the
initialization priority in the build with the device dependency
Loading