GCC Bugzilla – Bug 31197
[4.2/4.3 regression] TRANSPOSE/RESHAPE and strings
Last modified: 2007-08-30 22:13:21 UTC
With a recent gfortran, the following compiles, but generates the wrong results:
CHARACTER(LEN=3) :: A
TYPE(data), DIMENSION(10), TARGET :: Z
CHARACTER(LEN=10) :: res
IF (res.NE."2222222222") CALL ABORT
The TRANSPOSE(RESHAPE(Z(:)%A(2:2),(/5,2/))) gives:
Hey, this has turned into an ICE on today's trunk. Cool, it's gonna be easier to work on! :)
As I pointed out in http://gcc.gnu.org/ml/fortran/2007-04/msg00295.html, this makes it a potential double regression, which is nasty.
Program received signal SIGSEGV, Segmentation fault.
0x000000000048048b in gfc_conv_function_call (se=0x7fbfffe750, sym=0xe372c0,
2053 need_interface_mapping = ((sym->ts.type == BT_CHARACTER
#0 0x000000000048048b in gfc_conv_function_call (se=0x7fbfffe750,
sym=0xe372c0, arg=0xe004f0, append_args=0x0)
#1 0x000000000048a685 in gfc_conv_intrinsic_funcall (se=0x7fbfffe750,
#2 0x000000000048c341 in gfc_conv_intrinsic_function (se=0x7fbfffe750,
#3 0x0000000000480ae7 in gfc_conv_function_expr (se=0x7fbfffe750, expr=Variable "expr" is not available.
#4 0x000000000046f449 in gfc_add_loop_ss_code (loop=0x7fbfffe920,
ss=0xe37110, subscript=0 '\0')
#5 0x000000000046fcb6 in gfc_conv_loop_setup (loop=0x7fbfffe920)
#6 0x000000000048d881 in gfc_trans_transfer (code=Variable "code" is not available.
#7 0x0000000000467ae5 in gfc_trans_code (code=0xe2b9a0)
#8 0x000000000048fa82 in build_dt (function=0x2a984c5540, code=0xe36cc0)
#9 0x0000000000467b09 in gfc_trans_code (code=0xe36cc0)
#10 0x000000000047b437 in gfc_generate_function_code (ns=0xe2b1c0)
(In reply to comment #2)
> 2053 need_interface_mapping = ((sym->ts.type == BT_CHARACTER
My patch from r123924 (the one about magic numbers) touched gfc_add_interface_mapping. Can you try reverting it, otherwise I'll try tonight. In that case it would be a silly typo.
The segfault is present with r123923, so it's not my patch.
Segafult is because sym->ts.cl is NULL coming into gfc_conv_function_call. I suspect reshape is not getting resolved correctly.
(In reply to comment #5)
> Segafult is because sym->ts.cl is NULL coming into gfc_conv_function_call. I
> suspect reshape is not getting resolved correctly.
I've given this one a look this afternoon, and I couldn't see where the problem is when resolving reshape. It is in fact more general, since TRANSPOSE also has this issue, and I think other transformational intrinsics would behave the same:
character(len=1) :: a
type(data), target :: z(1,1)
z(:,:)%a = "1"
That being said, I don't have the slightest idea where the charlen should be set. (In the above code, the first TRANSPOSE line works fine: where is that charlen set?)
The problem is transpose related. In gfc_conv_intrinsic_funcall we pull in the symbol with gfc_get_symbol_for_expr and then do nothing with it before calling gfc_conv_function_call. The symbol type is BT_UNKNOWN and the name is _gfortran_transpose_char. cl is not set, so at this point I think we look at where that symbol is first generated in simplyfy.c or intrinsic.c. I will have to grep for it later.
I have been chasing this around a bit, clear into decl.c and matchexp.c Nowhere does cl get set. I wonder if there is a separate code path taken when A is defined as a component of a derived type. Stating the obvious, we are missing something here.
If ts.cl->length is set to one, the test case of comment 6 works; see also
http://gcc.gnu.org/ml/fortran/2007-05/msg00072.html which contains a bogus patch which fixes this.
As that patch messes around with length, the test case of comment 0 fails. (The "res" variable contains "111111111" instead of "2222222222", i.e. the value of a(1:1) instead of a(2:2) is used.)
The following code exposes the problem relating to conjg(transpose(.)), with which gfortran returns the wrong answer. If we break conjg(transpose(.)) into two lines, then we can get the right answer.
Is this related to bugs 31196, 31197 and 31258? Should I resubmit this as a new bug?
I have tried versions 4.1, 4.2 and the latest 4.3 (05-14-07). I could'nt find a version that works.
a(1,1) = (1.,1.)
a(2,1) = (2.,2.)
a(1,2) = (3.,3.)
a(2,2) = (4.,4.)
END program main
Ignore Comment #10. I resubmitted my test case as a new bug (31994) last Friday. The bug is fixed this morning.
I have a fix that I will submit tonight or tomorrow morning. It also fixes PR31258 and PR31897.
(In reply to comment #12)
> I have a fix that I will submit tonight or tomorrow morning. It also fixes
> PR31258 and PR31897.
is this still correct ?
(In reply to comment #13)
> (In reply to comment #12)
> > I have a fix that I will submit tonight or tomorrow morning. It also fixes
> > PR31258 and PR31897.
> > Paul
> is this still correct ?
Adding Paul, so he can see this question and hopefully answer affirmatively.
> > >
> > is this still correct ?
> Adding Paul, so he can see this question and hopefully answer affirmatively.
The patch was posted to the list 0615; whilst functional, in that it fixed the bugs, bootstrapped and regtested OK, it was not the whole job. The intention was to sort out some of the fundamental problems with characters and to get rid of some of the kludges in translation. The patch was also judged to be untimely
relative to the 4.3 release schedule so I agreed to hold back.
I am, in fact, working on it this week and hope to get it out of the door this weekend.
I am contemplating breaking the patch up into stages, of which the fix to this PR would be one. What do you think?
(In reply to comment #15)
> The patch was also judged to be
> relative to the 4.3 release schedule so I agreed to hold back.
since it is a regression wrt 4.1 , a fix could go in at 'anytime' ? If it is very invasive, one should fix 4.2 before 4.2.1 though...
> since it is a regression wrt 4.1 , a fix could go in at 'anytime' ? If it is
> very invasive, one should fix 4.2 before 4.2.1 though...
one should *not* fix 4.2 of course
(In reply to comment #17)
> > since it is a regression wrt 4.1 , a fix could go in at 'anytime' ? If it is
> > very invasive, one should fix 4.2 before 4.2.1 though...
> one should *not* fix 4.2 of course
All this argues for breaking it up into chunks. I'll spend this afternoon on it, rather than going on the conference excursion.
(In reply to comment #18)
> I'll spend this afternoon on
> it, rather than going on the conference excursion.
depending on location/weather, I'd go for the conference excursion ;-)
Subject: Re: [4.2/4.3 regression] TRANSPOSE/RESHAPE and
jv244 at cam dot ac dot uk wrote:
> ------- Comment #19 from jv244 at cam dot ac dot uk 2007-07-04 10:05 -------
> (In reply to comment #18)
>> I'll spend this afternoon on
>> it, rather than going on the conference excursion.
> depending on location/weather, I'd go for the conference excursion ;-)
Warsaw, 18.5 C, overcast. Of course, Paul's work on gfortran is more
important than anything else :-)
> Warsaw, 18.5 C, overcast. Of course, Paul's work on gfortran is more
> important than anything else :-)
There is also the question of what I am expected to do over the weekend after three weeks away from home. Catching up with gfc will not even be on the list.
Subject: Bug 31197
Date: Thu Aug 30 22:10:55 2007
New Revision: 127939
2007-08-31 Paul Thomas <email@example.com>
* gfortran.h : Add prototype for gfc_resolve_substring_charlen.
* resolve.c (gfc_resolve_substring_charlen): New function.
(resolve_ref): Call gfc_resolve_substring_charlen.
(gfc_resolve_character_operator): New function.
(gfc_resolve_expr): Call the new functions in cases where the
character length is missing.
* iresolve.c (cshift, eoshift, merge, pack, reshape, spread,
transpose, unpack): Call gfc_resolve_substring_charlen for
source expressions that are character and have a reference.
* trans.h (gfc_trans_init_string_length) Change name to
gfc_conv_string_length; modify references in trans-expr.c,
trans-array.c and trans-decl.c.
* trans-expr.c (gfc_trans_string_length): Handle case of no
(gfc_conv_aliased_arg): Remove code for treating substrings
and replace with call to gfc_trans_string_length.
* trans-array.c (gfc_conv_expr_descriptor): Remove code for
treating strings and call gfc_trans_string_length instead.
2007-08-31 Paul Thomas <firstname.lastname@example.org>
* gfortran.dg/char_length_7.f90: New test.
* gfortran.dg/char_length_9.f90: New test.
* gfortran.dg/char_assign_1.f90: Add extra warning.
* gfortran.dg/char_length_8.f90: New test.
Fixed on trunk.
Thanks for the report!