[Bug c++/102295] New: ELF symbol sizes for variable-length objects are too small (C++)

[Bug c++/102295] New: ELF symbol sizes for variable-length objects are too small (C++)

[pinskia at gcc dot gnu.org]

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

            Bug ID: 102295
           Summary: ELF symbol sizes for variable-length objects are too
                    small (C++)
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

Like PR 63373 but this one is for C++ (since the C front-end has been fixed
since GCC 5), GCC's C++ front-end only started to accept this in GCC 6 too.

Take:

struct blah
{
        float foo;
        int i[];
};

struct blah b = { 42.0, { 1, 2, 3, 4, 0 } };
------ CUT ----
GCC's C++ front-end outputs:
        .size   b, 4

Which is wrong, it should be 24.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note the C front-end has add_flexible_array_elts_to_size in c/c-decl.c which
sets the DECL_SIZE to be correct.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-09-13
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51454
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51454&action=edit
gcc12-pr102295.patch

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, we have other issues, consider:
struct A
{
  float a;
  int b[];
};

int x[4];
struct A c = { 42.0f, { ++x[0], ++x[1], ++x[2], ++x[3] } };
When splitting the init into DECL_INITIAL constant initializer and runtime
initialization, the flexible array member initialization is moved completely
into runtime initialization and nothing remains in DECL_INITIAL from it.  For
initializers of fields other than flexible array members that is ok, but by
getting rid of the flex array member initializer the size emitted for the var
in assembly doesn't include the flexible array member at all.  That is worse
than having too small .size, in this case it means overwriting whatever is
after the variable.  Small .size actually isn't wrong-code...

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Note, we have other issues, consider:
> struct A
> {
>   float a;
>   int b[];
> };
> 
> int x[4];
> struct A c = { 42.0f, { ++x[0], ++x[1], ++x[2], ++x[3] } };

Right that is PR 88578 .

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:818c505188ff5cd8eb048eb0e614c4ef732225bd

commit r12-3526-g818c505188ff5cd8eb048eb0e614c4ef732225bd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 14 16:56:30 2021 +0200

    c++: Update DECL_*SIZE for objects with flexible array members with
initializers [PR102295]

    The C FE updates DECL_*SIZE for vars which have initializers for flexible
    array members for many years, but C++ FE kept DECL_*SIZE the same as the
    type size (i.e. as if there were zero elements in the flexible array
    member).  This results e.g. in ELF symbol sizes being too small.

    Note, if the flexible array member is initialized only with non-constant
    initializers, we have a worse bug that this patch doesn't solve, the
    splitting of initializers into constant and dynamic initialization removes
    the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    emitted when emitting those vars into assembly either and so the dynamic
    initialization clobbers other vars that may overlap the variable.
    I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    flexible array member in that case.

    2021-09-14  Jakub Jelinek  <jakub@redhat.com>

            PR c++/102295
            * decl.c (layout_var_decl): For aggregates ending with a flexible
            array member, add the size of the initializer for that member to
            DECL_SIZE and DECL_SIZE_UNIT.

            * g++.target/i386/pr102295.C: New test.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 12+ so far.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:e5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6

commit r12-3556-ge5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Sep 15 22:21:17 2021 +0200

    c++: Fix handling of decls with flexible array members initialized with
side-effects [PR88578]

    > > Note, if the flexible array member is initialized only with
non-constant
    > > initializers, we have a worse bug that this patch doesn't solve, the
    > > splitting of initializers into constant and dynamic initialization
removes
    > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    > > emitted when emitting those vars into assembly either and so the
dynamic
    > > initialization clobbers other vars that may overlap the variable.
    > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    > > flexible array member in that case.
    >
    > Makes sense.

    So, the following patch fixes that.

    The typeck2.c change makes sure we keep those CONSTRUCTORs around (although
    they should be empty because all their elts had side-effects/was
    non-constant if it was removed earlier), and the varasm.c change is to
