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.

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

kernel: undefined reference with --no-gc-sections #26848

Closed
LeHack opened this issue Jul 14, 2020 · 9 comments · Fixed by #28194
Closed

kernel: undefined reference with --no-gc-sections #26848

LeHack opened this issue Jul 14, 2020 · 9 comments · Fixed by #28194
Assignees
Labels
area: Build System area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@LeHack
Copy link
Contributor

LeHack commented Jul 14, 2020

Describe the bug
When building with --no-gc-sections option to ld, reference to _sys_clock_tick_count (libmetal) is undefined

To Reproduce

  1. Add:
--- a/samples/basic/blinky/CMakeLists.txt
+++ b/samples/basic/blinky/CMakeLists.txt
@@ -4,4 +4,6 @@ cmake_minimum_required(VERSION 3.13.1)
 find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
 project(blinky)
 
+zephyr_ld_options(-Wl,--no-gc-sections)
+zephyr_ld_options(-Wl,--orphan-handling=place)
 target_sources(app PRIVATE src/main.c)
--- a/samples/basic/blinky/prj.conf
+++ b/samples/basic/blinky/prj.conf
@@ -1 +1,2 @@
 CONFIG_GPIO=y
+CONFIG_OPENAMP=y
  1. Build blinky:
west build -b nrf52840dk_nrf52840

Expected behavior
Sample builds.

Impact
Annoyance.

Logs and console output

arm-zephyr-eabi/bin/ld: modules/libmetal/libmetal/lib/libmetal.a(time.c.obj): in function `metal_get_timestamp':
/home/.../modules/hal/libmetal/libmetal/lib/system/zephyr/time.c:20: undefined reference to `_sys_clock_tick_count'

Environment (please complete the following information):

  • OS: Ubuntu Linux 18.04 (4.15.0-108)
  • Toolchain: Zephyr SDK v11.3
  • Commit SHA: 4ef29b3 (latest HEAD with modification above)

Additional context
A similar issue (but in another part of Zephyr) was reported not long ago: #20136

@LeHack LeHack added the bug The issue is a bug, or the PR is fixing a bug label Jul 14, 2020
@carlescufi
Copy link
Member

I don't think CONFIG_OPENAMP is enough to bring OpenAMP into the build, but @galak and @ioannisg can confirm.

@LeHack
Copy link
Contributor Author

LeHack commented Jul 16, 2020

@carlescufi Well certainly it is enough to trigger libmetal compilation and this error.
But of course the original code where I first encountered this issue involved a properly configured OpenAmp and proved to work without these compilation switches.

@galak
Copy link
Collaborator

galak commented Jul 16, 2020

@LeHack can you see if this PR fixes the issue:

OpenAMP/libmetal#122

@LeHack
Copy link
Contributor Author

LeHack commented Jul 16, 2020

@galak Yes it does!
Applying this commit as a cherry-pick to the current upstream version of libmetal fixes this issue.
I couldn't get it working with the whole PR, as the paths have changed within libmetal and I'm guessing this will require some include path updates in the zephyr build system (or at least a careful manifest manipulation).

@carlescufi
Copy link
Member

@LeHack can you see if this PR fixes the issue:

OpenAMP/libmetal#122

Will you update zephyr's OpenAMP repos with this fix @galak?

@galak
Copy link
Collaborator

galak commented Jul 20, 2020

Will you update zephyr's OpenAMP repos with this fix @galak?

I plan to once it gets merged upstream.

@MaureenHelm MaureenHelm added the priority: low Low impact/importance bug label Jul 21, 2020
@galak galak added the has-pr label Sep 8, 2020
@andrewboie
Copy link
Contributor

I'm not quite sure if --no-gc-sections is a supported configuration? Curious about your use-case at any rate.

@LeHack
Copy link
Contributor Author

LeHack commented Sep 8, 2020

@andrewboie Check the referenced issue (#20136). It contained a similar discussion on whether it should or shouldn't be supported and the final decision was that it should. Now I see that you were taking part in that discussion.

As for the use case, we are running some integration test scenarios on Nordic HW using a C/Python framework, which allows us to remotely control and execute code on the test platform. This way we can have a single hex built and sent to the DUT and then use Python to run a series of specific tests (using different methods/sequences/configuration) directly from the testing machine.
The problem is that since most of these embedded test methods are not run on their own, the linker treats them (correctly from it's point of view) as unused code.

So it's exactly what @SebastianBoe described: RPC execution happening without the linker knowing it.

@andrewboie
Copy link
Contributor

@andrewboie Check the referenced issue (#20136). It contained a similar discussion on whether it should or shouldn't be supported and the final decision was that it should. Now I see that you were taking part in that discussion.

As for the use case, we are running some integration test scenarios on Nordic HW using a C/Python framework, which allows us to remotely control and execute code on the test platform. This way we can have a single hex built and sent to the DUT and then use Python to run a series of specific tests (using different methods/sequences/configuration) directly from the testing machine.
The problem is that since most of these embedded test methods are not run on their own, the linker treats them (correctly from it's point of view) as unused code.

So it's exactly what @SebastianBoe described: RPC execution happening without the linker knowing it.

Ah I remember this now. Thanks for the detail.

galak added a commit to galak/zephyr that referenced this issue Sep 9, 2020
Update to a newer libmetal snapshot to pickup some upstream fixes

Fixes zephyrproject-rtos#26848

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
MaureenHelm pushed a commit that referenced this issue Sep 10, 2020
Update to a newer libmetal snapshot to pickup some upstream fixes

Fixes #26848

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants