[Bug c/53037] New: warn_if_not_aligned(X)

[Bug c/53037] New: warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

             Bug #: 53037
           Summary: warn_if_not_aligned(X)
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com


What I'd like to have is an __attribute__((warn_if_not_aligned(X)))
which issues a warning if the object/type/field in question is not
aligned to X.  In particular, I'd like to build with:

typedef unsigned long long __u64
       __attribute__((aligned(4),warn_if_not_aligned(8)));

... in order to get a list of the places where we have misaligned 64-bit
data pieces in x86.  Then we can do Linus' requested cleanups which
ultimately aims to getting __u64 to be __attribute__((aligned(8))) even
on i386, with an explicit __compat_u64 for the places where we need to
be misaligned for legacy reasons.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 00:33:55 UTC ---
We need to add another field to tree_type_common and tree_decl_common to
store the warn_if_not_aligned value.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 15:47:14 UTC ---
Given

typedef unsigned long long __u64 __attribute__((aligned(4)));

all most all __u64 will be aligned at 4.   The only case we may
do something about is

typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  int i3;

  __u64 x;
};

since alignment of the x field also depends on alignment of
struct foo.

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #3 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 15:51:35 UTC ---
Logically, about half of u64's will be properly aligned at the moment... Linus'
request is that we flag the currently misaligned __[su]64's as __compat_[su]64
and make __[su]64 aligned, so at least *new* interfaces will be properly
aligned.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 16:00:41 UTC ---
(In reply to comment #3)
> Logically, about half of u64's will be properly aligned at the moment... Linus'

No necessarily. For

u64 x;
int y;
u64 z;

both x and z may be 4 byte aligned.

> request is that we flag the currently misaligned __[su]64's as __compat_[su]64
> and make __[su]64 aligned, so at least *new* interfaces will be properly
> aligned.

Is this feature only used for function parameters?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #5 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 16:05:29 UTC ---
On 04/19/2012 09:00 AM, hjl.tools at gmail dot com wrote:
> 
>> request is that we flag the currently misaligned __[su]64's as __compat_[su]64
>> and make __[su]64 aligned, so at least *new* interfaces will be properly
>> aligned.
> 
> Is this feature only used for function parameters?
> 

No.

    -hpa

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #6 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 16:53:18 UTC ---
For a global or local 64bit variable, x, inside kernel,
why should it be 4 byte aligned if it isn't part of system
call interface?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #7 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 16:57:14 UTC ---
The __u64/__s64 types are used for interfaces only.  The kernel itself is
x86-64 and uses aligned types for internal uses.

The mismatch between i386 and x86-64 alignment has a tendency to cause
unexpected bugs, and Linus would like to avoid those by having new interfaces
use aligned types consistently.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 17:07:20 UTC ---
Shouldn't

typedef unsigned long long __u64 __attribute__((aligned(4)));

only be used in system call interface?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #9 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 17:11:00 UTC ---
Yes.

The point is: WE WANT TO MIGRATE THE SYSTEM CALL INTERFACE TO AN ALIGNED
__[us]64 INTERFACE, mostly so that new interfaces are properly aligned from the
start.

In order to do that, we need to flag the existing legacy interfaces which will
need to be flagged as __compat_[us]64 in order to not break.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 17:20:42 UTC ---
Isn't checking alignment of x in:

typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  int i3;

  __u64 x;
};

sufficient for kernel interface purpose?  Is there another case
we need to check?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #11 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 17:42:28 UTC ---
Sorry, that should be sufficient.  I'm not awake today, it seems.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #12 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 20:15:55 UTC ---
Created attachment 27197
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27197
A patch

I got

[hjl@gnu-6 pr53037]$ cat x.i
typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo1
{
  int i1;
  int i2;
  int i3;
  __u64 x;
};

struct foo2
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

struct foo3
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

struct foo4
{
  int i1;
  int i3;
  __u64 x;
};

union bar1
{
  int i1;
  int i3;
  __u64 x;
};

union bar2
{
  int i1;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));
[hjl@gnu-6 pr53037]$ make x.s
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -mx32 -Wall -S x.i
x.i:10:1: warning: alignment 32 of ‘struct foo1’ is less than 64 [enabled by
default]
 };
 ^
x.i:10:1: warning: ‘x’ offset 12 in ‘struct foo1’ isn't aligned to 8 [enabled
by default]
 };
 ^
x.i:18:1: warning: ‘x’ offset 12 in ‘struct foo2’ isn't aligned to 8 [enabled
by default]
 } __attribute__((aligned(8)));
 ^
x.i:32:1: warning: alignment 32 of ‘struct foo4’ is less than 64 [enabled by
default]
 };
 ^
x.i:39:1: warning: alignment 32 of ‘union bar1’ is less than 64 [enabled by
default]
 };
 ^
[hjl@gnu-6 pr53037]$

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27197|0                           |1
        is obsolete|                            |

--- Comment #13 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 20:18:56 UTC ---
Created attachment 27198
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27198
A better patch

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #14 from H. Peter Anvin <hpa at zytor dot com> 2012-04-19 20:24:25 UTC ---
Are the last two warnings in bits (as opposed to bytes)?  It looks a little
confusing...

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #15 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 20:41:47 UTC ---
(In reply to comment #14)
> Are the last two warnings in bits (as opposed to bytes)?  It looks a little
> confusing...

It is fixed by the updated patch.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #16 from H.J. Lu <hjl.tools at gmail dot com> 2012-04-19 21:06:49 UTC ---
Created attachment 27199
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27199
A smaller patch

There is no point to support

struct foo
{
  int i1;
  long long i2  __attribute__((aligned(4), warn_if_not_aligned(8)));
};

since GCC won't decrease alignment in field.  The smaller patch gave

[hjl@gnu-6 pr53037]$ cat x.i
typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo1
{
  int i1;
  int i2;
  int i3;
  __u64 x;
};

struct foo2
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

struct foo3
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

struct foo4
{
  int i1;
  int i3;
  __u64 x;
};

union bar1
{
  int i1;
  int i3;
  __u64 x;
};

union bar2
{
  int i1;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));
[hjl@gnu-6 pr53037]$ make x.s
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -mx32 -Wall -O2 -S x.i
x.i:10:1: warning: alignment 4 of ‘struct foo1’ is less than 8 [enabled by
default]
 };
 ^
x.i:9:9: warning: ‘x’ offset 12 in ‘struct foo1’ isn't aligned to 8 [enabled by
default]
   __u64 x;
         ^
x.i:17:9: warning: ‘x’ offset 12 in ‘struct foo2’ isn't aligned to 8 [enabled
by default]
   __u64 x;
         ^
x.i:32:1: warning: alignment 4 of ‘struct foo4’ is less than 8 [enabled by
default]
 };
 ^
x.i:39:1: warning: alignment 4 of ‘union bar1’ is less than 8 [enabled by
default]
 };
 ^
[hjl@gnu-6 pr53037]$

[Bug c/53037] warn_if_not_aligned(X)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |crillion at tiscali dot it

--- Comment #17 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-02-13 10:38:44 UTC ---
*** Bug 56304 has been marked as a duplicate of this bug. ***

[Bug c/53037] warn_if_not_aligned(X)

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

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

--- Comment #43 from Tom de Vries <vries at gcc dot gnu.org> ---
*** Bug 81909 has been marked as a duplicate of this bug. ***