Re: [Patch,Fortran] PR92284 – gfc_desc_to_cfi_desc fixes

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Patch,Fortran] PR92284 – gfc_desc_to_cfi_desc fixes

Paul Richard Thomas
Hi Tobias,

OK for trunk and for 9-branch. As with the patch for PR92277, you will
have to get release manager approval for 9.2.

Thanks for working on this.

Cheers

Paul

On Wed, 30 Oct 2019 at 23:29, Tobias Burnus <[hidden email]> wrote:

>
> Playing with the PR92284 test case revealed two issues related to
> gfc_desc_to_cfi_desc:
>
> * Access of uninitialized memory – copying the array bounds (in
> libgfortran) does not make sense for unallocted allocatables and
> nullified pointers. Hence, check for "<descr>.data == NULL".
>
> * There is a memory leak. I misunderstood the dump when fixing PR91863
> (rev.277502).
> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01651.html
>
> Regarding the latter: If one passed gfc_desc_to_cfi_desc a pointer var,
> pointing to NULL, as CFI (Bind(C) array descriptor) argument,
> libgfortran allocates the memory for the descriptor – which at some
> point has to be freed.
>
> Contrary to the original version, one can free that memory
> unconditionally. (Not only because "free" handles NULL pointers but –
> unless "malloc" failed – we know that ptr has been malloced.) I also
> tried to make the comments a bit clearer.
>
> Build and regtested.
> OK for trunk and GCC 9 (the latter is also affected)?
>
> Tobias
>
> PR: Related pending patch:
> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg02148.html
> Also missing: At the end of a bind(C) procedure written in Fortran,
> allocatable/pointers array arguments need get updated: the "data" and
> the bounds part of the array descriptor might have changed while running
> the procedure body. Cf. this PR and PR 92189
>


--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Reply | Threaded
Open this post in threaded view
|

Re: [Patch,Fortran] PR92284 – gfc_desc_to_cfi_desc fixes

Tobias Burnus-3
I have now committed the patch to the GCC 9 branch – and added the test
case for PR92277 (the ICE was a GCC 10 regression, only). – Rev. 277781.

Thanks for the review.

Tobias

PS: The release-manage item came up because the GCC home page linked to
the wrong GCC 9 status report (pre-9.2 release instead of post-9.2
release; both: August 2019). [Now fixed.]

On 10/31/19 10:46 AM, Paul Richard Thomas wrote:

> Hi Tobias,
>
> OK for trunk and for 9-branch. As with the patch for PR92277, you will
> have to get release manager approval for 9.2.
>
> Thanks for working on this.
>
> Cheers
>
> Paul
>
> On Wed, 30 Oct 2019 at 23:29, Tobias Burnus <[hidden email]> wrote:
>> Playing with the PR92284 test case revealed two issues related to
>> gfc_desc_to_cfi_desc:
>>
>> * Access of uninitialized memory – copying the array bounds (in
>> libgfortran) does not make sense for unallocted allocatables and
>> nullified pointers. Hence, check for "<descr>.data == NULL".
>>
>> * There is a memory leak. I misunderstood the dump when fixing PR91863
>> (rev.277502).
>> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01651.html
>>
>> Regarding the latter: If one passed gfc_desc_to_cfi_desc a pointer var,
>> pointing to NULL, as CFI (Bind(C) array descriptor) argument,
>> libgfortran allocates the memory for the descriptor – which at some
>> point has to be freed.
>>
>> Contrary to the original version, one can free that memory
>> unconditionally. (Not only because "free" handles NULL pointers but –
>> unless "malloc" failed – we know that ptr has been malloced.) I also
>> tried to make the comments a bit clearer.
>>
>> Build and regtested.
>> OK for trunk and GCC 9 (the latter is also affected)?
>>
>> Tobias
>>
>> PR: Related pending patch:
>> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg02148.html
>> Also missing: At the end of a bind(C) procedure written in Fortran,
>> allocatable/pointers array arguments need get updated: the "data" and
>> the bounds part of the array descriptor might have changed while running
>> the procedure body. Cf. this PR and PR 92189
>>
>

committed.diff (10K) Download Attachment