[Bug target/105549] New: aarch64: Wrong va_arg alignment handling

[Bug target/105549] New: aarch64: Wrong va_arg alignment handling

[clyon at gcc dot gnu.org]

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

            Bug ID: 105549
           Summary: aarch64: Wrong va_arg alignment handling
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: clyon at gcc dot gnu.org
  Target Milestone: ---

While working on enabling DFP for AArch64, I noticed new failures in
gcc.dg/compat/struct-layout-1.exp (t028) which were not actually
caused by DFP types handling.  I reduced the problem to:

  /* This test is derived from a case generated by struct-layout-1.exp:  */
enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 };
typedef enum E6 Tal16E6 __attribute__((aligned (16)));
typedef unsigned int Tuint;
int fails;
union S2844 {
  Tuint a:((((10) - 1) & 31) + 1);
  Tal16E6 __attribute__((aligned (2), packed)) b:31;
  struct{}c[0];
} ;
union S2844 s2844;
union S2844 a2844[5];
typedef __builtin_va_list __gnuc_va_list;
typedef __gnuc_va_list va_list;

void check2844va (int z, ...) {
  union S2844 arg, *p;
  va_list ap;

  __builtin_va_start(ap,z);
  if (__builtin_va_arg(ap,double) != 1.0)
    printf ("fail %d.%d\n", 2844, 0), ++fails;

  p = &s2844;
  arg = __builtin_va_arg(ap,union S2844);  /* This would fail.  */
  if (p->a != arg.a)
    printf ("fail %d.%d\n", 2844, 1), ++fails;

  if (__builtin_va_arg(ap,long long) != 3LL)
    printf ("fail %d.%d\n", 2844, 2), ++fails;

  p = &a2844[2];
  arg = __builtin_va_arg(ap,union S2844);  /* This would fail.  */
  if (p->a != arg.a)
    printf ("fail %d.%d\n", 2844, 3), ++fails;

  arg = __builtin_va_arg(ap,union S2844);  /* This would fail.  */
  if (p->a != arg.a)
    printf ("fail %d.%d\n", 2844, 4), ++fails;

  __builtin_va_end(ap);
}

int main (void) {
  int i, j;
  memset (&s2844, '\0', sizeof (s2844));
  memset (a2844, '\0', sizeof (a2844));
  s2844.a = 799U;
  a2844[2].a = 586U;
  check2844va (1, 1.0, s2844, 2LL, a2844[2], a2844[2]);
  exit (fails != 0);
}

This is a tough case mixing bitfields and alignment, where
aarch64_gimplify_va_arg_expr did not follow the exact same rule as
aarch64_layout_arg. When the va_arg parameter uses only one general
register, we do not want to introduce double-word alignment.

[Bug target/105549] aarch64: Wrong va_arg alignment handling

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P2
           Assignee|unassigned at gcc dot gnu.org      |clyon at gcc dot gnu.org
           Severity|major                       |normal
             Target|                            |aarch64

[Bug target/105549] aarch64: Wrong va_arg alignment handling

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |8.5.0, 9.4.1

--- Comment #1 from Christophe Lyon <clyon at gcc dot gnu.org> ---
It already failed with gcc-9. With gcc-8 only the 2nd check failed.

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|aarch64: Wrong va_arg       |aarch64: Wrong va_arg
                   |alignment handling          |alignment handling with
                   |                            |packed bitfields and
                   |                            |alignment
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-10-28
             Status|UNCONFIRMED                 |ASSIGNED
           Keywords|                            |ABI

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Christophe Lyon <clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:3df1a115be22caeab3ffe7afb12e71adb54ff132

commit r13-5123-g3df1a115be22caeab3ffe7afb12e71adb54ff132
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Tue Jun 14 21:08:33 2022 +0000

    aarch64: fix warning emission for ABI break since GCC 9.1

    While looking at PR 105549, which is about fixing the ABI break
    introduced in GCC 9.1 in parameter alignment with bit-fields, we
    noticed that the GCC 9.1 warning is not emitted in all the cases where
    it should be.  This patch fixes that and the next patch in the series
    fixes the GCC 9.1 break.

    We split this into two patches since patch #2 introduces a new ABI
    break starting with GCC 13.1.  This way, patch #1 can be back-ported
    to release branches if needed to fix the GCC 9.1 warning issue.

    The main idea is to add a new global boolean that indicates whether
    we're expanding the start of a function, so that aarch64_layout_arg
    can emit warnings for callees as well as callers.  This removes the
    need for aarch64_function_arg_boundary to warn (with its incomplete
    information).  However, in the first patch there are still cases where
    we emit warnings were we should not; this is fixed in patch #2 where
    we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly.

    The fix in aarch64_function_arg_boundary (replacing & with &&) looks
    like an oversight of a previous commit in this area which changed
    'abi_break' from a boolean to an integer.

    We also take the opportunity to fix the comment above
    aarch64_function_arg_alignment since the value of the abi_break
    parameter was changed in a previous commit, no longer matching the
    description.

    2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
                Richard Sandiford  <richard.sandiford@arm.com>

    gcc/ChangeLog:

            * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix
            comment.
            (aarch64_layout_arg): Factorize warning conditions.
            (aarch64_function_arg_boundary): Fix typo.
            * function.cc (currently_expanding_function_start): New variable.
            (expand_function_start): Handle
            currently_expanding_function_start.
            * function.h (currently_expanding_function_start): Declare.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning.h: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning.h: New test.

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Christophe Lyon <clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:6610daa1cfb75b72500c22ae97988ec2a48b85c6

