Hi Paul,

Am 11.05.24 um 08:20 schrieb Paul Richard Thomas:
Hi Harald,

Thanks for the review. The attached resubmission fixes all the invalid
accesses, memory leaks and puts right the incorrect result.

In the course of fixing the fix, I found that deferred character length
MOLDs gave an ICE because reallocation on assign was using 'dest_word_len'
before it was defined. This is fixed by not fixing 'dest_word_len' for
these MOLDs. Unfortunately, the same did not work for unlimited polymorphic
MOLD expressions and so I added a TODO error in iresolve.cc since it
results in all manner of memory errors in runtime. I will return to this
another day.

A resubmission of the patch for PR113363 will follow since it depends on
this one to fix all the memory problems.

OK for mainline?

this is OK from my side.

One minor nit: the updated testcase transfer_class_4.f90 has

  if (sz /= storage_size (real32)/8) stop 1

I think you meant either  storage_size (r)  or  storage_size (1._real32)
instead of checking the storage size of the integer real32 here...

Thanks for the patch!

Harald

Regards

Paul

On Thu, 9 May 2024 at 08:52, Paul Richard Thomas <
paul.richard.tho...@gmail.com> wrote:

Hi Harald,

The Linaro people caught that as well. Thanks.

Interestingly, I was about to re-submit the patch for PR113363, in which
all the invalid accesses and memory leaks are fixed but requires this patch
to do so. The final transfer was thrown in because it seemed to be working
out of the box but should be checked anyway.

Inserting your print statements, my test shows the difference in
size(chr_a) but prints chr_a as "abcdefgjj" no matter what I do. Needless
to say, the latter was the only check that I did. The problem, I suspect,
lies somewhere in the murky depths of
trans-array.cc(gfc_alloc_allocatable_for_assignment) or in the array part
of intrinsic_transfer, untouched by either patch, and is present in 13- and
14-branches.

I am onto it.

Cheers

Paul


On Wed, 8 May 2024 at 22:06, Harald Anlauf <anl...@gmx.de> wrote:

Hi Paul,

this looks mostly good, but the new testcase transfer_class_4.f90
does exhibit a problem with your patch.  Run it with valgrind,
or with -fcheck=bounds, or with -fsanitize=address, or add the
following around the final transfer:

print *, storage_size (star_a), storage_size (chr_a), size (chr_a), len
(chr_a)
    chr_a = transfer (star_a, chr_a)
print *, storage_size (star_a), storage_size (chr_a), size (chr_a), len
(chr_a)
print *, ">", chr_a, "<"

This prints for me:

            40          40           2           5$
            40          40           4           5$
   >abcdefghij^@^@^@^@^@^@^@^@^@^@<$

So since the physical representation of chr_a is sufficient
to hold star_a (F2023:16.9.212), no reallocation with a wrong
calculated size should happen.  (Intel and NAG get this right.)

Can you check again?

Thanks,
Harald





Reply via email to