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

fix libfdt linking when compiled using -fstack-protector #2595

Merged
merged 4 commits into from
Jun 3, 2021

Conversation

serban300
Copy link
Contributor

Reason for This PR

fix libfdt linking when compiled using -fstack-protector

Description of Changes

As part of #2579 we pushed a commit that involved compiling libfdt using -fno-stack-protector in order to workaround rust-lang/rust#79791 .

This is a cleaner fix that enables us to compile firecracker using rust toolchain 1.52.1 even when libfdt is compiled using -fstack-protector

  • This functionality can be added in rust-vmm.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any newly added unsafe code is properly documented.
  • Any API changes are reflected in firecracker/swagger.yaml.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

@serban300 serban300 added Status: Awaiting review Indicates that a pull request is ready to be reviewed NextRelease labels May 27, 2021
@serban300 serban300 self-assigned this May 27, 2021
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Changes look good, I only have a comment around isolating x86_64 from this change.

Comment on lines +1 to +6
[package]
name = "libfdt-bindings"
version = "0.1.0"
authors = ["Amazon Firecracker team <[email protected]>"]
edition = "2018"
build = "build.rs"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer we only build this under aarch64. Either with an arch gate at the crate level (the crate wouldn't even exist on other archs), or with a code-level gate (crate exists but is empty).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to add an arch gate at the crate level, but there are code-level gates. Also I added a gate for the dependencies.

luminitavoicu
luminitavoicu previously approved these changes Jun 3, 2021
acatangiu
acatangiu previously approved these changes Jun 3, 2021
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

💯

Serban Iorga added 3 commits June 3, 2021 14:03
acatangiu
acatangiu previously approved these changes Jun 3, 2021
Signed-off-by: Serban Iorga <[email protected]>
@serban300 serban300 merged commit 7efc012 into firecracker-microvm:main Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants