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

Bogus alignment mismatch diagnostic #125916

Open
PJBoy opened this issue Feb 5, 2025 · 5 comments
Open

Bogus alignment mismatch diagnostic #125916

PJBoy opened this issue Feb 5, 2025 · 5 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer

Comments

@PJBoy
Copy link

PJBoy commented Feb 5, 2025

Testcase ( https://godbolt.org/z/o1b83cEbG )

#include <cstdint>

using UnalignedType [[gnu::aligned(1)]] = struct
{
    uint32_t x;
};

alignas(4) UnalignedType alignedVariable{};

void f(UnalignedType)
{}

static_assert(alignof(UnalignedType) == 1);
static_assert(alignof(alignedVariable) == 4);

int main()
{
    f(alignedVariable);
}

The diagnostic reported is

passing 1-byte aligned argument to 4-byte aligned parameter 'this' of 'UnalignedType' may result in an unaligned pointer access [-Walign-mismatch]
18 | f(alignedVariable);
| ^

However, the argument alignedVariable is 4-byte aligned, and I would expect UnalignedType to be a one-byte aligned parameter; the reported alignments seem to be reversed

@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Feb 5, 2025
@cor3ntin
Copy link
Contributor

cor3ntin commented Feb 5, 2025

f(alignedVariable); calls the copy constructor of the unnamed type. That defaulted copy constructor of the unnamed type expect a 4-bytes this pointer, which is sensible - and at the point of declaration of the constructor, the unnamed struct has no knowledge of the enclosing alias, so the attribute has no effect yet.

As @Endilll pointed out to me, #pragma pack does what we expect. https://godbolt.org/z/hxjv9Pxoe
So I think the diagnostic is correct but maybe applying [[gnu::aligned]] to aliases is not - or maybe we are missing checks when applying the attribute

@Endilll
Copy link
Contributor

Endilll commented Feb 5, 2025

So I think the diagnostic is correct but maybe applying [[gnu::aligned]] to aliases is not

I think you are right, giving that GCC doesn't agree that alignment of UnalignedType is 1: https://godbolt.org/z/13sfhT9c7

@AaronBallman
Copy link
Collaborator

From GCC's documentation:

When used on a struct, or struct member, the aligned attribute can only increase the alignment; in order to decrease it, the packed attribute must be specified as well. When used as part of a typedef, the aligned attribute can both increase and decrease alignment, and specifying the packed attribute generates a warning.

But I think that may only apply in C: https://godbolt.org/z/PnGbh6Eaa

@meator
Copy link

meator commented Feb 9, 2025

From GCC's documentation:

When used on a struct, or struct member, the aligned attribute can only increase the alignment; in order to decrease it, the packed attribute must be specified as well. When used as part of a typedef, the aligned attribute can both increase and decrease alignment, and specifying the packed attribute generates a warning.

But I think that may only apply in C: https://godbolt.org/z/PnGbh6Eaa

The original problem (which was the reason this issue was posted) is related to efivar's efi_guid_t type, which is a typedef, so aligned() is able to decrease the alignment in this case.

This is the struct:

typedef struct {
	uint32_t	a;
	uint16_t	b;
	uint16_t	c;
	uint16_t	d;
	uint8_t		e[6];
} efi_guid_t __attribute__((__aligned__(1)));

f(alignedVariable); calls the copy constructor of the unnamed type. That defaulted copy constructor of the unnamed type expect a 4-bytes this pointer, which is sensible - and at the point of declaration of the constructor, the unnamed struct has no knowledge of the enclosing alias, so the attribute has no effect yet.

If I understand your explanation correctly, there is no way to call f() with a well-aligned argument, because the default copy constructor will always be involved. Is that right?

I also find it weird that gcc doesn't care about alignment with all warnings enabled: https://test.godbolt.org/z/bPj514zx9

@AaronBallman
Copy link
Collaborator

If I understand your explanation correctly, there is no way to call f() with a well-aligned argument, because the default copy constructor will always be involved. Is that right?

That matches my understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer
Projects
None yet
Development

No branches or pull requests

7 participants