avoid
    ICEs on those as well as ICEs on other flex array members that had some
    initializers without side-effects, but not on the last array element.

    The code was already asserting that the (index of the last elt in the
    CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the
local->val
    type, which is true for C flex arrays or for C++ if they don't have any
    side-effects or the last elt doesn't have side-effects, this patch changes
    that to assertion that the TYPE_SIZE_UNIT is greater than equal to the
    offset of the end of last element in the CONSTRUCTOR and uses
TYPE_SIZE_UNIT
    (int_size_in_bytes) in the code later on.

    2021-09-15  Jakub Jelinek  <jakub@redhat.com>

            PR c++/88578
            PR c++/102295
    gcc/
            * varasm.c (output_constructor_regular_field): Instead of assertion
            that array_size_for_constructor result is equal to size of
            TREE_TYPE (local->val) in bytes, assert that the type size is
greater
            or equal to array_size_for_constructor result and use type size as
            fieldsize.
    gcc/cp/
            * typeck2.c (split_nonconstant_init_1): Don't throw away empty
            initializers of flexible array members if they have non-zero type
            size.
    gcc/testsuite/
            * g++.dg/ext/flexary39.C: New test.
            * g++.dg/ext/flexary40.C: New test.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:6550198bd8467f435959fa25b69c217a6ef75c7a

commit r11-8998-g6550198bd8467f435959fa25b69c217a6ef75c7a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 14 16:56:30 2021 +0200

    c++: Update DECL_*SIZE for objects with flexible array members with
initializers [PR102295]

    The C FE updates DECL_*SIZE for vars which have initializers for flexible
    array members for many years, but C++ FE kept DECL_*SIZE the same as the
    type size (i.e. as if there were zero elements in the flexible array
    member).  This results e.g. in ELF symbol sizes being too small.

    Note, if the flexible array member is initialized only with non-constant
    initializers, we have a worse bug that this patch doesn't solve, the
    splitting of initializers into constant and dynamic initialization removes
    the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    emitted when emitting those vars into assembly either and so the dynamic
    initialization clobbers other vars that may overlap the variable.
    I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    flexible array member in that case.

    2021-09-14  Jakub Jelinek  <jakub@redhat.com>

            PR c++/102295
            * decl.c (layout_var_decl): For aggregates ending with a flexible
            array member, add the size of the initializer for that member to
            DECL_SIZE and DECL_SIZE_UNIT.

            * g++.target/i386/pr102295.C: New test.

    (cherry picked from commit 818c505188ff5cd8eb048eb0e614c4ef732225bd)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:352c0b2668a1e3ce28060ccc265d3427f19e175d

commit r11-8999-g352c0b2668a1e3ce28060ccc265d3427f19e175d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Sep 15 22:21:17 2021 +0200

    c++: Fix handling of decls with flexible array members initialized with
side-effects [PR88578]

    > > Note, if the flexible array member is initialized only with
non-constant
    > > initializers, we have a worse bug that this patch doesn't solve, the
    > > splitting of initializers into constant and dynamic initialization
removes
    > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    > > emitted when emitting those vars into assembly either and so the
dynamic
    > > initialization clobbers other vars that may overlap the variable.
    > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    > > flexible array member in that case.
    >
    > Makes sense.

    So, the following patch fixes that.

    The typeck2.c change makes sure we keep those CONSTRUCTORs around (although
    they should be empty because all their elts had side-effects/was
    non-constant if it was removed earlier), and the varasm.c change is to
avoid
    ICEs on those as well as ICEs on other flex array members that had some
    initializers without side-effects, but not on the last array element.

    The code was already asserting that the (index of the last elt in the
    CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the
local->val
    type, which is true for C flex arrays or for C++ if they don't have any
    side-effects or the last elt doesn't have side-effects, this patch changes
    that to assertion that the TYPE_SIZE_UNIT is greater than equal to the
    offset of the end of last element in the CONSTRUCTOR and uses
TYPE_SIZE_UNIT
    (int_size_in_bytes) in the code later on.

    2021-09-15  Jakub Jelinek  <jakub@redhat.com>

            PR c++/88578
            PR c++/102295
    gcc/
            * varasm.c (output_constructor_regular_field): Instead of assertion
            that array_size_for_constructor result is equal to size of
            TREE_TYPE (local->val) in bytes, assert that the type size is
greater
            or equal to array_size_for_constructor result and use type size as
            fieldsize.
    gcc/cp/
            * typeck2.c (split_nonconstant_init_1): Don't throw away empty
            initializers of flexible array members if they have non-zero type
            size.
    gcc/testsuite/
            * g++.dg/ext/flexary39.C: New test.
            * g++.dg/ext/flexary40.C: New test.

    (cherry picked from commit e5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed also for 11.3+.

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r10-10643-ge54c8a86166fff6357729a3cde1b1cd143985abe
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 14 16:56:30 2021 +0200

    c++: Update DECL_*SIZE for objects with flexible array members with
initializers [PR102295]

    The C FE updates DECL_*SIZE for vars which have initializers for flexible
    array members for many years, but C++ FE kept DECL_*SIZE the same as the
    type size (i.e. as if there were zero elements in the flexible array
    member).  This results e.g. in ELF symbol sizes being too small.

    Note, if the flexible array member is initialized only with non-constant
    initializers, we have a worse bug that this patch doesn't solve, the
    splitting of initializers into constant and dynamic initialization removes
    the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    emitted when emitting those vars into assembly either and so the dynamic
    initialization clobbers other vars that may overlap the variable.
    I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    flexible array member in that case.

    2021-09-14  Jakub Jelinek  <jakub@redhat.com>

            PR c++/102295
            * decl.c (layout_var_decl): For aggregates ending with a flexible
            array member, add the size of the initializer for that member to
            DECL_SIZE and DECL_SIZE_UNIT.

            * g++.target/i386/pr102295.C: New test.

    (cherry picked from commit 818c505188ff5cd8eb048eb0e614c4ef732225bd)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:96e3e3b1fc1c704d82af000f529b1ce88c66683a

commit r10-10644-g96e3e3b1fc1c704d82af000f529b1ce88c66683a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Sep 15 22:21:17 2021 +0200

    c++: Fix handling of decls with flexible array members initialized with
side-effects [PR88578]

    > > Note, if the flexible array member is initialized only with
non-constant
    > > initializers, we have a worse bug that this patch doesn't solve, the
    > > splitting of initializers into constant and dynamic initialization
removes
    > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    > > emitted when emitting those vars into assembly either and so the
dynamic
    > > initialization clobbers other vars that may overlap the variable.
    > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    > > flexible array member in that case.
    >
    > Makes sense.

    So, the following patch fixes that.

    The typeck2.c change makes sure we keep those CONSTRUCTORs around (although
    they should be empty because all their elts had side-effects/was
    non-constant if it was removed earlier), and the varasm.c change is to
avoid
    ICEs on those as well as ICEs on other flex array members that had some
    initializers without side-effects, but not on the last array element.

    The code was already asserting that the (index of the last elt in the
    CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the
local->val
    type, which is true for C flex arrays or for C++ if they don't have any
    side-effects or the last elt doesn't have side-effects, this patch changes
    that to assertion that the TYPE_SIZE_UNIT is greater than equal to the
    offset of the end of last element in the CONSTRUCTOR and uses
TYPE_SIZE_UNIT
    (int_size_in_bytes) in the code later on.

    2021-09-15  Jakub Jelinek  <jakub@redhat.com>

            PR c++/88578
            PR c++/102295
    gcc/
            * varasm.c (output_constructor_regular_field): Instead of assertion
            that array_size_for_constructor result is equal to size of
            TREE_TYPE (local->val) in bytes, assert that the type size is
greater
            or equal to array_size_for_constructor result and use type size as
            fieldsize.
    gcc/cp/
            * typeck2.c (split_nonconstant_init_1): Don't throw away empty
            initializers of flexible array members if they have non-zero type
            size.
    gcc/testsuite/
            * g++.dg/ext/flexary39.C: New test.
            * g++.dg/ext/flexary40.C: New test.

    (cherry picked from commit e5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r9-10100-ge13a79211a06c54e67245c664112c15f8bf3de7a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Sep 14 16:56:30 2021 +0200

    c++: Update DECL_*SIZE for objects with flexible array members with
initializers [PR102295]

    The C FE updates DECL_*SIZE for vars which have initializers for flexible
    array members for many years, but C++ FE kept DECL_*SIZE the same as the
    type size (i.e. as if there were zero elements in the flexible array
    member).  This results e.g. in ELF symbol sizes being too small.

    Note, if the flexible array member is initialized only with non-constant
    initializers, we have a worse bug that this patch doesn't solve, the
    splitting of initializers into constant and dynamic initialization removes
    the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    emitted when emitting those vars into assembly either and so the dynamic
    initialization clobbers other vars that may overlap the variable.
    I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    flexible array member in that case.

    2021-09-14  Jakub Jelinek  <jakub@redhat.com>

            PR c++/102295
            * decl.c (layout_var_decl): For aggregates ending with a flexible
            array member, add the size of the initializer for that member to
            DECL_SIZE and DECL_SIZE_UNIT.

            * g++.target/i386/pr102295.C: New test.

    (cherry picked from commit 818c505188ff5cd8eb048eb0e614c4ef732225bd)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r9-10101-gbd1562be917d088f8a34a27c4f91091382cbd4ab
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Sep 15 22:21:17 2021 +0200

    c++: Fix handling of decls with flexible array members initialized with
side-effects [PR88578]

    > > Note, if the flexible array member is initialized only with
non-constant
    > > initializers, we have a worse bug that this patch doesn't solve, the
    > > splitting of initializers into constant and dynamic initialization
removes
    > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    > > emitted when emitting those vars into assembly either and so the
dynamic
    > > initialization clobbers other vars that may overlap the variable.
    > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    > > flexible array member in that case.
    >
    > Makes sense.

    So, the following patch fixes that.

    The typeck2.c change makes sure we keep those CONSTRUCTORs around (although
    they should be empty because all their elts had side-effects/was
    non-constant if it was removed earlier), and the varasm.c change is to
avoid
    ICEs on those as well as ICEs on other flex array members that had some
    initializers without side-effects, but not on the last array element.

    The code was already asserting that the (index of the last elt in the
    CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the
local->val
    type, which is true for C flex arrays or for C++ if they don't have any
    side-effects or the last elt doesn't have side-effects, this patch changes
    that to assertion that the TYPE_SIZE_UNIT is greater than equal to the
    offset of the end of last element in the CONSTRUCTOR and uses
TYPE_SIZE_UNIT
    (int_size_in_bytes) in the code later on.

    2021-09-15  Jakub Jelinek  <jakub@redhat.com>

            PR c++/88578
            PR c++/102295
    gcc/
            * varasm.c (output_constructor_regular_field): Instead of assertion
            that array_size_for_constructor result is equal to size of
            TREE_TYPE (local->val) in bytes, assert that the type size is
greater
            or equal to array_size_for_constructor result and use type size as
            fieldsize.
    gcc/cp/
            * typeck2.c (split_nonconstant_init_1): Don't throw away empty
            initializers of flexible array members if they have non-zero type
            size.
    gcc/testsuite/
            * g++.dg/ext/flexary39.C: New test.
            * g++.dg/ext/flexary40.C: New test.

    (cherry picked from commit e5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6)

[Bug c++/102295] ELF symbol sizes for variable-length objects are too small (C++)

[jakub at gcc dot gnu.org]

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

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

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

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