Skip to content

Check union initialization portably #10178

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

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented May 15, 2025

Improved testing for #9814 and #9975. This PR just combines the bits and may not be needed if all submodules get updated through alternative PR.

Needs preceding PR:

PR checklist

@gilles-peskine-arm gilles-peskine-arm added the component-crypto Crypto primitives and low-level interfaces label May 15, 2025
@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests needs-preceding-pr Requires another PR to be merged first priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels May 15, 2025
@gilles-peskine-arm gilles-peskine-arm added the needs-backports Backports are missing or are pending review and approval. label May 15, 2025
@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label May 15, 2025
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM except for the submodule pointers:

  • I think framework no longer needs updating
  • crypto will need updating when the companion crypto PR is merged.

# disable the new problematic optimization.
loc_cflags="${loc_cflags} -fzero-init-padding-bits=unions"
# Also allow a warning that we don't yet comply to.
# Allow a warning that we don't yet comply to.
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, looking at current development if looks like this change was already done. I'm surprised github doesn't show it as a conflict. But overall I'm not sure we really need this PR, considering the only changes it's doing are either already done, or submodule updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-crypto Crypto primitives and low-level interfaces needs-backports Backports are missing or are pending review and approval. needs-preceding-pr Requires another PR to be merged first priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Development

Successfully merging this pull request may close these issues.

2 participants