[Bug c/56337] New: __attribute__((aligned(N))) allows too-high values of N

[Bug c/56337] New: __attribute__((aligned(N))) allows too-high values of N

[brooks at gcc dot gnu.org]

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

             Bug #: 56337
           Summary: __attribute__((aligned(N))) allows too-high values of
                    N
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: brooks@gcc.gnu.org


This is a pretty minor thing, but consider the following .c file, which defines
four arrays with very high alignments:

  float __attribute__((aligned(1<<25))) a[128];
  float __attribute__((aligned(1<<26))) b[128];
  float __attribute__((aligned(1<<27))) c[128];
  float __attribute__((aligned(1<<28))) d[128];

For the smaller three alignments, the .comm directive in the assembly is
correct.  However, for the alignment of 1<<28, the alignment entry goes wildly
wrong:

  $ i686-pc-linux-gnu-gcc  -O2 -S align6.c -o-
        .file   "align6.c"
        .comm   d,512,4026531840
        .comm   c,512,134217728
        .comm   b,512,67108864
        .comm   a,512,33554432
        .ident  "GCC: (GNU) 4.8.0 20130131 (experimental)"
        .section        .note.GNU-stack,"",@progbits

This is probably a pretty minor bug; it seems unlikely to arise in real
practice.  I'm only filing it because I expect the solution is likewise trivial
-- if we try an alignment of 1<<29, we get an error of "requested alignment is
too large".  The threshold for this error merely needs to be lowered by a
factor of two.

This patch from H.J. Lu on bug 39323 appears to be the relevant code in
gcc/config/elfos.h, although that hunk of the patch was never applied:
http://gcc.gnu.org/ml/gcc-patches/2009-02/msg01274.html

[Bug c/56337] __attribute__((aligned(N))) breaks for N=1<<28

[brooks at gcc dot gnu.org]

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

--- Comment #1 from Brooks Moses <brooks at gcc dot gnu.org> 2013-02-15 07:06:45 UTC ---
Note that pr39323-2.c tests a closely-related case that presumably is working
correctly:

http://gcc.gnu.org/viewcvs/trunk/gcc/testsuite/gcc.dg/pr39323-2.c?view=markup

[Bug c/56337] __attribute__((aligned(N))) breaks for N=1<<28

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-02-15
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> 2013-02-15 09:26:16 UTC ---
That's because we record alignment in bits but do not check whether

      DECL_ALIGN (decl) = (1U << i) * BITS_PER_UNIT;

overflows (DECL_ALIGN has unsigned int storage).  Confirmed.

I think too large alignment specifications should saturate to
MAX_OFILE_ALIGNEMNT
(and of course warn).

[Bug middle-end/56337] __attribute__((aligned(N))) breaks for N=1<<28

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end
             Target|                            |x86_64-linux-gnu

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
x86_64:
[apinski@xeond2 gcc]$ ~/upstream-gcc/bin/gcc t.c -O2 -S -o -   -fcommon
        .file   "t.c"
        .text
        .comm   d,512,4026531840
        .comm   c,512,134217728
        .comm   b,512,67108864
        .comm   a,512,33554432
        .ident  "GCC: (GNU) 12.0.0 20210710 (experimental) [master revision
783ce7c191d:c9543701c18:eaa5225c686d496244a6614ba2aacbf02e0f4daa]"
        .section        .note.GNU-stack,"",@progbits

aarch64:
ubuntu@ubuntu:~/src/upstream-gcc-aarch64/gcc/objdir/gcc\# gcc  t.c -o -
-fcommon -S
        .arch armv8-a
        .file   "t.c"
        .text
        .comm   a,512,33554432
        .comm   b,512,67108864
        .comm   c,512,134217728
        .comm   d,512,268435456
        .ident  "GCC: (GNU) 12.0.0 20210721 (experimental) [master revision
ccf3e245d4e:8edb685b0dd:a50b951c25631b9c3a5dbfa89488e84117961be1]"
        .section        .note.GNU-stack,"",@progbits


----- CUT -----
So it looks like the problem is in the printing out the alignment for x86_64.

[Bug target/56337] __attribute__((aligned(N))) breaks for N=1<<28

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
          Component|middle-end                  |target

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The problem is in the x86_64 backend.  It uses int align when it should have
been unsigned align.

I have a patch:
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 51376fcc454..d8dac87c61b 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -325,9 +325,9 @@ struct ix86_address
 extern int ix86_decompose_address (rtx, struct ix86_address *);
 extern int memory_address_length (rtx, bool);
 extern void x86_output_aligned_bss (FILE *, tree, const char *,
-                                   unsigned HOST_WIDE_INT, int);
+                                   unsigned HOST_WIDE_INT, unsigned);
 extern void x86_elf_aligned_decl_common (FILE *, tree, const char *,
-                                        unsigned HOST_WIDE_INT, int);
+                                        unsigned HOST_WIDE_INT, unsigned);

 #ifdef RTX_CODE
 extern void ix86_fp_comparison_codes (enum rtx_code code, enum rtx_code *,
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ff96134fb37..ad5717b3c57 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -837,7 +837,7 @@ x86_64_elf_unique_section (tree decl, int reloc)
 void
 x86_elf_aligned_decl_common (FILE *file, tree decl,
                        const char *name, unsigned HOST_WIDE_INT size,
-                       int align)
+                       unsigned align)
 {
   if ((ix86_cmodel == CM_MEDIUM || ix86_cmodel == CM_MEDIUM_PIC)
       && size > (unsigned int)ix86_section_threshold)
@@ -858,7 +858,7 @@ x86_elf_aligned_decl_common (FILE *file, tree decl,

 void
 x86_output_aligned_bss (FILE *file, tree decl, const char *name,
-                       unsigned HOST_WIDE_INT size, int align)
+                       unsigned HOST_WIDE_INT size, unsigned align)
 {
   if ((ix86_cmodel == CM_MEDIUM || ix86_cmodel == CM_MEDIUM_PIC)
       && size > (unsigned int)ix86_section_threshold)

[Bug target/56337] __attribute__((aligned(N))) breaks for N=1<<28

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2021-July/57
                   |                            |5967.html

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch submitted:
https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575967.html

[Bug target/56337] __attribute__((aligned(N))) breaks for N=1<<28

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:e4d86078881bb7bb57bc6e68c22211707d2b3dc7

commit r12-3242-ge4d86078881bb7bb57bc6e68c22211707d2b3dc7
Author: Andrew Pinski <apinski@marvell.com>
Date:   Fri Jul 23 17:21:06 2021 +0000

    Fix x86/56337 : 1<<28 alignment is broken

    The problem here is the x86_64 back-end uses a signed integer
    for alignment and then divides by BITS_PER_UNIT so if we had
    INT_MIN (which is what 1<<28*8 is), we would get the wrong result.

    This fixes the problem by using unsigned for the argument to
    x86_output_aligned_bss and x86_output_aligned_bss.

    OK? Bootstrapped and tested on x86_64-linux-gnu.

    gcc/ChangeLog:

            PR target/56337
            * config/i386/i386-protos.h (x86_output_aligned_bss):
            Change align argument to unsigned type.
            (x86_elf_aligned_decl_common): Likewise.
            * config/i386/i386.c (x86_elf_aligned_decl_common): Likewise.
            (x86_output_aligned_bss): Likewise.

[Bug target/56337] __attribute__((aligned(N))) breaks for N=1<<28

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.