commit r13-5124-g6610daa1cfb75b72500c22ae97988ec2a48b85c6
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Fri Nov 25 13:35:11 2022 +0000

    aarch64: Fix bit-field alignment in param passing [PR105549]

    While working on enabling DFP for AArch64, I noticed new failures in
    gcc.dg/compat/struct-layout-1.exp (t028) which were not actually
    caused by DFP types handling. These tests are generated during 'make
    check' and enabling DFP made generation different (not sure if new
    non-DFP tests are generated, or if existing ones are generated
    differently, the tests in question are huge and difficult to compare).

    Anyway, I reduced the problem to what I attach at the end of the new
    gcc.target/aarch64/aapcs64/va_arg-17.c test and rewrote it in the same
    scheme as other va_arg* AArch64 tests.  Richard Sandiford further
    reduced this to a non-vararg function, added as a second testcase.

    This is a tough case mixing bit-fields and alignment, where
    aarch64_function_arg_alignment did not follow what its descriptive
    comment says: we want to use the natural alignment of the bit-field
    type only if the user didn't reduce the alignment for the bit-field
    itself.

    The patch also adds a comment and assert that would help someone who
    has to look at this area again.

    The fix would be very small, except that this introduces a new ABI
    break, and we have to warn about that.  Since this actually fixes a
    problem introduced in GCC 9.1, we keep the old computation to detect
    when we now behave differently.

    This patch adds two new tests (va_arg-17.c and
    pr105549.c). va_arg-17.c contains the reduced offending testcase from
    struct-layout-1.exp for reference.  We update some tests introduced by
    the previous patch, where parameters with bit-fields and packed
    attribute now emit a different warning.

    2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
                Richard Sandiford  <richard.sandiford@arm.com>

            gcc/
            PR target/105549
            * config/aarch64/aarch64.cc (aarch64_function_arg_alignment):
            Check DECL_PACKED for bitfield.
            (aarch64_layout_arg): Warn when parameter passing ABI changes.
            (aarch64_function_arg_boundary): Do not warn here.
            (aarch64_gimplify_va_arg_expr): Warn when parameter passing ABI
            changes.

            gcc/testsuite/
            PR target/105549
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Update.
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c:
Update.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Update.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c:
Update.
            * gcc.target/aarch64/aapcs64/va_arg-17.c: New test.
            * gcc.target/aarch64/pr105549.c: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Update.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C:
Update.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Update.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C:
Update.

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[clyon at gcc dot gnu.org]

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

--- Comment #5 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Fixed on trunk

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

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

commit r10-11174-gfea013065580084578b1b78d8f727c0323f2a9a0
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Tue Jan 24 09:46:30 2023 +0000

    [PATCH] aarch64: fix warning emission for ABI break since GCC 9.1

    While looking at PR 105549, which is about fixing the ABI break
    introduced in GCC 9.1 in parameter alignment with bit-fields, we
    noticed that the GCC 9.1 warning is not emitted in all the cases where
    it should be.  This patch fixes that and the next patch in the series
    fixes the GCC 9.1 break.

    We split this into two patches since patch #2 introduces a new ABI
    break starting with GCC 13.1.  This way, patch #1 can be back-ported
    to release branches if needed to fix the GCC 9.1 warning issue.

    The main idea is to add a new global boolean that indicates whether
    we're expanding the start of a function, so that aarch64_layout_arg
    can emit warnings for callees as well as callers.  This removes the
    need for aarch64_function_arg_boundary to warn (with its incomplete
    information).  However, in the first patch there are still cases where
    we emit warnings were we should not; this is fixed in patch #2 where
    we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly.

    The fix in aarch64_function_arg_boundary (replacing & with &&) looks
    like an oversight of a previous commit in this area which changed
    'abi_break' from a boolean to an integer.

    We also take the opportunity to fix the comment above
    aarch64_function_arg_alignment since the value of the abi_break
    parameter was changed in a previous commit, no longer matching the
    description.

    2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
                Richard Sandiford  <richard.sandiford@arm.com>

    gcc/ChangeLog:

            * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Fix
            comment.
            (aarch64_layout_arg): Factorize warning conditions.
            (aarch64_function_arg_boundary): Fix typo.
            * function.c (currently_expanding_function_start): New variable.
            (expand_function_start): Handle
            currently_expanding_function_start.
            * function.h (currently_expanding_function_start): Declare.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning.h: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning.h: New test.

    (cherry picked from commit 3df1a115be22caeab3ffe7afb12e71adb54ff132)

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

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

