public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17
@ 2020-03-28 15:54 redi at gcc dot gnu.org
  2020-03-28 15:55 ` [Bug target/94383] " redi at gcc dot gnu.org
                   ` (20 more replies)
  0 siblings, 21 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-03-28 15:54 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 94383
           Summary: [8/9/10 Regression] class with empty base passed
                    incorrectly with -std=c++17
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Keywords: ABI
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64-*-linux

This code is miscompiled on aarch64 with -std=c++17, apparently due to the
empty base class:

struct base { };
struct pair : base
{
  float first;
  float second;
  pair(float f, float s) : first(f), second(s) { }
};

void f(pair);

int main()
{
  f({3.14, 666});
}

It is wrong with any optimization level, but with -std=gnu++14 -O1 we get:

main:
        stp     x29, x30, [sp, -16]!
        mov     x29, sp
        mov     x0, 0
        mov     x1, 62915
        movk    x1, 0x4048, lsl 16
        bfi     x0, x1, 0, 32
        mov     x1, 32768
        movk    x1, 0x4426, lsl 16
        bfi     x0, x1, 32, 32
        lsr     w1, w0, 0
        fmov    d0, x0
        ushr    d1, d0, 32
        fmov    s0, w1
        bl      f(pair)
        mov     w0, 0
        ldp     x29, x30, [sp], 16
        ret

With -std=gnu++17 -O1 we get:

main:
        stp     x29, x30, [sp, -16]!
        mov     x29, sp
        mov     x0, 0
        mov     x1, 62915
        movk    x1, 0x4048, lsl 16
        bfi     x0, x1, 0, 32
        mov     x1, 32768
        movk    x1, 0x4426, lsl 16
        bfi     x0, x1, 32, 32
        bl      f(pair)
        mov     w0, 0
        ldp     x29, x30, [sp], 16
        ret

If the translation unit containing the called function is:

struct base { };
struct pair : base
{
  float first;
  float second;
  pair(float f, float s) : first(f), second(s) { }
};

void f(pair lr)
{
  __builtin_printf("%f %f\n", lr.first, lr.second);
}

and the caller is compiled with -std=gnu++14 and the callee is compiled with
-std=gnu++17 then the callee prints garabage:

