Skip to content

core: compile kernel_init and core_panic conditionally - #13238

Closed
nmeum wants to merge 1 commit into
RIOT-OS:masterfrom
nmeum:pr/riotboot_link_issues
Closed

core: compile kernel_init and core_panic conditionally#13238
nmeum wants to merge 1 commit into
RIOT-OS:masterfrom
nmeum:pr/riotboot_link_issues

Conversation

@nmeum

@nmeum nmeum commented Jan 30, 2020

Copy link
Copy Markdown
Member

Contribution description

riotboot provides its own implementation of these functions in bootloaders/riotboot/main.c. Compiling them unconditionally causes two symbols with the same name to be present for core_panic and kernel_init, the linker will discard one of them.

Testing procedure

This was discovered during the implementation of link time reordering in #13176. Either test this using the aforementioned PR or read the code and check with tests/riotboot that both bootloaders/riotboot/main.c and core/kernel_init.c as well as core/panic.c are compiled and contain symbols with the same name.

Issues/PRs references

RIOTBOOT provides its own implementation of these functions in
`bootloaders/riotboot/*`. Compiling them unconditionally causes two
symbols with the same name to be present, the linker will discard one of
them.
@benpicco benpicco added Area: core Area: RIOT kernel. Handle PRs marked with this with care! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 30, 2020
@nmeum

nmeum commented Jan 30, 2020

Copy link
Copy Markdown
Member Author

From Murdock:

--- result: BUILD FAILED!

--- static tests: failed!
dwqw: command timed out

What does this mean? What is dwqw?

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 30, 2020
@kaspar030

Copy link
Copy Markdown
Contributor

What does this mean? What is dwqw?

dwqw is the tool that executes jobs on the worker. The message means that the job timed out (ran for more than 5 minutes).

@benpicco benpicco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense and Murdock is happy.

@kaspar030

Copy link
Copy Markdown
Contributor

I'm not happy with the RIOTBOOT defines.
I suggest the following:

  • rename "kernel_init" "core_init"

  • make it a default (sub-)module

  • disable it for RIOTBOOT

  • move the minimal "panic()" to panic.c

  • make it selectable as module

I'll try how it looks and report back.

@kaspar030 kaspar030 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think turning both kernel_init and core panic into submodules is cleaner.

@nmeum

nmeum commented Jan 31, 2020

Copy link
Copy Markdown
Member Author

I prefer @kaspar030's solution closing this.

@nmeum nmeum closed this Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: core Area: RIOT kernel. Handle PRs marked with this with care! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants