[PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero

[PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero

[Fritz Reese]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 .

This code was originally authored by me and the fix is trivial, so I
intend to commit the attached patch in the next few days if there is
no dissent.


The bug is caused by gfc_conv_union_initializer in
gcc/fortran/trans-expr.cc, which accepts a pointer to a vector of
constructor trees (vec<constructor_elt, va_gc>*) as an argument, then
appends one or two field constructors to the vector. The problem is
the use of CONSTRUCTOR_APPEND_ELT(v, ...) within
gfc_conv_union_initializer, which modifies the vector pointer v when a
reallocation of the vector occurs, but the pointer is passed by value.
Therefore, when a vector reallocation occurs, the caller's
(gfc_conv_structure) vector pointer is not updated and subsequently
points to freed memory. Chaos ensues.

The bug only occurs when gfc_conv_union_initializer itself triggers
the reallocation, which is whenever the vector is "full"
(v->m_vecpfx.m_alloc == v->m_vecpfx.m_num). Since the vector defaults
to allocating 8 elements and doubles in size for every reallocation,
the bug only occurs when there are 8, 16, 32, etc... fields with
initializers prior to the union, causing the vector of constructors to
be resized when entering gfc_conv_union_initializer. The
-finit-derived and -finit-local-zero options together ensure each
field has an initializer, triggering the bug.

The patch fixes the bug by passing the vector pointer to
gfc_conv_union_initializer by reference, matching the signature of
vec_safe_push from within the CONSTRUCTOR_APPEND_ELT macro.

--
Fritz Reese

[-- Attachment #2: pr105310.patch --]
[-- Type: application/x-patch, Size: 499 bytes --]

Re: [PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero

[Harald Anlauf]

Hi Fritz,

Am 20.04.22 um 20:03 schrieb Fritz Reese via Fortran:
> See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 .
>
> This code was originally authored by me and the fix is trivial, so I
> intend to commit the attached patch in the next few days if there is
> no dissent.

OK if you add a/the testcase.

>
> The bug is caused by gfc_conv_union_initializer in
> gcc/fortran/trans-expr.cc, which accepts a pointer to a vector of
> constructor trees (vec<constructor_elt, va_gc>*) as an argument, then
> appends one or two field constructors to the vector. The problem is
> the use of CONSTRUCTOR_APPEND_ELT(v, ...) within
> gfc_conv_union_initializer, which modifies the vector pointer v when a
> reallocation of the vector occurs, but the pointer is passed by value.
> Therefore, when a vector reallocation occurs, the caller's
> (gfc_conv_structure) vector pointer is not updated and subsequently
> points to freed memory. Chaos ensues.
>
> The bug only occurs when gfc_conv_union_initializer itself triggers
> the reallocation, which is whenever the vector is "full"
> (v->m_vecpfx.m_alloc == v->m_vecpfx.m_num). Since the vector defaults
> to allocating 8 elements and doubles in size for every reallocation,
> the bug only occurs when there are 8, 16, 32, etc... fields with
> initializers prior to the union, causing the vector of constructors to
> be resized when entering gfc_conv_union_initializer. The
> -finit-derived and -finit-local-zero options together ensure each
> field has an initializer, triggering the bug.
>
> The patch fixes the bug by passing the vector pointer to
> gfc_conv_union_initializer by reference, matching the signature of
> vec_safe_push from within the CONSTRUCTOR_APPEND_ELT macro.
>
> --
> Fritz Reese

As this affects all branches, you may backport the patch as far as
you feel reasonable.  (No, I do not use DEC extensions personally.)

Thanks for the patch!

Harald

Re: [PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero

[Harald Anlauf]

Hi Fritz,

Am 20.04.22 um 20:03 schrieb Fritz Reese via Fortran:
> See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 .
> 
> This code was originally authored by me and the fix is trivial, so I
> intend to commit the attached patch in the next few days if there is
> no dissent.

OK if you add a/the testcase.

> 
> The bug is caused by gfc_conv_union_initializer in
> gcc/fortran/trans-expr.cc, which accepts a pointer to a vector of
> constructor trees (vec<constructor_elt, va_gc>*) as an argument, then
> appends one or two field constructors to the vector. The problem is
> the use of CONSTRUCTOR_APPEND_ELT(v, ...) within
> gfc_conv_union_initializer, which modifies the vector pointer v when a
> reallocation of the vector occurs, but the pointer is passed by value.
> Therefore, when a vector reallocation occurs, the caller's
> (gfc_conv_structure) vector pointer is not updated and subsequently
> points to freed memory. Chaos ensues.
> 
> The bug only occurs when gfc_conv_union_initializer itself triggers
> the reallocation, which is whenever the vector is "full"
> (v->m_vecpfx.m_alloc == v->m_vecpfx.m_num). Since the vector defaults
> to allocating 8 elements and doubles in size for every reallocation,
> the bug only occurs when there are 8, 16, 32, etc... fields with
> initializers prior to the union, causing the vector of constructors to
> be resized when entering gfc_conv_union_initializer. The
> -finit-derived and -finit-local-zero options together ensure each
> field has an initializer, triggering the bug.
> 
> The patch fixes the bug by passing the vector pointer to
> gfc_conv_union_initializer by reference, matching the signature of
> vec_safe_push from within the CONSTRUCTOR_APPEND_ELT macro.
> 
> --
> Fritz Reese

As this affects all branches, you may backport the patch as far as
you feel reasonable.  (No, I do not use DEC extensions personally.)

Thanks for the patch!

Harald

Re: [PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero

[Fritz Reese]

On Wed, Apr 20, 2022, 16:27 Harald Anlauf <anlauf@gmx.de> wrote:
>
> Hi Fritz,
>
> Am 20.04.22 um 20:03 schrieb Fritz Reese via Fortran:
> > See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 .
>
> OK if you add a/the testcase.
..
>
> As this affects all branches, you may backport the patch as far as
> you feel reasonable.  (No, I do not use DEC extensions personally.)
>
> Thanks for the patch!
>
> Harald

Thanks for taking a look Harald. I've committed the test case along
with the patch and backported to 9, 10, 11. I would love to backport
to 8 as well but I think that branch is closed by now.

Cheers,
Fritz