./transfer_assumed_size_1.f90==29961== Invalid read of size 1 ==29961== at 0x4C26B50: memmove (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) ==29961== by 0x400EA5: __transferbug_MOD_bytestostring (transfer_assumed_size_1.f90:31) ==29961== by 0x401657: MAIN__ (transfer_assumed_size_1.f90:41) ==29961== by 0x40174B: main (fmain.c:21) ==29961== Address 0x58c5793 is 13 bytes before a block of size 2 free'd ==29961== at 0x4C2430F: free (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
Take a look at this with valgrind after compiling with -m32. Very interesting
Tobias, what target and options are you compiling with? I can't reproduce this on x86_64-linux...
(In reply to comment #2) > Tobias, what target and options are you compiling with? I can't reproduce this > on x86_64-linux... Nor on i686-linux.
$> gfortran-svn -v Target: i686-pc-linux-gnu gcc version 4.4.0 20080315 (experimental) (GCC) $> gfortran-svn -g -Wall -W transfer_assumed_size_1.f90 $> valgrind --tool=memcheck --leak-check=full a.out [...] ==6291== Invalid read of size 1 ==6291== at 0x4023A83: memmove (mc_replace_strmem.c:516) ==6291== by 0x8048C12: __transferbug_MOD_bytestostring (transfer_assumed_size_1.f90:31) ==6291== by 0x80493A0: MAIN__ (transfer_assumed_size_1.f90:41) ==6291== by 0x80494A8: main (fmain.c:21) ==6291== Address 0x425067b is 13 bytes before a block of size 2 free'd ==6291== at 0x402248F: free (vg_replace_malloc.c:323) ==6291== by 0x804912E: __transferbug_MOD_stringtobytes (transfer_assumed_size_1.f90:21) ==6291== by 0x8049383: MAIN__ (transfer_assumed_size_1.f90:41) ==6291== by 0x80494A8: main (fmain.c:21) ==6291== ==6291== Invalid read of size 1 ==6291== at 0x4023A90: memmove (mc_replace_strmem.c:516) ==6291== by 0x8048C12: __transferbug_MOD_bytestostring (transfer_assumed_size_1.f90:31) ==6291== by 0x80493A0: MAIN__ (transfer_assumed_size_1.f90:41) ==6291== by 0x80494A8: main (fmain.c:21) ==6291== Address 0x4250679 is 15 bytes before a block of size 2 free'd ==6291== at 0x402248F: free (vg_replace_malloc.c:323) ==6291== by 0x804912E: __transferbug_MOD_stringtobytes (transfer_assumed_size_1.f90:21) ==6291== by 0x8049383: MAIN__ (transfer_assumed_size_1.f90:41) ==6291== by 0x80494A8: main (fmain.c:21) [...]
FX, You'll be glad to know that your memory leak patch reports: !! Memory deallocation error in the code generated by the GNU Fortran compiler. !! Please report this issue to http://gcc.gnu.org/bugzilla/ Even on Cygwin_NT:) Paul
OK, I now can reproduce it. Here's a reduced testcase: module TransferBug type ByteType integer(kind=1) :: singleByte end type type(ByteType) :: BytesPrototype(1) contains function StringToBytes(v) result (bytes) character(len=2) :: v type (ByteType) :: bytes(size(transfer(v, BytesPrototype))) bytes = transfer('Hi', BytesPrototype) end function subroutine BytesToString(bytes, string) type (ByteType) :: bytes(2) character(len=*) :: string string = transfer(bytes, string) end subroutine end module program main use TransferBug character(len=3) :: str call BytesToString( StringToBytes('Hi'), str ) end program
For cris-elf, this graduated to a runtime error equivalent to SEGV (no valgrind); worked with 134139, failed from 134147.
Try what? I always start with a clean slate; an empty build directory! Perhaps you mean something else?
(In reply to comment #8) > Try what? I always start with a clean slate; an empty build directory! > Perhaps you mean something else? Oops, replied in wrong PR... please ignore.
Worked again with trunk as of r134973, stopped working again as in comment #7 with r135298. I don't see any signs of this bug actively fixed?
(In reply to comment #10) > I don't see any signs of this bug actively fixed? I wanted to look into it, but I can't reproduce it any more on x86_64-linux with current mainline.
(In reply to comment #11) > (In reply to comment #10) > > I don't see any signs of this bug actively fixed? > > I wanted to look into it, but I can't reproduce it any more on x86_64-linux > with current mainline. The bug is apparently of the kind that hides from time to time. If you want to look into it, I suggest you use one of the mentioned revisions. IIUC you once had a revision where you could repeat it? For CRIS, the bug hid again (or with less probability, was fixed) with r135315, no surprises.
valgrind says: ==3482== 474 bytes in 3 blocks are definitely lost in loss record 4 of 11 ==3482== at 0x4A04D1F: calloc (vg_replace_malloc.c:279) ==3482== by 0xB3500A: xcalloc (xmalloc.c:162) ==3482== by 0x558A47: init_emit (emit-rtl.c:5012) ==3482== by 0x5F0B52: prepare_function_start (function.c:3903) ==3482== by 0x5F2B68: init_function_start (function.c:3950) ==3482== by 0x4916AD: trans_function_start (trans-decl.c:1599) ==3482== by 0x49847D: gfc_generate_function_code (trans-decl.c:3108) ==3482== by 0x47F921: gfc_generate_module_code (trans.c:1206) ==3482== by 0x454AFB: gfc_parse_file (parse.c:3580) ==3482== by 0x47C69D: gfc_be_parse_file (f95-lang.c:258) ==3482== by 0x6E1140: toplev_main (toplev.c:962) ==3482== by 0x3FF061E073: (below main) (libc-start.c:220) ==3482== ==3482== ==3482== 816 bytes in 6 blocks are definitely lost in loss record 5 of 11 ==3482== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==3482== by 0xB34FCB: xrealloc (xmalloc.c:177) ==3482== by 0x8C53B0: vec_heap_o_reserve_1 (vec.c:176) ==3482== by 0x50336D: insn_locators_alloc (vecprim.h:27) ==3482== by 0xA8FEA8: tree_expand_cfg (cfgexpand.c:1850) ==3482== by 0x6656A1: execute_one_pass (passes.c:1252) ==3482== by 0x6658A0: execute_pass_list (passes.c:1302) ==3482== by 0x745C85: tree_rest_of_compilation (tree-optimize.c:421) ==3482== by 0x8F4A01: cgraph_expand_function (cgraphunit.c:1148) ==3482== by 0x8F67A3: cgraph_assemble_pending_functions (cgraphunit.c:514) ==3482== by 0x8F5CF4: cgraph_finalize_function (cgraphunit.c:632) ==3482== by 0x499705: gfc_generate_function_code (trans-decl.c:3371) ==3482== ==3482== ==3482== 91,856 (70,320 direct, 21,536 indirect) bytes in 1,827 blocks are definitely lost in loss record 10 of 11 ==3482== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==3482== by 0xB35057: xmalloc (xmalloc.c:147) ==3482== by 0x447514: gfc_getmem (misc.c:37) ==3482== by 0x470C37: gfc_get_namespace (symbol.c:2079) ==3482== by 0x4544B1: parse_contained (parse.c:3103) ==3482== by 0x454C82: gfc_parse_file (parse.c:3409) ==3482== by 0x47C69D: gfc_be_parse_file (f95-lang.c:258) ==3482== by 0x6E1140: toplev_main (toplev.c:962) ==3482== by 0x3FF061E073: (below main) (libc-start.c:220) ==3482== ==3482== LEAK SUMMARY: ==3482== definitely lost: 71,610 bytes in 1,836 blocks. ==3482== indirectly lost: 21,536 bytes in 145 blocks. ==3482== possibly lost: 64 bytes in 2 blocks. ==3482== still reachable: 335,995 bytes in 1,228 blocks. ==3482== suppressed: 0 bytes in 0 blocks.
(In reply to comment #13) > valgrind says What target? I don't see anything.
x86-64-linux with valgrind --leak-check=full f951 transfer_assumed_size_1.f90 Using built-in specs. Target: x86_64-unknown-linux-gnu Configured with: ../gcc44/configure --prefix=/home/jerry/gcc/usr --enable-languages=c,fortran --disable-libmudflap --enable-libgomp --with-mpfr-lib=/home/jerry/gcc/usr/lib --with-mpfr-include=/home/jerry/gcc/usr/include --disable-bootstrap Thread model: posix gcc version 4.4.0 20080514 (experimental) [trunk revision 134151] (GCC)
(In reply to comment #15) > x86-64-linux with valgrind --leak-check=full f951 transfer_assumed_size_1.f90 Oh, I see. But the rest of the PR is about running the compiled program. I don't think the two are related. For what it's worth, the following is a reduced testcase for the compiler leak issue: type ByteType integer(kind=1) :: singleByte end type type(ByteType) :: bytes bytes = transfer(' ', bytes) end
On AMD Atholon64 x2 4800+ with openSUSE 11 (x86-64 beta3) I still get the following valgrind message with today's gfortran (4.4.0 20080515 [trunk revision 135326]): $ gfortran -g gfortran.dg/transfer_assumed_size_1.f90 $ valgrind ./a.out [...] Invalid read of size 1 at 0x4C26AF0: memmove (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) by 0x400E9F: __transferbug_MOD_bytestostring (transfer_assumed_size_1.f90:31) by 0x40163C: MAIN__ (transfer_assumed_size_1.f90:41) by 0x40172B: main (fmain.c:21) Address 0x58e27cb is 13 bytes before a block of size 2 free'd at 0x4C2430F: free (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so) by 0x4013AD: __transferbug_MOD_stringtobytes (transfer_assumed_size_1.f90:21) by 0x401624: MAIN__ (transfer_assumed_size_1.f90:41) by 0x40172B: main (fmain.c:21)
The problem is in ByteToString. The assignment of the transfer result is changed to a memmove. The memmove is controlled by the size of both the lhs and the rhs. The size of the rhs (actually the charlen=3 in this case) is not the same as that of the actual data, as the charlen was copied from string's typespec in transfer. Thus, we are copying 3 bytes, instead of 2. I think the code is still a valid program and a wrong-code, as while the standard allow to have a result (partially) undefined/processor-dependent, it doesn't allow to read unallocated memory. module TransferBug type ByteType integer(kind=1) :: singleByte end type type(ByteType) :: BytesPrototype(2) contains function StringToBytes(v) result (bytes) character(len=2) :: v type (ByteType) :: bytes(2)!size(transfer(v, BytesPrototype))) bytes = transfer('Hi', BytesPrototype) end function subroutine BytesToString(bytes, string) type (ByteType) :: bytes(2) character(len=*) :: string string = transfer(bytes, string) end subroutine end module program main use TransferBug character(len=3) :: str type(ByteType) :: i(1) call BytesToString( StringToBytes('Hi'), str ) end program Valgrind doesn't complain on this testcase, but I strongly suspect that the bug is still there. Using the commented array size forces the creation of temporaries, and then valgrind catches the error. Now, how to fix this? We could unconditionally create a temporary in transfer, so that we have both the original data size and the target data size at the time of the memmove. That would probably mean a lot of regressions, and not only speed ones. We could keep track of the original data size. I must admit I'm not satisfied with this one either. What else?
I have noticed that the scalar TRANSFER does not respect the undefined condition for the rest of the result if the SOURCE size is less than the MOLD size. I have a patch for this, which is probably not complete but goes in the right direction. In particular, it assumes that array types are compelete, which was not the case when array TRANSFER was written. My inclination is to combine the two to ensure complete bomb-proofness. I will post my provisional patch tomorrow. Paul
Created attachment 17079 [details] A preliminary patch for the PR A slightly earlier version of this regtested OK. Will do it to this version tonight and then will submit. Cheers Paul
On i686-apple-darwin9 I cannot test the valgrind part of this pr, however with the patch in http://gcc.gnu.org/ml/fortran/2009-01/msg00162.html the following test now succeeds: character(len=1) :: string = "z" character(len=20) :: tmp = "" tmp = Upper ("abcdefgh") print *, tmp contains Character (len=20) Function Upper (string) Character(len=*) string integer :: ij print *, len(string) print *, size(transfer(string,"xy",len(string))) i = size(transfer(string,"xy",len(string))) if (i /= len(string)) call abort() Upper = "" Upper(1:2) = & transfer(merge(transfer(string,"xy",len(string)), & string(1:2), .true.), "xy") return end function Upper end (coming from pr31608). I saw one regression on char_cast_1.f90 which needs some adjustment of the test, the following change allows it to pass: --- ../_gcc_clean/gcc/testsuite/gfortran.dg/char_cast_1.f90 2008-05-19 14:20:35.000000000 +0200 +++ gcc/testsuite/gfortran.dg/char_cast_1.f90 2009-01-14 07:37:03.000000000 +0100 @@ -27,5 +27,5 @@ end ! The sign that all is well is that [S.5][1] appears twice. ! Platform dependent variations are [S$5][1], [__S_5][1], [S___5][1] -! { dg-final { scan-tree-dump-times "5\\\]\\\[1\\\]" 2 "original" } } +! { dg-final { scan-tree-dump-times "6\\\]\\\[1\\\]" 2 "original" } } ! { dg-final { cleanup-tree-dump "original" } } From the comment the test seems fragile and should probably changed to something more robust. Thanks for the patch.
Subject: Bug 34955 Author: pault Date: Sat Jan 17 11:32:02 2009 New Revision: 143462 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143462 Log: 2009-01-17 Paul Thomas <pault@gcc.gnu.org> PR fortran/34955 * trans-intrinsic.c (gfc_conv_intrinsic_array_transfer): Has been absorbed into gfc_conv_intrinsic_transfer. All references to it in trans-intrinsic.c have been changed accordingly. PR fixed by using a temporary for scalar character transfer, when the source is shorter than the destination. 2009-01-17 Paul Thomas <pault@gcc.gnu.org> PR fortran/34955 * gfortran.dg/transfer_intrinsic_1.f90: New test. * gfortran.dg/transfer_intrinsic_2.f90: New test. Added: trunk/gcc/testsuite/gfortran.dg/transfer_intrinsic_1.f90 trunk/gcc/testsuite/gfortran.dg/transfer_intrinsic_2.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-intrinsic.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gfortran.dg/char_cast_1.f90
Fixed on trunk. Thanks for the report and sorry that it took so long to fix. Paul