Bug 96293 - Extraneously noisy "taking address of packed member may result in an unaligned pointer value"
Summary: Extraneously noisy "taking address of packed member may result in an unaligne...
Status: UNCONFIRMED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 9.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-23 00:23 UTC by lavr
Modified: 2022-07-07 13:13 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description lavr 2020-07-23 00:23:20 UTC
Hi,

Beginning version 9 GCC started to generate tons of warnings of "unaligned pointer value".  While this may be useful in some cases, many of these warnings should not be emitted (because the packed attribute is generally used by those, who follow the layout and usually know, what they are doing), IMO.

Consider the following:

```
#include <stdio.h>

struct S {
    char          a;
    unsigned char b;
    short         c;
    unsigned int  d;
} __attribute__((packed));

void f1(char* p)
{
   *p = '0';
}

void f2(unsigned char* p)
{
   *p = '1';
}

void f3(short* p)
{
    *p = 2;
}

void f4(unsigned int* p)
{
    *p = 3;
}

int main()
{
    struct S s;

    f1(&s.a);
    f2(&s.b);
    f3(&s.c);
    f4(&s.d);
    printf("%c %c %hd %u\n", s.a, s.b, s.c, s.d);
    return 0;
}
```

On most platforms all members of the packed 'struct S' above are well-aligned to their respective type boundary, and the warnings like:
```
test.c: In function ‘main’:
test.c:36:8: warning: taking address of packed member of ‘struct S’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   36 |     f3(&s.c);
      |        ^~~~
test.c:37:8: warning: taking address of packed member of ‘struct S’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   37 |     f4(&s.d);
      |        ^~~~

```
are distracting and unhelpful!

I'd say that GCC should only issue a warning about the alignment if the packed member does not indeed align with its natural boundary (what would have been assigned by GCC if the structure wasn't packed).  That is, if the "b" member above would have been missing, then the warnings are indeed legitimate.  Otherwise, "c", being a two-byte short integer, is perfectly fine to be located at offset 2, and "d" (a 4 byte integer) is perfectly fine to be located at offset 4, there's no need for any warnings.

The same rule should apply for aggregates, too.

Thanks for considering this suggestion.
Comment 1 Richard Biener 2020-07-23 06:02:49 UTC
The packed attribute means that _Alignof (S) is 1.  If you want the structure
to be aligned to its largest member you have to use __attribute__((packed,aligned(__alignof__(unsigned int))))
Comment 2 lavr 2020-07-23 14:59:50 UTC
I don't want my structure to be aligned at the int boundary.  I want my structure to reflect the actual data layout "byte","byte","short","int" as they are laid out without any gaps, and "packed" guarantees such a disposition.  I also don't want GCC issue warnings "just in case" where there's nothing happening: like I said the "short int" field is located at the native "short int" offset (multiple of 20, so there's no need for the warning; and so on with the int field.  GCC should worry only if, for example, an int is placed at an odd offset, or, for 4 byte ints, at an offset not multiple of 4.
Comment 3 lavr 2020-07-23 15:01:00 UTC
I don't want my structure to be aligned at the int boundary.  I want my structure to reflect the actual data layout "byte","byte","short","int" as they are laid out without any gaps, and "packed" guarantees such a disposition.  I also don't want GCC issue warnings "just in case" where there's nothing happening: like I said the "short int" field is located at the native "short int" offset (multiple of 20, so there's no need for the warning; and so on with the int field.  GCC should worry only if, for example, an int is placed at an odd offset, or, for 4 byte ints, at an offset not multiple of 4.
Comment 4 Jakub Jelinek 2020-07-27 11:15:59 UTC
Your testcase is invalid, so it certainly deserves a warning.
One thing is that e.g. the int member is at offset 4, which is a multiple of sizeof (int), but that doesn't help when the structure has alignment of 1 due to packed, at which point the s variable doesn't have any alignment guarantees, so (uintptr_t) &s.d % sizeof (int) can be any value from 0 to sizeof (int) - 1.
Comment 5 lavr 2020-07-27 16:08:52 UTC
My test case is not invalid.  You're talking about base address of a structure, which would make offsets within it all unaligned, which is why I said "the same rule should apply for aggregates".  So should "struct S" appear anywhere in an outer structure at an offset, which would not be its "native offset otherwise assigned by GCC, as if it wasn't packed", then there's a potential problem.  Otherwise, the member addresses will still remain well-aligned, and no warnings would be ever necessary.  As for your example, any structure's address can be type-cast to any value (it's C, for the sake of it), yet GCC doesn't assume that happens for non-packed structures, right?

Extra warnings in large projects are disruptive, and if GCC warns about something, it should be well-warranted and verified.  Applying a bit of an extra-logic (which does not take a lot of CPU cycles, or additional compilation time) for those new warnings, and not issuing them just mechanically, would help a lot.
Comment 6 Long Deng 2022-07-06 10:05:57 UTC
I met the same problem. I found that gcc issue this warning probably because `struct S` can located any address, which means that `s.d` may not alignment to 4.
So as Richard said, you can use `__attribute__((packed, aligned(4))` to get rid of this warning, it still guaranteed that you struct is packed (i.e. `sizeof(S) == 8`), and struct itself will alignment to 4, so `s.d` will aligns correctly.
Comment 7 lavr 2022-07-06 14:05:44 UTC
The problem with the aligned(4) attribute is that if this structure appears as a member of an outer packed structure, it may not be "enclosed" properly without a gap.

The warnings are pointless is they are emitted for members that are located already aligned to their native boundaries.  GCC is perfectly aware of all the offsets and it so can only warn of those, which are indeed off.

Otherwise, it's the same if GCC begins warning about taking address of any member, whose structure pointer was passed to a function, because that pointer may not be assumed as properly aligned (any structure pointer passed to a function is align(1) because the pointer's origin may not be generally even known!).
But that'd be ridiculous!
Comment 8 lavr 2022-07-06 15:15:15 UTC
Consider the following code:

struct record {
    unsigned char  len;
    unsigned short data[5];
} __attribute__((packed));


struct attribute {
    unsigned char code;
    struct record record;
} __attribute__((packed));

If I had "struct attribute* a" in my code then "&a->record.data[0]" would be a well-aligned access, even though "struct record" is not "properly aligned".  If I had "struct record" defined with the align(2) attribute (to cater to "short"s), it might have been placed in "struct attribute" at offset 2, leaving a one byte gap after the "code" member.  Newer gcc gives even more warnings where there should be none (but fortunately, it does not leave a gap: I can't check the behavior with GCC 9 anymore because I do no longer have it).

$ cat align.c
#include <stdio.h>


#ifdef ALIGN2
#  define ALIGN  __attribute__((packed, aligned(2)))
#else
#  define ALIGN  __attribute__((packed))
#endif


void fun(unsigned short* n)
{
    *n = 0xCAFE;
}


struct record {
    unsigned char  len;
    unsigned short data[5];
} ALIGN;


struct attribute {
    unsigned char code;
    struct record record;
} __attribute__((packed));


int main()
{
    struct attribute a;
    printf("offsetof data = %zu\n",
           offsetof(struct attribute, record.data[0]));
    fun(&a.record.data[0]);
    return 0;
}

$ gcc -Wall -o align align.c
align.c: In function ‘main’:
align.c:34:9: warning: taking address of packed member of ‘struct record’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   34 |     fun(&a.record.data[0]);
      |         ^~~~~~~~~~~~~~~~~

$ ./align
offsetof data = 2

$ gcc -Wall -DALIGN2 -o align align.c
align.c:26:1: warning: alignment 1 of ‘struct attribute’ is less than 2 [-Wpacked-not-aligned]
   26 | } __attribute__((packed));
      | ^
align.c:25:19: warning: ‘record’ offset 1 in ‘struct attribute’ isn’t aligned to 2 [-Wpacked-not-aligned]
   25 |     struct record record;
      |                   ^~~~~~
align.c: In function ‘main’:
align.c:34:9: warning: taking address of packed member of ‘struct record’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   34 |     fun(&a.record.data[0]);
      |         ^~~~~~~~~~~~~~~~~

$ ./align
offsetof data = 2
Comment 9 Long Deng 2022-07-07 08:23:12 UTC
I take your point, although there seems to be a slight problem with your example. In your code, `&a->record.data[0]` is not a well-aligned access, because `struct attribute` is defined as packed, so compiler has no any information about where `a` itself is aligned to. `a` could start at any address so `a->record.data` might not well-aligned.

Maybe a better example is defining `struct record` as `((packed))` and `struct attribute` as `((packed, aligned(2))`, which ensure `a` is well-aligned. Even in this case, gcc still issue the warning.

Full example:

```
#include <stdio.h>
#include <stddef.h>
#include <stdint.h>
#include <assert.h>

struct Unaligned {
    uint8_t b;
    uint32_t c;
    uint16_t d;
}__attribute__((packed));

struct Aligned {
    uint8_t d;
    struct Unaligned u;
}__attribute__((packed, aligned(8)));
static_assert(sizeof(struct Aligned) == 8);

void fun(uint16_t* addr) {
    *addr = 1234;
}

int main() {
    struct Aligned a;
    printf("addr of a: %p\n", &a);
    printf("offsetof d: %zu\n",offsetof(struct Aligned, u.d));
    fun(&a.u.d);
    return 0;
}
```
Comment 10 lavr 2022-07-07 13:13:35 UTC
> In your code, `&a->record.data[0]` is not a well-aligned access

It is well-aligned, its offset gets printed out by the very test code, and it's 2.

> because `struct attribute` is defined as packed, so compiler has no any
> information about where `a` itself is aligned to.

Are you kidding me?  "a" is an automatic object on stack, which compiler itself
allocates.  So it is well-aligned, for sure.

Even if "a" was coming as an argument of a function call (via a pointer), the compiler should assume it's aligned, because it always does for any other regular structures.