-13874.335938 0.000000

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
@ 2020-03-28 15:55 ` redi at gcc dot gnu.org
  2020-03-28 16:01 ` redi at gcc dot gnu.org
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-03-28 15:55 UTC (permalink / raw)
  To: gcc-bugs

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |6.4.0
      Known to fail|                            |7.5.0, 8.3.0, 9.2.1
   Last reconfirmed|                            |2020-03-28
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I haven't actually tested this with gcc-10 yet but I can't find an existing
bug, so I assume it's also still present on master.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
  2020-03-28 15:55 ` [Bug target/94383] " redi at gcc dot gnu.org
@ 2020-03-28 16:01 ` redi at gcc dot gnu.org
  2020-03-28 17:37 ` [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64 redi at gcc dot gnu.org
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-03-28 16:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
https://godbolt.org/z/BnTEsn is an example with both functions in the same
translation unit, showing the generated code is different for both caller and
callee. If the caller and callee are not in the same TU the differences produce
garbage.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
  2020-03-28 15:55 ` [Bug target/94383] " redi at gcc dot gnu.org
  2020-03-28 16:01 ` redi at gcc dot gnu.org
@ 2020-03-28 17:37 ` redi at gcc dot gnu.org
  2020-03-28 17:39 ` redi at gcc dot gnu.org
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-03-28 17:37 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Is the difference maybe related to the empty field that is added for c++17
mode, mentioned in Bug 89358 comment 12?

Is the aarch64 back end not ignoring that field?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2020-03-28 17:37 ` [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64 redi at gcc dot gnu.org
@ 2020-03-28 17:39 ` redi at gcc dot gnu.org
  2020-03-28 18:00 ` mpolacek at gcc dot gnu.org
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-03-28 17:39 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
That would be consistent with the new field being introduced in gcc-7 (by
r241187).

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2020-03-28 17:39 ` redi at gcc dot gnu.org
@ 2020-03-28 18:00 ` mpolacek at gcc dot gnu.org
  2020-03-28 19:26 ` pinskia at gcc dot gnu.org
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2020-03-28 18:00 UTC (permalink / raw)
  To: gcc-bugs

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Yep, bisected it to r241187.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2020-03-28 18:00 ` mpolacek at gcc dot gnu.org
@ 2020-03-28 19:26 ` pinskia at gcc dot gnu.org
  2020-03-30  8:13 ` rguenth at gcc dot gnu.org
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: pinskia at gcc dot gnu.org @ 2020-03-28 19:26 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |8.5
           Severity|normal                      |blocker

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2020-03-28 19:26 ` pinskia at gcc dot gnu.org
@ 2020-03-30  8:13 ` rguenth at gcc dot gnu.org
  2020-03-30 14:04 ` jakub at gcc dot gnu.org
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-03-30  8:13 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2020-03-30  8:13 ` rguenth at gcc dot gnu.org
@ 2020-03-30 14:04 ` jakub at gcc dot gnu.org
  2020-03-30 14:38 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-30 14:04 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |ktkachov at gcc dot gnu.org,
                   |                            |rearnsha at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Seems it is in aapcs_vfp_sub_candidate, which for C++17 recurses on the
DECL_FIELD_IS_BASE FIELD_DECL and because
16039           /* There must be no padding.  */
16040           if (maybe_ne (wi::to_poly_wide (TYPE_SIZE (type)),
16041                         count * GET_MODE_BITSIZE (*modep)))
16042             return -1;
check fails, as the type is base, with no elts (count == 0) but TYPE_SIZE of
BITS_PER_UNIT, it returns -1 and thus returns it for the whole pair class too.
The question is what exactly the AAPCS says.  And if we want to ignore the
empty base fields, the question is how to exactly recognize them in the
backend, because DECL_FIELD_IS_BASE is a FE macro, and I doubt one can use it
e.g. in LTO.
So, shall aapcs_vfp_sub_candidate ignore DECL_ARTIFICIAL FIELD_DECLs that have
DECL_SIZE (field) && integer_zerop (DECL_SIZE (field)) ?  Something else?
What do other compilers do here?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2020-03-30 14:04 ` jakub at gcc dot gnu.org
@ 2020-03-30 14:38 ` jakub at gcc dot gnu.org
  2020-04-15 15:38 ` jakub at gcc dot gnu.org
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-30 14:38 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
--- gcc/config/aarch64/aarch64.c.jj     2020-03-18 12:51:41.051640609 +0100
+++ gcc/config/aarch64/aarch64.c        2020-03-30 16:28:29.133717645 +0200
@@ -16030,6 +16030,16 @@ aapcs_vfp_sub_candidate (const_tree type
            if (TREE_CODE (field) != FIELD_DECL)
              continue;

+           /* Ignore C++17 empty base fields, while their type indicates
+              they do contain padding, they have zero size and thus don't
+              contain any padding.  */
+           if (DECL_ARTIFICIAL (field)
+               && DECL_NAME (field) == NULL_TREE
+               && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
+               && DECL_SIZE (field)
+               && integer_zerop (DECL_SIZE (field)))
+             continue;
+
            sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep);
            if (sub_count < 0)
              return -1;
fixes that.  Though, guess there might need to be a -Wpsabi warning and that
would likely need propagate to callers whether such field has been seen and if
it has been seen, let the code retry in a mode where it wouldn't ignore those,
compare the result and if it is different, warn.
Will defer to aarch64 maintainers.

And, guess it would be nice to test other targets for similar issues.
Do we want e.g. to adjust struct-layout-1_generate.c, so that it only tests
structures with such an empty base and just check value passing of such classes
(both caller and callee at various spots in argument list) or returning,
compile everything in C++14 and C++17 and compare assembly, verify that it
fails on aarch64 and then look for others?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2020-03-30 14:38 ` jakub at gcc dot gnu.org
@ 2020-04-15 15:38 ` jakub at gcc dot gnu.org
  2020-04-16 14:08 ` matmal01 at gcc dot gnu.org
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-15 15:38 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'd like to ping this, it would be nice to at least decide if this should be
handled for GCC10 or postponed to GCC11 only.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2020-04-15 15:38 ` jakub at gcc dot gnu.org
@ 2020-04-16 14:08 ` matmal01 at gcc dot gnu.org
  2020-04-16 14:14 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: matmal01 at gcc dot gnu.org @ 2020-04-16 14:08 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Matthew Malcomson <matmal01 at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> I'd like to ping this, it would be nice to at least decide if this should be
> handled for GCC10 or postponed to GCC11 only.

Hi Jakub -- I'm taking a look at this at the moment, so I'm hoping I can get it
done for GCC10.

So far I've only double checked what the AAPCS64 says and confirmed that we're
producing the correct code for gnu++14 and not gnu++17 (the base class is empty
and a language type that occupies zero bytes has no mapping on the ABI level,
hence that `pair` structure is an HFA and should be passed in the vector
registers).

I'm just about to start looking at changes & testing.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2020-04-16 14:08 ` matmal01 at gcc dot gnu.org
@ 2020-04-16 14:14 ` jakub at gcc dot gnu.org
  2020-04-20 18:21 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-16 14:14 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2020-04-16 14:14 ` jakub at gcc dot gnu.org
@ 2020-04-20 18:21 ` jakub at gcc dot gnu.org
  2020-04-21 15:10 ` cvs-commit at gcc dot gnu.org
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-20 18:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48317
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48317&action=edit
gcc10-pr94383.patch

Testsuite coverage.  This passes make -j32 -k check-c++-all
RUNTESTFLAGS=struct-layout-1.exp
on x86_64-linux, but it would be nice to know if it does FAIL on aarch64-linux
and/or arm*-linux-gnueabi.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2020-04-20 18:21 ` jakub at gcc dot gnu.org
@ 2020-04-21 15:10 ` cvs-commit at gcc dot gnu.org
  2020-04-22 12:10 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-21 15:10 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:619602346aed9dae3f338d9f18767414446adf78

commit r10-7850-g619602346aed9dae3f338d9f18767414446adf78
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 21 17:08:10 2020 +0200

    testsuite: Extend C++ struct-layout-1.exp testing to test C++14 vs. C++17
interoperability of structs with empty bases [PR94383]

    Jonathan reported an ABI incompatibility between C++14 and C++17 in
    passing some aggregates with empty bases on aarch64 (and apparently on arm
    too).

    The following patch adds 3000 (by default) tests for such interoperability,
    using the struct-layout-1* framework.  The current 3000 tests are generated
    as is (so unchanged from previous ones), and afterwards there is another
set
    of 3000 ones, where always one of the tNNN_x.C and tNNN_y.C tests get added
    -std=c++14 -DCXX14_VS_CXX17 and another one -std=c++17 -DCXX14_VS_CXX17
    options (which one which is chosen pseudo-randomly), which causes the
    structs to have an empty base.

    I haven't added (yet) checks if the alternate compiler does support these
    options (I think that can be done incrementally), so for now this testing
is
    done only if the alternate compiler is not used.

    I had to fix a bug in the flexible array handling, because while we were
    lucky in the 3000 generated tests not to have toplevel fields after field
    with flexible array members, in the next 3000 we aren't lucky anymore.
    But even with that change, diff -upr between old and new
    testsuite/g++/g++.dg/g++.dg-struct-layout-1/ doesn't show any differences
    except for the ^Only in... messages for the new tests in there.

    Bootstrapped/regtested on x86_64-linux and i686-linux and additionally
    tested on aarch64-linux, where
    FAIL: tmpdir-g++.dg-struct-layout-1/t032
cp_compat_x_tst.o-cp_compat_y_tst.o execute
    FAIL: tmpdir-g++.dg-struct-layout-1/t056
cp_compat_x_tst.o-cp_compat_y_tst.o execute
    FAIL: tmpdir-g++.dg-struct-layout-1/t057
cp_compat_x_tst.o-cp_compat_y_tst.o execute
    FAIL: tmpdir-g++.dg-struct-layout-1/t058
cp_compat_x_tst.o-cp_compat_y_tst.o execute
    FAIL: tmpdir-g++.dg-struct-layout-1/t059
cp_compat_x_tst.o-cp_compat_y_tst.o execute
    because of the backend bug, and with that bug fixed it succeeds.
    Matthew has kindly tested it also on aarch64-linux and arm*-*.

    The primary goal of the patch is catch if some targets other than aarch64
or
    arm aren't affected too.

    2020-04-21  Jakub Jelinek  <jakub@redhat.com>

            PR c++/94383
            * g++.dg/compat/struct-layout-1.exp: If !$use_alt, add -c to
generator
            args.
            * g++.dg/compat/struct-layout-1_generate.c (dg_options): Add
another
            %s to the start of dg-options arg.
            (cxx14_vs_cxx17, do_cxx14_vs_cxx17): New variables.
            (switchfiles): If cxx14_vs_cxx17, prepend -std=c++14
-DCXX14_VS_CXX17
            or -std=c++17 -DCXX17_VS_CXX14 - randomly - to dg-options.
            (output): Don't append further fields once one with flexible array
            member is added.
            (generate_random_tests): Don't use toplevel unions if
cxx14_vs_cxx17.
            (main): If -c, emit second set of tests for -std=c++14 vs.
-std=c++17
            testing.
            * g++.dg/compat/struct-layout-1_x1.h (empty_base): New type.
            (EMPTY_BASE): Define.
            (TX): Use EMPTY_BASE.
            * g++.dg/compat/struct-layout-1_y1.h (empty_base): New type.
            (EMPTY_BASE): Define.
            (TX): Use EMPTY_BASE.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2020-04-21 15:10 ` cvs-commit at gcc dot gnu.org
@ 2020-04-22 12:10 ` jakub at gcc dot gnu.org
  2020-04-22 14:47 ` cvs-commit at gcc dot gnu.org
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-22 12:10 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P1
   Target Milestone|8.5                         |10.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2020-04-22 12:10 ` jakub at gcc dot gnu.org
@ 2020-04-22 14:47 ` cvs-commit at gcc dot gnu.org
  2020-04-23 14:34 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-22 14:47 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r10-7881-gc3a34659036b55fa36a5355fdd67133f427eeb2d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 22 16:44:42 2020 +0200

    calls: Introduce cxx17_empty_base_field_p [PR94383]

    As multiple targets are affected apparently, I believe at least
    aarch64, arm, powerpc64le, s390{,x} and ia64,
    I think we should have a middle-end predicate for this, so that if we need
    to tweak it, we can do it in one spot.

    2020-04-22  Jakub Jelinek  <jakub@redhat.com>

            PR target/94383
            * calls.h (cxx17_empty_base_field_p): Declare.
            * calls.c (cxx17_empty_base_field_p): Define.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2020-04-22 14:47 ` cvs-commit at gcc dot gnu.org
@ 2020-04-23 14:34 ` cvs-commit at gcc dot gnu.org
  2020-04-23 14:38 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-23 14:34 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Matthew Malcomson <matmal01@gcc.gnu.org>:

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

commit r10-7914-ge73a32d6d47ec7c5fb5a5fe7eb896c0e1258ea68
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Thu Apr 23 15:33:55 2020 +0100

    [AArch64] (PR94383) Avoid C++17 empty base field checking for HVA/HFA

    In C++17, an empty class deriving from an empty base is not an
    aggregate, while in C++14 it is.  In order to implement this, GCC adds
    an artificial field to such classes.

    This artificial field has no mapping to Fundamental Data Types in the
    AArch64 PCS ABI and hence should not count towards determining whether an
    object can be passed using the vector registers as per section
    "6.4.2 Parameter Passing Rules" in the AArch64 PCS.
   
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#the-base-procedure-call-standard

    This patch avoids counting this artificial field in
    aapcs_vfp_sub_candidate, and hence calculates whether such objects
    should be passed in vector registers in the same manner as C++14 (where
    the artificial field does not exist).

    Before this change, the test below would pass the arguments to `f` in
    general registers.  After this change, the test passes the arguments to
    `f` using the vector registers.

    The new behaviour matches the behaviour of `armclang`, and also matches
    the behaviour when run with `-std=gnu++14`.

    > gcc -std=gnu++17 test.cpp

    ``` test.cpp
    struct base {};

    struct pair : base
    {
      float first;
      float second;
      pair (float f, float s) : first(f), second(s) {}
    };

    void f (pair);
    int main()
    {
      f({3.14, 666});
      return 1;
    }
    ```

    We add a `-Wpsabi` warning to catch cases where this fix has changed the
ABI for
    some functions.  Unfortunately this warning is not emitted twice for
multiple
    calls to the same function, but I feel this is not much of a problem and
can be
    fixed later if needs be.

    (i.e. if `main` called `f` twice in a row we only emit a diagnostic for the
    first).

    Testing:
        Bootstrap and regression test on aarch64-linux.
        All struct-layout-1 tests now pass.

    gcc/ChangeLog:

    2020-04-23  Matthew Malcomson  <matthew.malcomson@arm.com>
                Jakub Jelinek  <jakub@redhat.com>

            PR target/94383
            * config/aarch64/aarch64.c (aapcs_vfp_sub_candidate): Account for
C++17
            empty base class artificial fields.
            (aarch64_vfp_is_call_or_return_candidate): Warn when ABI PCS
decision is
            different after this fix.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2020-04-23 14:34 ` cvs-commit at gcc dot gnu.org
@ 2020-04-23 14:38 ` jakub at gcc dot gnu.org
  2020-04-23 16:54 ` redi at gcc dot gnu.org
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 22+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-23 14:38 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2020-04-23 14:38 ` jakub at gcc dot gnu.org
@ 2020-04-23 16:54 ` redi at gcc dot gnu.org
  2020-04-24 17:15 ` cvs-commit at gcc dot gnu.org
  2021-11-05 23:18 ` timturnerc at yahoo dot com
  20 siblings, 0 replies; 22+ messages in thread
From: redi at gcc dot gnu.org @ 2020-04-23 16:54 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to CVS Commits from comment #14)
>     [AArch64] (PR94383) Avoid C++17 empty base field checking for HVA/HFA
>     
>     In C++17, an empty class deriving from an empty base is not an
>     aggregate, while in C++14 it is.

FWIW that's backwards.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (18 preceding siblings ...)
  2020-04-23 16:54 ` redi at gcc dot gnu.org
@ 2020-04-24 17:15 ` cvs-commit at gcc dot gnu.org
  2021-11-05 23:18 ` timturnerc at yahoo dot com
  20 siblings, 0 replies; 22+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-24 17:15 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9407f0c32b215d55d3474a234b0043bddc185b1c

commit r10-7948-g9407f0c32b215d55d3474a234b0043bddc185b1c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 24 19:14:27 2020 +0200

    testsuite: C++14 vs. C++17 struct-layout-1.exp testing with
ALT_CXX_UNDER_TEST [PR94383]

    > I haven't added (yet) checks if the alternate compiler does support these
    > options (I think that can be done incrementally), so for now this testing
is
    > done only if the alternate compiler is not used.

    This patch does that, so now when testing against not too old compiler
    it can do the -std=c++14 vs. -std=c++17 testing also between under test and
    alt compilers.

    2020-04-24  Jakub Jelinek  <jakub@redhat.com>

            PR c++/94383
            * g++.dg/compat/struct-layout-1.exp: Use the -std=c++14 vs.
-std=c++17
            ABI compatibility testing even with ALT_CXX_UNDER_TEST, as long as
            that compiler accepts -std=c++14 and -std=c++17 options.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64
  2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
                   ` (19 preceding siblings ...)
  2020-04-24 17:15 ` cvs-commit at gcc dot gnu.org
@ 2021-11-05 23:18 ` timturnerc at yahoo dot com
  20 siblings, 0 replies; 22+ messages in thread
From: timturnerc at yahoo dot com @ 2021-11-05 23:18 UTC (permalink / raw)
  To: gcc-bugs

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

Tim Turner <timturnerc at yahoo dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |timturnerc at yahoo dot com

--- Comment #18 from Tim Turner <timturnerc at yahoo dot com> ---
$ cat test.c
        struct foo {
            int len; https://www.webb-dev.co.uk/category/computers/
            int items[];
        };

        struct foo *p;
    http://www.compilatori.com/category/technology/
        int main() {
            return 0;
        }
        $ gcc test.c -g -O0 -o test
http://www.acpirateradio.co.uk/category/computers/
        $ ./gdb -q -nx --data-directory=data-directory ./test -ex 'python
gdb.parse_and_eval("p").type.target()["items"].type.range()'
        Reading symbols from ./test...
http://www-look-4.com/category/computers/
        /home/simark/src/binutils-gdb/gdb/gdbtypes.h:435: internal-error:
LONGEST dynamic_prop::const_val() const: Assertion `m_kind == PROP_CONST'
failed. http://www.mconstantine.co.uk/category/services/ 
        A problem internal to GDB has been detected,
        further debugging may prove unreliable.
        Quit this debugging session? (y or n)
http://www.logoarts.co.uk/category/computers/

    This is because the Python code (typy_range) blindly reads the high
    bound of the type of `items` as a constant value.  Since it is a
http://www.iu-bloomington.com/category/computers/
    flexible array member, it has no high bound, the property is undefined.
    Since commit 8c2e4e0689 https://komiya-dental.com/category/computers/
("gdb: add accessors to struct dynamic_prop"),
    the getters check that you are not
http://www.go-mk-websites.co.uk/category/services/ getting a property value of
the wrong
    kind, so this causes a failed assertion.
http://www.slipstone.co.uk/category/computers/

    Fix it by checking if the property is indeed a constant value before
http://embermanchester.uk/category/computers/
    accessing it as such.  Otherwise, use 0.  This restores the previous GDB
http://fishingnewsletters.co.uk/category/services/
    behavior: because the structure was zero-initialized,
http://connstr.net/category/computers/  this is what was
    returned before.  But now this behavior is explicit and not accidental. But
now this behavior is explicit and not accidental. But now this behavior is
explicit and not  http://the-hunters.org/technology/new-robot/ accidental. But
now this behavior is explicit and not accidental. But now this behavior is
explicit and is explicit and not accidental.
    http://joerg.li/category/computers/
    Add a test, gdb.python/flexible-array-member.exp, that is derived from
    gdb.base/flexible-array-member.exp.
http://www.jopspeech.com/category/computers/  It tests the same things, but
    through the Python API.  It also specifically tests getting the range
    from the various kinds http://www.wearelondonmade.com/category/computers/
of flexible array member types (AFAIK it wasn't
    possible to do the equivalent through the CLI).
https://waytowhatsnext.com/category/computers/

    gdb/ChangeLog:

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-11-05 23:18 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-28 15:54 [Bug target/94383] New: [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 redi at gcc dot gnu.org
2020-03-28 15:55 ` [Bug target/94383] " redi at gcc dot gnu.org
2020-03-28 16:01 ` redi at gcc dot gnu.org
2020-03-28 17:37 ` [Bug target/94383] [8/9/10 Regression] class with empty base passed incorrectly with -std=c++17 on aarch64 redi at gcc dot gnu.org
2020-03-28 17:39 ` redi at gcc dot gnu.org
2020-03-28 18:00 ` mpolacek at gcc dot gnu.org
2020-03-28 19:26 ` pinskia at gcc dot gnu.org
2020-03-30  8:13 ` rguenth at gcc dot gnu.org
2020-03-30 14:04 ` jakub at gcc dot gnu.org
2020-03-30 14:38 ` jakub at gcc dot gnu.org
2020-04-15 15:38 ` jakub at gcc dot gnu.org
2020-04-16 14:08 ` matmal01 at gcc dot gnu.org
2020-04-16 14:14 ` jakub at gcc dot gnu.org
2020-04-20 18:21 ` jakub at gcc dot gnu.org
2020-04-21 15:10 ` cvs-commit at gcc dot gnu.org
2020-04-22 12:10 ` jakub at gcc dot gnu.org
2020-04-22 14:47 ` cvs-commit at gcc dot gnu.org
2020-04-23 14:34 ` cvs-commit at gcc dot gnu.org
2020-04-23 14:38 ` jakub at gcc dot gnu.org
2020-04-23 16:54 ` redi at gcc dot gnu.org
2020-04-24 17:15 ` cvs-commit at gcc dot gnu.org
2021-11-05 23:18 ` timturnerc at yahoo dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).