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 "flexible array member X not at end of struct" compiling with GCC-11 #348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maurimo
Copy link

@maurimo maurimo commented Jan 13, 2023

Fix a problem compiling with GCC-11 (present when I compiled latest Torch which includes gloo). The problem was also reported on Debian's ML, see for full context:
https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1875386.html

This problem should not be occurring in the first place and it's probably a bug of GCC, but luckily the workaround is quite simple too.

Copy link

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

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

This is indeed correct while #346 is wrong. Needs rebase/Fix after that got already merged though.

CC @kumpera

} ecmd;
constexpr auto bufsize = sizeof(struct ethtool_link_settings) + sizeof(__u32)*link_mode_data_nwords;
char buf[bufsize];
struct ethtool_link_settings& ecmd = *(struct ethtool_link_settings*)buf;

Choose a reason for hiding this comment

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

IMO this is technically a strict aliasing violation and hence UB. At least buf needs to be aligned as struct ethtool_link_settings and possibly a placement new needs to be used.

Idea: Using an union to enforce the size might work:

  constexpr auto bufsize = sizeof(struct ethtool_link_settings) + sizeof(__u32)*link_mode_data_nwords;
  union {
    struct ethtool_link_settings req;
    char buf[bufsize];
  } ecmd;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants