[Bug c/96293] New: Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[Bug c/96293] New: Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

            Bug ID: 96293
           Summary: Extraneously noisy "taking address of packed member
                    may result in an unaligned pointer value"
           Product: gcc
           Version: 9.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lavr at ncbi dot nlm.nih.gov
  Target Milestone: ---

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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
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))))

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #2 from lavr at ncbi dot nlm.nih.gov ---
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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

lavr at ncbi dot nlm.nih.gov changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---

--- Comment #3 from lavr at ncbi dot nlm.nih.gov ---
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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #5 from lavr at ncbi dot nlm.nih.gov ---
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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[ldeng at mail dot ustc.edu.cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #6 from Long Deng <ldeng at mail dot ustc.edu.cn> ---
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.

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #7 from lavr at ncbi dot nlm.nih.gov ---
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!

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #8 from lavr at ncbi dot nlm.nih.gov ---
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

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[ldeng at mail dot ustc.edu.cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #9 from Long Deng <ldeng at mail dot ustc.edu.cn> ---
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;
}
```

[Bug c/96293] Extraneously noisy "taking address of packed member may result in an unaligned pointer value"

[lavr at ncbi dot nlm.nih.gov]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96293

--- Comment #10 from lavr at ncbi dot nlm.nih.gov ---
> 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.