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

compilation errors because of references to packed struct members #47

Closed
jsynacek opened this issue Oct 21, 2019 · 7 comments
Closed

compilation errors because of references to packed struct members #47

jsynacek opened this issue Oct 21, 2019 · 7 comments
Assignees

Comments

@jsynacek
Copy link

With gcc 9.2.1, I'm getting a lot of these during the compilation of ledmon-0.93:

...
amd_sgpio.c:482:18: error: taking address of packed member of 'struct transmit_register' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  482 |  _dump_sgpio_req(&tx_reg->req);
      |                  ^~~~~~~~~~~~
amd_sgpio.c: In function '_write_tx_register':
amd_sgpio.c:489:18: error: taking address of packed member of 'struct transmit_register' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  489 |  _init_sgpio_hdr(&tx_reg->hdr, 0, sizeof(*tx_reg));
      |                  ^~~~~~~~~~~~
amd_sgpio.c:490:18: error: taking address of packed member of 'struct transmit_register' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  490 |  _init_sgpio_req(&tx_reg->req, 0x40, 0x82, SGPIO_REQ_REG_TYPE_TX, 0, 1);
      |                  ^~~~~~~~~~~~
amd_sgpio.c: In function '_dump_amd_register':
amd_sgpio.c:543:2: error: converting a packed 'struct amd_register' pointer (alignment 1) to a 'uint32_t' {aka 'unsigned int'} pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member]
  543 |  uint32_t *reg = (uint32_t *)amd_reg;
      |  ^~~~~~~~
amd_sgpio.c:144:8: note: defined here
  144 | struct amd_register {
      |        ^~~~~~~~~~~~
amd_sgpio.c:548:18: error: taking address of packed member of 'struct amd_register' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  548 |  _dump_sgpio_hdr(&amd_reg->hdr);
      |                  ^~~~~~~~~~~~~
...

To be honest, I don't know if such references are even supposed to work, but I can imagine that the generated code might act funny on architectures like ppc64.

@mdabrows
Copy link
Contributor

@nfont could you take a look on this issue? -Waddress-of-packed-member is enabled by default with gcc-9, so it seems that amd_sgpio code has to be refactored to not use pointers to members of packed structures.

@jsynacek
Copy link
Author

I've consulted this with people who know more than I do about this topic and I was told that the code itself should be safe, as architectures that do not support unaligned memory accesses are rare. However, I still think it's better to not use them, unless there is a really strong reason to do so, which I can't even think of and, in my opinion, is not the case here.

@nfont
Copy link
Contributor

nfont commented Oct 25, 2019

I can take a look at updating the amd code. The use of the packed structures were to keep the compiler from padding any of the structures so they can be written directly to the sysfs file.

@jsynacek
Copy link
Author

The use of packed structures is not a problem. The problem is the use of references to the members of those structures in function calls. Having a function(s) that take(s) a reference to the entire structure and then deal(s) with the members inside should be fine.

@mdabrows
Copy link
Contributor

Hello @nfont do you have any progress on this issue?

Regards,
Mariusz

mdabrows pushed a commit that referenced this issue Nov 29, 2019
New compiler versions might output new warnings by default
(see #47 for an example), which makes -Werror problematic
for distributions.

The more specific -Werror= settings are usually not a problem.

The warnings are still shown, but they will no longer break
the build.

Signed-off-by: Adrian Bunk <bunk@debian.org>
@nfont
Copy link
Contributor

nfont commented Dec 2, 2019

I have code that I have been testing and will get a pull request put together this afternoon.

@nfont
Copy link
Contributor

nfont commented Dec 2, 2019

I opened pull request #52 to resolve this issue.

@rsobans rsobans closed this as completed Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants