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

[Harald Anlauf]

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.