Fortran: fix assignment to allocatable scalar polymorphic component [PR121616]

  Dear all,

here's a - once found - seemingly simple and obvious fix for a memory
corruption happening when intrinsic assignment is used to set a scalar
allocatable polymorphic component of a derived type when the latter
is instanciated as an array of rank > 0.  Just get the dimension
attribute right when using gfc_variable_attr ...

The testcase is an extended version of the reporter's with unlimited
polymorphism, including another simpler one contributed by a friend.
Without the fix, both tests crash with memory corruption of various
kinds.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

If there are no objections, I would like to backport to at least
15-branch.

Thanks,
Harald
  

On 9/11/25 11:28 AM, Harald Anlauf wrote:
> Dear all,
> 
> here's a - once found - seemingly simple and obvious fix for a memory
> corruption happening when intrinsic assignment is used to set a scalar
> allocatable polymorphic component of a derived type when the latter
> is instanciated as an array of rank > 0.  Just get the dimension
> attribute right when using gfc_variable_attr ...
> 
> The testcase is an extended version of the reporter's with unlimited
> polymorphism, including another simpler one contributed by a friend.
> Without the fix, both tests crash with memory corruption of various
> kinds.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
> If there are no objections, I would like to backport to at least
> 15-branch.
> 
> Thanks,
> Harald
> 

OK and backport, very simple.

Jerry
  

Hi Jerry!

Am 11.09.25 um 21:30 schrieb Jerry D:
> On 9/11/25 11:28 AM, Harald Anlauf wrote:
>> Dear all,
>>
>> here's a - once found - seemingly simple and obvious fix for a memory
>> corruption happening when intrinsic assignment is used to set a scalar
>> allocatable polymorphic component of a derived type when the latter
>> is instanciated as an array of rank > 0.  Just get the dimension
>> attribute right when using gfc_variable_attr ...
>>
>> The testcase is an extended version of the reporter's with unlimited
>> polymorphism, including another simpler one contributed by a friend.
>> Without the fix, both tests crash with memory corruption of various
>> kinds.
>>
>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>
>> If there are no objections, I would like to backport to at least
>> 15-branch.
>>
>> Thanks,
>> Harald
>>
> 
> OK and backport, very simple.

Thanks for the review!

Pushed as r16-3813-g0899b826f7196f.

I wonder how many more cases we have in the frontend where walking
a reference chain misses similar simple things...

Harald

> Jerry
>
  

Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
> Dear all,
> 
> here's a - once found - seemingly simple and obvious fix for a memory
> corruption happening when intrinsic assignment is used to set a scalar
> allocatable polymorphic component of a derived type when the latter
> is instanciated as an array of rank > 0.  Just get the dimension
> attribute right when using gfc_variable_attr ...
> 
> The testcase is an extended version of the reporter's with unlimited
> polymorphism, including another simpler one contributed by a friend.
> Without the fix, both tests crash with memory corruption of various
> kinds.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> 
Hello Harald,

> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
> index 6df95558bb1..2cb930d83b8 100644
> --- a/gcc/fortran/primary.cc
> +++ b/gcc/fortran/primary.cc
> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *ts)
>  
>  	if (comp->ts.type == BT_CLASS)
>  	  {
> +	    dimension = CLASS_DATA (comp)->attr.dimension;
>  	    codimension = CLASS_DATA (comp)->attr.codimension;
>  	    pointer = CLASS_DATA (comp)->attr.class_pointer;
>  	    allocatable = CLASS_DATA (comp)->attr.allocatable;
>  	  }
>  	else
>  	  {
> +	    dimension = comp->attr.dimension;
>  	    codimension = comp->attr.codimension;
>  	    if (expr->ts.type == BT_CLASS && strcmp (comp->name, "_data") == 0)
>  	      pointer = comp->attr.class_pointer;

I think the dimension flag should additionally be cleared if there is an 
array element reference after the component.  Otherwise one could get 
the dimension attribute for a scalar expression (say 
derived%array_comp(123)).
I don't really have a testcase that would exhibit a failure, I'm just 
being overly cautious.
Thanks for the patch in any case.
  

Am 11.09.25 um 22:27 schrieb Mikael Morin:
> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>> Dear all,
>>
>> here's a - once found - seemingly simple and obvious fix for a memory
>> corruption happening when intrinsic assignment is used to set a scalar
>> allocatable polymorphic component of a derived type when the latter
>> is instanciated as an array of rank > 0.  Just get the dimension
>> attribute right when using gfc_variable_attr ...
>>
>> The testcase is an extended version of the reporter's with unlimited
>> polymorphism, including another simpler one contributed by a friend.
>> Without the fix, both tests crash with memory corruption of various
>> kinds.
>>
>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>
> Hello Harald,
> 
>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>> index 6df95558bb1..2cb930d83b8 100644
>> --- a/gcc/fortran/primary.cc
>> +++ b/gcc/fortran/primary.cc
>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>> gfc_typespec *ts)
>>
>>      if (comp->ts.type == BT_CLASS)
>>        {
>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>        }
>>      else
>>        {
>> +        dimension = comp->attr.dimension;
>>          codimension = comp->attr.codimension;
>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, "_data") 
>> == 0)
>>            pointer = comp->attr.class_pointer;
> 
> I think the dimension flag should additionally be cleared if there is an 
> array element reference after the component.  Otherwise one could get 
> the dimension attribute for a scalar expression (say 
> derived%array_comp(123)).
> I don't really have a testcase that would exhibit a failure, I'm just 
> being overly cautious.
> Thanks for the patch in any case.

