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

core: build_id: support alternate GNU build ID sources #60

Closed

Conversation

JordanYates
Copy link

External build systems may already be enabling GNU build ID, in which case the Memfault provided implementation either breaks the upstream version or doesn't work.

CONFIG_MEMFAULT_PLATFORM_CONFIG enables users to override the configuration file if they desire.

By setting CONFIG_MEMFAULT_GNU_BUILD_ID_EXTERNAL=y and appropriately configuring MEMFAULT_GNU_BUILD_ID_SYMBOL, memfault_build_id.c will pick up whatever symbol is being created by the build system.

This enables integration with solutions such as zephyrproject-rtos/zephyr#51532, and presumably the final result of zephyrproject-rtos/zephyr#54464

Jordan Yates added 2 commits July 7, 2023 10:28
Let downstream users specify an alternate platform configuration file
through Kconfig.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Provide the option to link to an existing GNU build ID symbol.

Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
@JordanYates
Copy link
Author

JordanYates commented Jul 7, 2023

A .elf file created with this PR linking to an alternate symbol (__g_note_build_id) and manually uploaded to Memfault cloud works as expected:
image
image

@ejohnso49
Copy link

@JordanYates I'm working on these changes right now and I have a couple of questions:

  • For your example of linking to an alternate symbol, did you use your own linker script fragment similar to ports/zephyr/common/memfault-build-id.ld?
  • Could you explain the need to override memfault_zephyr_platform_config.h? Why not use the user-defined memfault_platform_config.h to override any values?

@JordanYates
Copy link
Author

My implementation is the same as what I proposed upstreaming here: zephyrproject-rtos/zephyr#51532.

For the config's I possibly am confused about which configs are applying where, there are many versions in the tree to keep track of :) I thought memfault_zephyr_platform_config.h was the version of memfault_platform_config.h used when compiling with Zephyr?

@ejohnso49
Copy link

For the config's I possibly am confused about which configs are applying where, there are many versions in the tree to keep track of :) I thought memfault_zephyr_platform_config.h was the version of memfault_platform_config.h used when compiling with Zephyr?

Probably means we could add some documentation around how these layer together. memfault_platform_config.h is intended for your application's config and overrides default SDK values. We could use some improvement in where this is placed and the file name but in general that's where you should override values like MEMFAULT_GNU_BUILD_ID_SYMBOL.

My implementation is the same as what I proposed upstreaming here: zephyrproject-rtos/zephyr#51532.

Great, I'm working on getting a PR for this review soon. Hope to have this included with our next release. I opted to build a menu around the build ID options

@JordanYates
Copy link
Author

Integrated with v1.1.3

@JordanYates JordanYates deleted the 230707_alternate_gnu_build_id branch September 14, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants