Skip to content

Fix "flexible array member X not at end of struct" compiling with GCC-11 #348

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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;

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

Successfully merging this pull request may close these issues.

3 participants