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

plat/common: Ensure the .eh_frame section stays #776

Conversation

mschlumpp
Copy link
Member

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): N/A
  • Platform(s): N/A
  • Application(s): N/A

Additional configuration

  • CONFIG_OPTIMIZE_DEADELIM=y

Description of changes

Using the CONFIG_OPTIMIZE_DEADELIM KConfig option adds the --gc-sections command-line flag to the linker. Because the .eh_frame section is not really referenced anywhere the linker will happily throw it away. The solution is to mark it with KEEP in the linker scripts.

Using the `CONFIG_OPTIMIZE_DEADELIM` KConfig option adds the
`--gc-sections` command-line flag to the linker. Because the `.eh_frame`
section is not really referenced anywhere the linker will happily throw
it away. The solution is to mark it with `KEEP` in the linker scripts.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@mschlumpp mschlumpp requested a review from a team as a code owner March 3, 2023 09:45
@unikraft-bot unikraft-bot added area/plat Unikraft Patform plat/common Common to all platforms labels Mar 3, 2023
@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
472f2a8 plat/common: Ensure the `.eh_frame` section stays

@razvand razvand requested review from mogasergiu and removed request for a team March 13, 2023 12:34
@razvand razvand added this to the v0.13.0 (Atlas) milestone Mar 13, 2023
Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

The change makes sense 👍

All good here. Thanks!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@unikraft.io

Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

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

Looks good.

Reviewed-by: Sergiu Moga sergiu.moga@protonmail.com

@craciunoiuc
Copy link
Member

@nderjung 👀

@dinhngtu
Copy link
Member

hi, I have a small question: do .gcc_except_table sections need to be kept as well?

@mschlumpp
Copy link
Member Author

@dinhngtu The data in .gcc_except_table should be referenced by .eh_frame entries. Therefore, the linker shouldn't need an explicit KEEP directive for them. (The default linker script on Linux also doesn't use a KEEP directive for the section)

@craciunoiuc
Copy link
Member

@nderjung 👀👀

Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Updated my review status, see below

Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Thanks!

Approved-by: Alexander Jung alex@unikraft.io

@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Apr 19, 2023
@mschlumpp mschlumpp deleted the mschlumpp/fix/keep-eh-frame-section branch April 19, 2023 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plat Unikraft Patform ci/merged Merged by CI plat/common Common to all platforms
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

7 participants