You mean further up?

     switch (ref->type)
       {
       case REF_ARRAY:

	switch (ref->u.ar.type)
	  {
...
	  case AR_ELEMENT:
	    /* Handle coarrays.  */
	    if (ref->u.ar.dimen > 0)
	      allocatable = pointer = optional = false;
	    break;

I saw that in passing, but did not hit it in the debugging session.
I'd have to think of a more complex testcase.
  

Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>> Dear all,
>>>
>>> here's a - once found - seemingly simple and obvious fix for a memory
>>> corruption happening when intrinsic assignment is used to set a scalar
>>> allocatable polymorphic component of a derived type when the latter
>>> is instanciated as an array of rank > 0.  Just get the dimension
>>> attribute right when using gfc_variable_attr ...
>>>
>>> The testcase is an extended version of the reporter's with unlimited
>>> polymorphism, including another simpler one contributed by a friend.
>>> Without the fix, both tests crash with memory corruption of various
>>> kinds.
>>>
>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>
>> Hello Harald,
>>
>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>> index 6df95558bb1..2cb930d83b8 100644
>>> --- a/gcc/fortran/primary.cc
>>> +++ b/gcc/fortran/primary.cc
>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>> gfc_typespec *ts)
>>>
>>>      if (comp->ts.type == BT_CLASS)
>>>        {
>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>        }
>>>      else
>>>        {
>>> +        dimension = comp->attr.dimension;
>>>          codimension = comp->attr.codimension;
>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>> "_data") == 0)
>>>            pointer = comp->attr.class_pointer;
>>
>> I think the dimension flag should additionally be cleared if there is 
>> an array element reference after the component.  Otherwise one could 
>> get the dimension attribute for a scalar expression (say 
>> derived%array_comp(123)).
>> I don't really have a testcase that would exhibit a failure, I'm just 
>> being overly cautious.
>> Thanks for the patch in any case.
> 
> You mean further up?
> 
>      switch (ref->type)
>        {
>        case REF_ARRAY:
> 
>      switch (ref->u.ar.type)
>        {
> ...
>        case AR_ELEMENT:
>          /* Handle coarrays.  */
>          if (ref->u.ar.dimen > 0)
>            allocatable = pointer = optional = false;
>          break;
> 
Yes, that's the place.

The more I look at your patch, the less I understand it.
So, given an array expression such as array(:)%scalar_comp, 
gfc_variable_attr on it would return a result without the dimension 
attribute?
  

Am 12.09.25 um 11:12 schrieb Mikael Morin:
> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>> Dear all,
>>>>
>>>> here's a - once found - seemingly simple and obvious fix for a memory
>>>> corruption happening when intrinsic assignment is used to set a scalar
>>>> allocatable polymorphic component of a derived type when the latter
>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>> attribute right when using gfc_variable_attr ...
>>>>
>>>> The testcase is an extended version of the reporter's with unlimited
>>>> polymorphism, including another simpler one contributed by a friend.
>>>> Without the fix, both tests crash with memory corruption of various
>>>> kinds.
>>>>
>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>
>>> Hello Harald,
>>>
>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>> index 6df95558bb1..2cb930d83b8 100644
>>>> --- a/gcc/fortran/primary.cc
>>>> +++ b/gcc/fortran/primary.cc
>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>>> gfc_typespec *ts)
>>>>
>>>>      if (comp->ts.type == BT_CLASS)
>>>>        {
>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>        }
>>>>      else
>>>>        {
>>>> +        dimension = comp->attr.dimension;
>>>>          codimension = comp->attr.codimension;
>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>>> "_data") == 0)
>>>>            pointer = comp->attr.class_pointer;
>>>
>>> I think the dimension flag should additionally be cleared if there is 
>>> an array element reference after the component.  Otherwise one could 
>>> get the dimension attribute for a scalar expression (say 
>>> derived%array_comp(123)).
>>> I don't really have a testcase that would exhibit a failure, I'm just 
>>> being overly cautious.
>>> Thanks for the patch in any case.
>>
>> You mean further up?
>>
>>      switch (ref->type)
>>        {
>>        case REF_ARRAY:
>>
>>      switch (ref->u.ar.type)
>>        {
>> ...
>>        case AR_ELEMENT:
>>          /* Handle coarrays.  */
>>          if (ref->u.ar.dimen > 0)
>>            allocatable = pointer = optional = false;
>>          break;
>>
> Yes, that's the place.
> 
> The more I look at your patch, the less I understand it.
> So, given an array expression such as array(:)%scalar_comp, 
> gfc_variable_attr on it would return a result without the dimension 
> attribute?

Well, the comment before gfc_variable_attr says:

/* Given an expression that is a variable, figure out what the
    ultimate variable's type and attribute is, traversing the reference
    structures if necessary.

    This subroutine is trickier than it looks.  We start at the base
    symbol and store the attribute.  Component references load a
    completely new attribute.

...

Assuming that scalar_comp is the ultimate component, its dimension is 0.


The standard appears somewhat ambiguous on the expression you give:

9.4 Scalars

Note 2
ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent

9.5 Arrays

Note 3

SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
  

Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
> Am 12.09.25 um 11:12 schrieb Mikael Morin:
>> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>>> Dear all,
>>>>>
>>>>> here's a - once found - seemingly simple and obvious fix for a memory
>>>>> corruption happening when intrinsic assignment is used to set a scalar
>>>>> allocatable polymorphic component of a derived type when the latter
>>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>>> attribute right when using gfc_variable_attr ...
>>>>>
>>>>> The testcase is an extended version of the reporter's with unlimited
>>>>> polymorphism, including another simpler one contributed by a friend.
>>>>> Without the fix, both tests crash with memory corruption of various
>>>>> kinds.
>>>>>
>>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>
>>>> Hello Harald,
>>>>
>>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>>> index 6df95558bb1..2cb930d83b8 100644
>>>>> --- a/gcc/fortran/primary.cc
>>>>> +++ b/gcc/fortran/primary.cc
>>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>>>> gfc_typespec *ts)
>>>>>
>>>>>      if (comp->ts.type == BT_CLASS)
>>>>>        {
>>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>>        }
>>>>>      else
>>>>>        {
>>>>> +        dimension = comp->attr.dimension;
>>>>>          codimension = comp->attr.codimension;
>>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>>>> "_data") == 0)
>>>>>            pointer = comp->attr.class_pointer;
>>>>
>>>> I think the dimension flag should additionally be cleared if there 
>>>> is an array element reference after the component.  Otherwise one 
>>>> could get the dimension attribute for a scalar expression (say 
>>>> derived%array_comp(123)).
>>>> I don't really have a testcase that would exhibit a failure, I'm 
>>>> just being overly cautious.
>>>> Thanks for the patch in any case.
>>>
>>> You mean further up?
>>>
>>>      switch (ref->type)
>>>        {
>>>        case REF_ARRAY:
>>>
>>>      switch (ref->u.ar.type)
>>>        {
>>> ...
>>>        case AR_ELEMENT:
>>>          /* Handle coarrays.  */
>>>          if (ref->u.ar.dimen > 0)
>>>            allocatable = pointer = optional = false;
>>>          break;
>>>
>> Yes, that's the place.
>>
>> The more I look at your patch, the less I understand it.
>> So, given an array expression such as array(:)%scalar_comp, 
>> gfc_variable_attr on it would return a result without the dimension 
>> attribute?
> 
> Well, the comment before gfc_variable_attr says:
> 
> /* Given an expression that is a variable, figure out what the
>     ultimate variable's type and attribute is, traversing the reference
>     structures if necessary.
> 
>     This subroutine is trickier than it looks.  We start at the base
>     symbol and store the attribute.  Component references load a
>     completely new attribute.
> 
The latter sentence applies to allocatable, pointer/target, etc, but I 
don't think it should apply to dimension.
Dimension is different because it's the parent reference that determines 
whether the subreference is a scalar or an array, thus whether it has 
the dimension attribute or not.

> ...
> 
> Assuming that scalar_comp is the ultimate component, its dimension is 0.
> 
Err, well, no. I mean, it depends on what's before it.
derived(123)%scalar_comp is a scalar, so it doesn't have the dimension 
attribute.
But derived(:)%scalar_comp is an array, so it has the dimension attribute.

> 
> The standard appears somewhat ambiguous on the expression you give:
> 
> 9.4 Scalars
> 
> Note 2
> ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent
> 
That one is definitely an array, I hope we can agree on that.
See the second sentence of 9.4.2 saying:
     A structure component may be a scalar or an array.

So the presence in the section about scalars is not an indication that 
it really is scalar.

> 9.5 Arrays
> 
> Note 3
> 
> SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
> SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
> 

Coming back to your patch, rephrasing my previous concerns.
If I understand correctly, your patch changes the dimension attribute 
for the following cases:
                            old value   new value
1. array(i)%scalar_comp       1           0
2. array(:)%scalar_comp       1           0
3. scalar%array_comp(i)       0           1

1. is the desired change, but I think 2. and 3. are undesired.
  

Am 13.09.25 um 22:54 schrieb Mikael Morin:
> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>> Am 12.09.25 um 11:12 schrieb Mikael Morin:
>>> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>>>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>>>> Dear all,
>>>>>>
>>>>>> here's a - once found - seemingly simple and obvious fix for a memory
>>>>>> corruption happening when intrinsic assignment is used to set a 
>>>>>> scalar
>>>>>> allocatable polymorphic component of a derived type when the latter
>>>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>>>> attribute right when using gfc_variable_attr ...
>>>>>>
>>>>>> The testcase is an extended version of the reporter's with unlimited
>>>>>> polymorphism, including another simpler one contributed by a friend.
>>>>>> Without the fix, both tests crash with memory corruption of various
>>>>>> kinds.
>>>>>>
>>>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>>
>>>>> Hello Harald,
>>>>>
>>>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>>>> index 6df95558bb1..2cb930d83b8 100644
>>>>>> --- a/gcc/fortran/primary.cc
>>>>>> +++ b/gcc/fortran/primary.cc
>>>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>>>>> gfc_typespec *ts)
>>>>>>
>>>>>>      if (comp->ts.type == BT_CLASS)
>>>>>>        {
>>>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>>>        }
>>>>>>      else
>>>>>>        {
>>>>>> +        dimension = comp->attr.dimension;
>>>>>>          codimension = comp->attr.codimension;
>>>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>>>>> "_data") == 0)
>>>>>>            pointer = comp->attr.class_pointer;
>>>>>
>>>>> I think the dimension flag should additionally be cleared if there 
>>>>> is an array element reference after the component.  Otherwise one 
>>>>> could get the dimension attribute for a scalar expression (say 
>>>>> derived%array_comp(123)).
>>>>> I don't really have a testcase that would exhibit a failure, I'm 
>>>>> just being overly cautious.
>>>>> Thanks for the patch in any case.
>>>>
>>>> You mean further up?
>>>>
>>>>      switch (ref->type)
>>>>        {
>>>>        case REF_ARRAY:
>>>>
>>>>      switch (ref->u.ar.type)
>>>>        {
>>>> ...
>>>>        case AR_ELEMENT:
>>>>          /* Handle coarrays.  */
>>>>          if (ref->u.ar.dimen > 0)
>>>>            allocatable = pointer = optional = false;
>>>>          break;
>>>>
>>> Yes, that's the place.
>>>
>>> The more I look at your patch, the less I understand it.
>>> So, given an array expression such as array(:)%scalar_comp, 
>>> gfc_variable_attr on it would return a result without the dimension 
>>> attribute?
>>
>> Well, the comment before gfc_variable_attr says:
>>
>> /* Given an expression that is a variable, figure out what the
>>     ultimate variable's type and attribute is, traversing the reference
>>     structures if necessary.
>>
>>     This subroutine is trickier than it looks.  We start at the base
>>     symbol and store the attribute.  Component references load a
>>     completely new attribute.
>>
> The latter sentence applies to allocatable, pointer/target, etc, but I 
> don't think it should apply to dimension.
> Dimension is different because it's the parent reference that determines 
> whether the subreference is a scalar or an array, thus whether it has 
> the dimension attribute or not.
> 
>> ...
>>
>> Assuming that scalar_comp is the ultimate component, its dimension is 0.
>>
> Err, well, no. I mean, it depends on what's before it.
> derived(123)%scalar_comp is a scalar, so it doesn't have the dimension 
> attribute.
> But derived(:)%scalar_comp is an array, so it has the dimension attribute.
> 
>>
>> The standard appears somewhat ambiguous on the expression you give:
>>
>> 9.4 Scalars
>>
>> Note 2
>> ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent
>>
> That one is definitely an array, I hope we can agree on that.
> See the second sentence of 9.4.2 saying:
>      A structure component may be a scalar or an array.
> 
> So the presence in the section about scalars is not an indication that 
> it really is scalar.
> 
>> 9.5 Arrays
>>
>> Note 3
>>
>> SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
>> SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
>>
> 
> Coming back to your patch, rephrasing my previous concerns.
> If I understand correctly, your patch changes the dimension attribute 
> for the following cases:
>                             old value   new value
> 1. array(i)%scalar_comp       1           0
> 2. array(:)%scalar_comp       1           0
> 3. scalar%array_comp(i)       0           1
> 
> 1. is the desired change, but I think 2. and 3. are undesired.

Well, the patch changes gfc_variable_attr, but see also the comment
I cited.  This is needed that the scalarized assignment sees the
ultimate component, which is an allocatable scalar for the testcase.

I did not claim that gfc_expr_attr always returns the right attributes.
  

Le 14/09/2025 à 21:17, Harald Anlauf a écrit :
> Am 13.09.25 um 22:54 schrieb Mikael Morin:
>> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>>> Am 12.09.25 um 11:12 schrieb Mikael Morin:
>>>> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>>>>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>>>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>>>>> Dear all,
>>>>>>>
>>>>>>> here's a - once found - seemingly simple and obvious fix for a 
>>>>>>> memory
>>>>>>> corruption happening when intrinsic assignment is used to set a 
>>>>>>> scalar
>>>>>>> allocatable polymorphic component of a derived type when the latter
>>>>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>>>>> attribute right when using gfc_variable_attr ...
>>>>>>>
>>>>>>> The testcase is an extended version of the reporter's with unlimited
>>>>>>> polymorphism, including another simpler one contributed by a friend.
>>>>>>> Without the fix, both tests crash with memory corruption of various
>>>>>>> kinds.
>>>>>>>
>>>>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>>>
>>>>>> Hello Harald,
>>>>>>
>>>>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>>>>> index 6df95558bb1..2cb930d83b8 100644
>>>>>>> --- a/gcc/fortran/primary.cc
>>>>>>> +++ b/gcc/fortran/primary.cc
>>>>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>>>>>> gfc_typespec *ts)
>>>>>>>
>>>>>>>      if (comp->ts.type == BT_CLASS)
>>>>>>>        {
>>>>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>>>>        }
>>>>>>>      else
>>>>>>>        {
>>>>>>> +        dimension = comp->attr.dimension;
>>>>>>>          codimension = comp->attr.codimension;
>>>>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>>>>>> "_data") == 0)
>>>>>>>            pointer = comp->attr.class_pointer;
>>>>>>
>>>>>> I think the dimension flag should additionally be cleared if there 
>>>>>> is an array element reference after the component.  Otherwise one 
>>>>>> could get the dimension attribute for a scalar expression (say 
>>>>>> derived%array_comp(123)).
>>>>>> I don't really have a testcase that would exhibit a failure, I'm 
>>>>>> just being overly cautious.
>>>>>> Thanks for the patch in any case.
>>>>>
>>>>> You mean further up?
>>>>>
>>>>>      switch (ref->type)
>>>>>        {
>>>>>        case REF_ARRAY:
>>>>>
>>>>>      switch (ref->u.ar.type)
>>>>>        {
>>>>> ...
>>>>>        case AR_ELEMENT:
>>>>>          /* Handle coarrays.  */
>>>>>          if (ref->u.ar.dimen > 0)
>>>>>            allocatable = pointer = optional = false;
>>>>>          break;
>>>>>
>>>> Yes, that's the place.
>>>>
>>>> The more I look at your patch, the less I understand it.
>>>> So, given an array expression such as array(:)%scalar_comp, 
>>>> gfc_variable_attr on it would return a result without the dimension 
>>>> attribute?
>>>
>>> Well, the comment before gfc_variable_attr says:
>>>
>>> /* Given an expression that is a variable, figure out what the
>>>     ultimate variable's type and attribute is, traversing the reference
>>>     structures if necessary.
>>>
>>>     This subroutine is trickier than it looks.  We start at the base
>>>     symbol and store the attribute.  Component references load a
>>>     completely new attribute.
>>>
>> The latter sentence applies to allocatable, pointer/target, etc, but I 
>> don't think it should apply to dimension.
>> Dimension is different because it's the parent reference that 
>> determines whether the subreference is a scalar or an array, thus 
>> whether it has the dimension attribute or not.
>>
>>> ...
>>>
>>> Assuming that scalar_comp is the ultimate component, its dimension is 0.
>>>
>> Err, well, no. I mean, it depends on what's before it.
>> derived(123)%scalar_comp is a scalar, so it doesn't have the dimension 
>> attribute.
>> But derived(:)%scalar_comp is an array, so it has the dimension 
>> attribute.
>>
>>>
>>> The standard appears somewhat ambiguous on the expression you give:
>>>
>>> 9.4 Scalars
>>>
>>> Note 2
>>> ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent
>>>
>> That one is definitely an array, I hope we can agree on that.
>> See the second sentence of 9.4.2 saying:
>>      A structure component may be a scalar or an array.
>>
>> So the presence in the section about scalars is not an indication that 
>> it really is scalar.
>>
>>> 9.5 Arrays
>>>
>>> Note 3
>>>
>>> SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
>>> SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
>>>
>>
>> Coming back to your patch, rephrasing my previous concerns.
>> If I understand correctly, your patch changes the dimension attribute 
>> for the following cases:
>>                             old value   new value
>> 1. array(i)%scalar_comp       1           0
>> 2. array(:)%scalar_comp       1           0
>> 3. scalar%array_comp(i)       0           1
>>
>> 1. is the desired change, but I think 2. and 3. are undesired.
> 
> Well, the patch changes gfc_variable_attr, but see also the comment
> I cited.  This is needed that the scalarized assignment sees the
> ultimate component, which is an allocatable scalar for the testcase.
> 
> I did not claim that gfc_expr_attr always returns the right attributes.
> 
I bet it does for some definition of "right".
Anyway, I'm propably wasting my time arguing about this.
Let's wait and see if something breaks.
  

On 9/15/25 1:56 AM, Mikael Morin wrote:
> Le 14/09/2025 à 21:17, Harald Anlauf a écrit :
>> Am 13.09.25 um 22:54 schrieb Mikael Morin:
>>> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>>>> Am 12.09.25 um 11:12 schrieb Mikael Morin:
>>>>> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>>>>>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>>>>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>>>>>> Dear all,
>>>>>>>>
>>>>>>>> here's a - once found - seemingly simple and obvious fix for a memory
>>>>>>>> corruption happening when intrinsic assignment is used to set a scalar
>>>>>>>> allocatable polymorphic component of a derived type when the latter
>>>>>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>>>>>> attribute right when using gfc_variable_attr ...
>>>>>>>>
>>>>>>>> The testcase is an extended version of the reporter's with unlimited
>>>>>>>> polymorphism, including another simpler one contributed by a friend.
>>>>>>>> Without the fix, both tests crash with memory corruption of various
>>>>>>>> kinds.
>>>>>>>>
>>>>>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>>>>
>>>>>>> Hello Harald,
>>>>>>>
>>>>>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>>>>>> index 6df95558bb1..2cb930d83b8 100644
>>>>>>>> --- a/gcc/fortran/primary.cc
>>>>>>>> +++ b/gcc/fortran/primary.cc
>>>>>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec 
>>>>>>>> *ts)
>>>>>>>>
>>>>>>>>      if (comp->ts.type == BT_CLASS)
>>>>>>>>        {
>>>>>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>>>>>        }
>>>>>>>>      else
>>>>>>>>        {
>>>>>>>> +        dimension = comp->attr.dimension;
>>>>>>>>          codimension = comp->attr.codimension;
>>>>>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, "_data") 
>>>>>>>> == 0)
>>>>>>>>            pointer = comp->attr.class_pointer;
>>>>>>>
>>>>>>> I think the dimension flag should additionally be cleared if there is an 
>>>>>>> array element reference after the component.  Otherwise one could get the 
>>>>>>> dimension attribute for a scalar expression (say derived%array_comp(123)).
>>>>>>> I don't really have a testcase that would exhibit a failure, I'm just 
>>>>>>> being overly cautious.
>>>>>>> Thanks for the patch in any case.
>>>>>>
>>>>>> You mean further up?
>>>>>>
>>>>>>      switch (ref->type)
>>>>>>        {
>>>>>>        case REF_ARRAY:
>>>>>>
>>>>>>      switch (ref->u.ar.type)
>>>>>>        {
>>>>>> ...
>>>>>>        case AR_ELEMENT:
>>>>>>          /* Handle coarrays.  */
>>>>>>          if (ref->u.ar.dimen > 0)
>>>>>>            allocatable = pointer = optional = false;
>>>>>>          break;
>>>>>>
>>>>> Yes, that's the place.
>>>>>
>>>>> The more I look at your patch, the less I understand it.
>>>>> So, given an array expression such as array(:)%scalar_comp, 
>>>>> gfc_variable_attr on it would return a result without the dimension attribute?
>>>>
>>>> Well, the comment before gfc_variable_attr says:
>>>>
>>>> /* Given an expression that is a variable, figure out what the
>>>>     ultimate variable's type and attribute is, traversing the reference
>>>>     structures if necessary.
>>>>
>>>>     This subroutine is trickier than it looks.  We start at the base
>>>>     symbol and store the attribute.  Component references load a
>>>>     completely new attribute.
>>>>
>>> The latter sentence applies to allocatable, pointer/target, etc, but I don't 
>>> think it should apply to dimension.
>>> Dimension is different because it's the parent reference that determines 
>>> whether the subreference is a scalar or an array, thus whether it has the 
>>> dimension attribute or not.
>>>
>>>> ...
>>>>
>>>> Assuming that scalar_comp is the ultimate component, its dimension is 0.
>>>>
>>> Err, well, no. I mean, it depends on what's before it.
>>> derived(123)%scalar_comp is a scalar, so it doesn't have the dimension 
>>> attribute.
>>> But derived(:)%scalar_comp is an array, so it has the dimension attribute.
>>>
>>>>
>>>> The standard appears somewhat ambiguous on the expression you give:
>>>>
>>>> 9.4 Scalars
>>>>
>>>> Note 2
>>>> ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent
>>>>
>>> That one is definitely an array, I hope we can agree on that.
>>> See the second sentence of 9.4.2 saying:
>>>      A structure component may be a scalar or an array.
>>>
>>> So the presence in the section about scalars is not an indication that it 
>>> really is scalar.
>>>
>>>> 9.5 Arrays
>>>>
>>>> Note 3
>>>>
>>>> SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
>>>> SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
>>>>
>>>
>>> Coming back to your patch, rephrasing my previous concerns.
>>> If I understand correctly, your patch changes the dimension attribute for the 
>>> following cases:
>>>                             old value   new value
>>> 1. array(i)%scalar_comp       1           0
>>> 2. array(:)%scalar_comp       1           0
>>> 3. scalar%array_comp(i)       0           1
>>>
>>> 1. is the desired change, but I think 2. and 3. are undesired.
>>
>> Well, the patch changes gfc_variable_attr, but see also the comment
>> I cited.  This is needed that the scalarized assignment sees the
>> ultimate component, which is an allocatable scalar for the testcase.
>>
>> I did not claim that gfc_expr_attr always returns the right attributes.
>>
> I bet it does for some definition of "right".
> Anyway, I'm propably wasting my time arguing about this.
> Let's wait and see if something breaks.

It is never a waste of time to have open dialogue. We all learn from this 
exchange. Breaking things is also a way to learn and promote further investigation.

Cheers to all,

Jerry
  

Am 15.09.25 um 19:21 schrieb Jerry D:
> On 9/15/25 1:56 AM, Mikael Morin wrote:
>> Le 14/09/2025 à 21:17, Harald Anlauf a écrit :
>>> Am 13.09.25 um 22:54 schrieb Mikael Morin:
>>>> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>>> Well, the patch changes gfc_variable_attr, but see also the comment
>>> I cited.  This is needed that the scalarized assignment sees the
>>> ultimate component, which is an allocatable scalar for the testcase.
>>>
>>> I did not claim that gfc_expr_attr always returns the right attributes.
>>>
>> I bet it does for some definition of "right".
>> Anyway, I'm propably wasting my time arguing about this.
>> Let's wait and see if something breaks.
> 
> It is never a waste of time to have open dialogue. We all learn from 
> this exchange. Breaking things is also a way to learn and promote 
> further investigation.

Quite true.

Ideally, if we kick the tree, all bad apples (bugs) will fall down
eventually.

Regarding attributes, dimensions, and all that stuff, there is an
apparent history visible when looking at the code.  And when it
comes to component, substring, or specially inquiry references
(type, kind, real and imaginary part, all of which were added
over time), the handling seems scattered over lots of places.

I've been wondering if there is some strategy to "differentially"
debug the frontend: using a simple testcase, and replacing just
a single token by another one, and see when and how the path
splits and maybe rejoins later.

Cases at hand: pr108581.  Replacing fixed-length character by
deferred length.  Or pr121939: character(kind=4) vs. kind=1.

Cheers!

> Cheers to all,
> 
> Jerry
>
  

Le 15/09/2025 à 19:21, Jerry D a écrit :
> On 9/15/25 1:56 AM, Mikael Morin wrote:
>> Le 14/09/2025 à 21:17, Harald Anlauf a écrit :
>>> Am 13.09.25 um 22:54 schrieb Mikael Morin:
>>>> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>>>>> Am 12.09.25 um 11:12 schrieb Mikael Morin:
>>>>>> Le 11/09/2025 à 22:46, Harald Anlauf a écrit :
>>>>>>> Am 11.09.25 um 22:27 schrieb Mikael Morin:
>>>>>>>> Le 11/09/2025 à 20:28, Harald Anlauf a écrit :
>>>>>>>>> Dear all,
>>>>>>>>>
>>>>>>>>> here's a - once found - seemingly simple and obvious fix for a 
>>>>>>>>> memory
>>>>>>>>> corruption happening when intrinsic assignment is used to set a 
>>>>>>>>> scalar
>>>>>>>>> allocatable polymorphic component of a derived type when the 
>>>>>>>>> latter
>>>>>>>>> is instanciated as an array of rank > 0.  Just get the dimension
>>>>>>>>> attribute right when using gfc_variable_attr ...
>>>>>>>>>
>>>>>>>>> The testcase is an extended version of the reporter's with 
>>>>>>>>> unlimited
>>>>>>>>> polymorphism, including another simpler one contributed by a 
>>>>>>>>> friend.
>>>>>>>>> Without the fix, both tests crash with memory corruption of 
>>>>>>>>> various
>>>>>>>>> kinds.
>>>>>>>>>
>>>>>>>>> Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>>>>>
>>>>>>>> Hello Harald,
>>>>>>>>
>>>>>>>>> diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
>>>>>>>>> index 6df95558bb1..2cb930d83b8 100644
>>>>>>>>> --- a/gcc/fortran/primary.cc
>>>>>>>>> +++ b/gcc/fortran/primary.cc
>>>>>>>>> @@ -3057,12 +3057,14 @@ gfc_variable_attr (gfc_expr *expr, 
>>>>>>>>> gfc_typespec *ts)
>>>>>>>>>
>>>>>>>>>      if (comp->ts.type == BT_CLASS)
>>>>>>>>>        {
>>>>>>>>> +        dimension = CLASS_DATA (comp)->attr.dimension;
>>>>>>>>>          codimension = CLASS_DATA (comp)->attr.codimension;
>>>>>>>>>          pointer = CLASS_DATA (comp)->attr.class_pointer;
>>>>>>>>>          allocatable = CLASS_DATA (comp)->attr.allocatable;
>>>>>>>>>        }
>>>>>>>>>      else
>>>>>>>>>        {
>>>>>>>>> +        dimension = comp->attr.dimension;
>>>>>>>>>          codimension = comp->attr.codimension;
>>>>>>>>>          if (expr->ts.type == BT_CLASS && strcmp (comp->name, 
>>>>>>>>> "_data") == 0)
>>>>>>>>>            pointer = comp->attr.class_pointer;
>>>>>>>>
>>>>>>>> I think the dimension flag should additionally be cleared if 
>>>>>>>> there is an array element reference after the component.  
>>>>>>>> Otherwise one could get the dimension attribute for a scalar 
>>>>>>>> expression (say derived%array_comp(123)).
>>>>>>>> I don't really have a testcase that would exhibit a failure, I'm 
>>>>>>>> just being overly cautious.
>>>>>>>> Thanks for the patch in any case.
>>>>>>>
>>>>>>> You mean further up?
>>>>>>>
>>>>>>>      switch (ref->type)
>>>>>>>        {
>>>>>>>        case REF_ARRAY:
>>>>>>>
>>>>>>>      switch (ref->u.ar.type)
>>>>>>>        {
>>>>>>> ...
>>>>>>>        case AR_ELEMENT:
>>>>>>>          /* Handle coarrays.  */
>>>>>>>          if (ref->u.ar.dimen > 0)
>>>>>>>            allocatable = pointer = optional = false;
>>>>>>>          break;
>>>>>>>
>>>>>> Yes, that's the place.
>>>>>>
>>>>>> The more I look at your patch, the less I understand it.
>>>>>> So, given an array expression such as array(:)%scalar_comp, 
>>>>>> gfc_variable_attr on it would return a result without the 
>>>>>> dimension attribute?
>>>>>
>>>>> Well, the comment before gfc_variable_attr says:
>>>>>
>>>>> /* Given an expression that is a variable, figure out what the
>>>>>     ultimate variable's type and attribute is, traversing the 
>>>>> reference
>>>>>     structures if necessary.
>>>>>
>>>>>     This subroutine is trickier than it looks.  We start at the base
>>>>>     symbol and store the attribute.  Component references load a
>>>>>     completely new attribute.
>>>>>
>>>> The latter sentence applies to allocatable, pointer/target, etc, but 
>>>> I don't think it should apply to dimension.
>>>> Dimension is different because it's the parent reference that 
>>>> determines whether the subreference is a scalar or an array, thus 
>>>> whether it has the dimension attribute or not.
>>>>
>>>>> ...
>>>>>
>>>>> Assuming that scalar_comp is the ultimate component, its dimension 
>>>>> is 0.
>>>>>
>>>> Err, well, no. I mean, it depends on what's before it.
>>>> derived(123)%scalar_comp is a scalar, so it doesn't have the 
>>>> dimension attribute.
>>>> But derived(:)%scalar_comp is an array, so it has the dimension 
>>>> attribute.
>>>>
>>>>>
>>>>> The standard appears somewhat ambiguous on the expression you give:
>>>>>
>>>>> 9.4 Scalars
>>>>>
>>>>> Note 2
>>>>> ARRAY_PARENT(1:N)%SCALAR_FIELD  component of array section parent
>>>>>
>>>> That one is definitely an array, I hope we can agree on that.
>>>> See the second sentence of 9.4.2 saying:
>>>>      A structure component may be a scalar or an array.
>>>>
>>>> So the presence in the section about scalars is not an indication 
>>>> that it really is scalar.
>>>>
>>>>> 9.5 Arrays
>>>>>
>>>>> Note 3
>>>>>
>>>>> SCALAR_PARENT%ARRAY_FIELD(1:N)               array section
>>>>> SCALAR_PARENT%ARRAY_FIELD(1:N)%SCALAR_FIELD  array section
>>>>>
>>>>
>>>> Coming back to your patch, rephrasing my previous concerns.
>>>> If I understand correctly, your patch changes the dimension 
>>>> attribute for the following cases:
>>>>                             old value   new value
>>>> 1. array(i)%scalar_comp       1           0
>>>> 2. array(:)%scalar_comp       1           0
>>>> 3. scalar%array_comp(i)       0           1
>>>>
>>>> 1. is the desired change, but I think 2. and 3. are undesired.
>>>
>>> Well, the patch changes gfc_variable_attr, but see also the comment
>>> I cited.  This is needed that the scalarized assignment sees the
>>> ultimate component, which is an allocatable scalar for the testcase.
>>>
>>> I did not claim that gfc_expr_attr always returns the right attributes.
>>>
>> I bet it does for some definition of "right".
>> Anyway, I'm propably wasting my time arguing about this.
>> Let's wait and see if something breaks.
> 
> It is never a waste of time to have open dialogue. 
Yeah, well, communication can be difficult.

My premise when I started commenting was that the function should return 
a value as defined by the standard (for the standard attributes). Harald 
doesn't seem to agree with that, and without that premise the function 
is free to return any result, so my point becomes moot.

> We all learn from 
> this exchange. Breaking things is also a way to learn and promote 
> further investigation.
>
  

Le 15/09/2025 à 21:33, Harald Anlauf a écrit :
> Am 15.09.25 um 19:21 schrieb Jerry D:
>> On 9/15/25 1:56 AM, Mikael Morin wrote:
>>> Le 14/09/2025 à 21:17, Harald Anlauf a écrit :
>>>> Am 13.09.25 um 22:54 schrieb Mikael Morin:
>>>>> Le 12/09/2025 à 22:51, Harald Anlauf a écrit :
>>>> Well, the patch changes gfc_variable_attr, but see also the comment
>>>> I cited.  This is needed that the scalarized assignment sees the
>>>> ultimate component, which is an allocatable scalar for the testcase.
>>>>
>>>> I did not claim that gfc_expr_attr always returns the right attributes.
>>>>
>>> I bet it does for some definition of "right".
>>> Anyway, I'm propably wasting my time arguing about this.
>>> Let's wait and see if something breaks.
>>
>> It is never a waste of time to have open dialogue. We all learn from 
>> this exchange. Breaking things is also a way to learn and promote 
>> further investigation.
> 
> Quite true.
> 
> Ideally, if we kick the tree, all bad apples (bugs) will fall down
> eventually.
> 
> Regarding attributes, dimensions, and all that stuff, there is an
> apparent history visible when looking at the code.  And when it
> comes to component, substring, or specially inquiry references
> (type, kind, real and imaginary part, all of which were added
> over time), the handling seems scattered over lots of places.
> 
> I've been wondering if there is some strategy to "differentially"
> debug the frontend: using a simple testcase, and replacing just
> a single token by another one, and see when and how the path
> splits and maybe rejoins later.
> 
Maybe you can use code coverage data, but I'm not aware of any 
ready-made diff tool for it.  gcov can produce json data with the 
--json-format argument, that can be a good starting point.
  

Am 15.09.25 um 22:32 schrieb Mikael Morin:
> Le 15/09/2025 à 21:33, Harald Anlauf a écrit :
>> I've been wondering if there is some strategy to "differentially"
>> debug the frontend: using a simple testcase, and replacing just
>> a single token by another one, and see when and how the path
>> splits and maybe rejoins later.
>>
> Maybe you can use code coverage data, but I'm not aware of any ready- 
> made diff tool for it.  gcov can produce json data with the --json- 
> format argument, that can be a good starting point.

I have no idea how to use that.

What I meant: the modified token may show up e.g. in a variable decl,
and when it is passed to some function in trans*.cc, that tiny
difference evolves etc.

I just get overwhelmed by all the forest I usually end up with,
and hardly see the tiny little twig that counts.