commit r11-10485-gd7f8b1c54bd054d214a73d4b534d599365f8658b
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Tue Jun 14 21:08:33 2022 +0000

    aarch64: fix warning emission for ABI break since GCC 9.1

    While looking at PR 105549, which is about fixing the ABI break
    introduced in GCC 9.1 in parameter alignment with bit-fields, we
    noticed that the GCC 9.1 warning is not emitted in all the cases where
    it should be.  This patch fixes that and the next patch in the series
    fixes the GCC 9.1 break.

    We split this into two patches since patch #2 introduces a new ABI
    break starting with GCC 13.1.  This way, patch #1 can be back-ported
    to release branches if needed to fix the GCC 9.1 warning issue.

    The main idea is to add a new global boolean that indicates whether
    we're expanding the start of a function, so that aarch64_layout_arg
    can emit warnings for callees as well as callers.  This removes the
    need for aarch64_function_arg_boundary to warn (with its incomplete
    information).  However, in the first patch there are still cases where
    we emit warnings were we should not; this is fixed in patch #2 where
    we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly.

    The fix in aarch64_function_arg_boundary (replacing & with &&) looks
    like an oversight of a previous commit in this area which changed
    'abi_break' from a boolean to an integer.

    We also take the opportunity to fix the comment above
    aarch64_function_arg_alignment since the value of the abi_break
    parameter was changed in a previous commit, no longer matching the
    description.

    2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
                Richard Sandiford  <richard.sandiford@arm.com>

    gcc/ChangeLog:

            * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Fix
            comment.
            (aarch64_layout_arg): Factorize warning conditions.
            (aarch64_function_arg_boundary): Fix typo.
            * function.c (currently_expanding_function_start): New variable.
            (expand_function_start): Handle
            currently_expanding_function_start.
            * function.h (currently_expanding_function_start): Declare.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning.h: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning.h: New test.

    (cherry picked from commit 3df1a115be22caeab3ffe7afb12e71adb54ff132)

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

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

commit r12-9067-gf593bfa059fbd2c145d8dd2bae4860959e9e55fe
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Tue Jun 14 21:08:33 2022 +0000

    aarch64: fix warning emission for ABI break since GCC 9.1

    While looking at PR 105549, which is about fixing the ABI break
    introduced in GCC 9.1 in parameter alignment with bit-fields, we
    noticed that the GCC 9.1 warning is not emitted in all the cases where
    it should be.  This patch fixes that and the next patch in the series
    fixes the GCC 9.1 break.

    We split this into two patches since patch #2 introduces a new ABI
    break starting with GCC 13.1.  This way, patch #1 can be back-ported
    to release branches if needed to fix the GCC 9.1 warning issue.

    The main idea is to add a new global boolean that indicates whether
    we're expanding the start of a function, so that aarch64_layout_arg
    can emit warnings for callees as well as callers.  This removes the
    need for aarch64_function_arg_boundary to warn (with its incomplete
    information).  However, in the first patch there are still cases where
    we emit warnings were we should not; this is fixed in patch #2 where
    we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly.

    The fix in aarch64_function_arg_boundary (replacing & with &&) looks
    like an oversight of a previous commit in this area which changed
    'abi_break' from a boolean to an integer.

    We also take the opportunity to fix the comment above
    aarch64_function_arg_alignment since the value of the abi_break
    parameter was changed in a previous commit, no longer matching the
    description.

    2022-11-28  Christophe Lyon  <christophe.lyon@arm.com>
                Richard Sandiford  <richard.sandiford@arm.com>

    gcc/ChangeLog:

            * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix
            comment.
            (aarch64_layout_arg): Factorize warning conditions.
            (aarch64_function_arg_boundary): Fix typo.
            * function.cc (currently_expanding_function_start): New variable.
            (expand_function_start): Handle
            currently_expanding_function_start.
            * function.h (currently_expanding_function_start): Declare.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New
            test.
            * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test.
            * gcc.target/aarch64/bitfield-abi-warning.h: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New
            test.
            * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test.
            * g++.target/aarch64/bitfield-abi-warning.h: New test.

    (cherry picked from commit 3df1a115be22caeab3ffe7afb12e71adb54ff132)

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #9 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Fixed.

[Bug target/105549] aarch64: Wrong va_arg alignment handling with packed bitfields and alignment

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0