Bug 31197 - [4.2/4.3 regression] TRANSPOSE/RESHAPE and strings
Summary: [4.2/4.3 regression] TRANSPOSE/RESHAPE and strings
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P5 normal
Target Milestone: 4.2.1
Assignee: Paul Thomas
URL:
Keywords: ice-on-valid-code, wrong-code
Depends on:
Blocks: Fortran_character 32834
  Show dependency treegraph
 
Reported: 2007-03-16 11:25 UTC by Joost VandeVondele
Modified: 2007-08-30 22:13 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.1.2
Known to fail: 4.3.0 4.2.0
Last reconfirmed: 2007-06-11 12:53:18


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joost VandeVondele 2007-03-16 11:25:33 UTC
With a recent gfortran, the following compiles, but generates the wrong results:

TYPE data
 CHARACTER(LEN=3) :: A
END TYPE
TYPE(data), DIMENSION(10), TARGET :: Z
CHARACTER(LEN=10) :: res
Z(:)%A="123"
write(res,'(10A1)') TRANSPOSE(RESHAPE(Z(:)%A(2:2),(/5,2/)))
IF (res.NE."2222222222") CALL ABORT
END
Comment 1 Tobias Burnus 2007-03-16 13:25:05 UTC
Expected:
  "2222222222"
The TRANSPOSE(RESHAPE(Z(:)%A(2:2),(/5,2/))) gives:
  "2222222222.�L+"
res contains:
  "22�+2�?"
Comment 2 Francois-Xavier Coudert 2007-04-18 10:54:16 UTC
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, 
    arg=0xe004f0, append_args=0x0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-expr.c:2053
2053      need_interface_mapping = ((sym->ts.type == BT_CHARACTER
(gdb) where
#0  0x000000000048048b in gfc_conv_function_call (se=0x7fbfffe750, 
    sym=0xe372c0, arg=0xe004f0, append_args=0x0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-expr.c:2053
#1  0x000000000048a685 in gfc_conv_intrinsic_funcall (se=0x7fbfffe750, 
    expr=0xe35fe0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-intrinsic.c:1514
#2  0x000000000048c341 in gfc_conv_intrinsic_function (se=0x7fbfffe750, 
    expr=0xe35fe0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-intrinsic.c:3945
#3  0x0000000000480ae7 in gfc_conv_function_expr (se=0x7fbfffe750, expr=Variable "expr" is not available.
)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-expr.c:2698
#4  0x000000000046f449 in gfc_add_loop_ss_code (loop=0x7fbfffe920, 
    ss=0xe37110, subscript=0 '\0')
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-array.c:1803
#5  0x000000000046fcb6 in gfc_conv_loop_setup (loop=0x7fbfffe920)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-array.c:3232
#6  0x000000000048d881 in gfc_trans_transfer (code=Variable "code" is not available.
)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-io.c:1850
#7  0x0000000000467ae5 in gfc_trans_code (code=0xe2b9a0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans.c:605
#8  0x000000000048fa82 in build_dt (function=0x2a984c5540, code=0xe36cc0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-io.c:1507
#9  0x0000000000467b09 in gfc_trans_code (code=0xe36cc0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans.c:581
#10 0x000000000047b437 in gfc_generate_function_code (ns=0xe2b1c0)
    at /utmp/coudert/gfortran/trunk/gcc/fortran/trans-decl.c:3181
Comment 3 Tobias Schlüter 2007-04-18 15:56:03 UTC
(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.
Comment 4 Tobias Schlüter 2007-04-18 19:29:59 UTC
The segfault is present with r123923, so it's not my patch.
Comment 5 Jerry DeLisle 2007-05-04 17:20:51 UTC
Segafult is because sym->ts.cl is NULL coming into gfc_conv_function_call.  I suspect reshape is not getting resolved correctly.
Comment 6 Francois-Xavier Coudert 2007-05-04 17:29:35 UTC
(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:

  type data
    character(len=1) :: a
  end type
  type(data), target :: z(1,1)
  z(:,:)%a = "1"
  write(*,*) transpose(z(:,:)%a(:))
  write(*,*) transpose(z(:,:)%a(1:1))
end

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?)
Comment 7 Jerry DeLisle 2007-05-04 17:51:43 UTC
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.
Comment 8 Jerry DeLisle 2007-05-04 20:39:15 UTC
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.
Comment 9 Tobias Burnus 2007-05-04 22:29:24 UTC
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.)
Comment 10 Elizabeth Yip 2007-05-15 07:48:00 UTC
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.

Elizabeth Yip
---------------------------------------------------------------------

program main
  implicit none
  complex (kind=4),dimension(2,2)::a,b,c
  a(1,1) = (1.,1.)
  a(2,1) = (2.,2.)
  a(1,2) = (3.,3.)
  a(2,2) = (4.,4.) 
  print *,"original",a
  b=conjg(transpose(a))
  print *,"H(a)-wrong",b
  c=transpose(a)
  c=conjg(c) 
  print *,"H(a)-right",c
END program main

Comment 11 Elizabeth Yip 2007-05-21 20:10:06 UTC
Ignore Comment #10.  I resubmitted my test case as a new bug (31994) last Friday.  The bug is fixed this morning.
Comment 12 Paul Thomas 2007-06-11 12:53:18 UTC
I have a fix that I will submit tonight or tomorrow morning. It also fixes PR31258 and PR31897.

Paul
Comment 13 Joost VandeVondele 2007-07-03 18:33:23 UTC
(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 ?
Comment 14 Tobias Schlüter 2007-07-03 18:36:28 UTC
(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.
Comment 15 Paul Thomas 2007-07-04 08:50:11 UTC
> > > 
> > 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?

Paul
Comment 16 Joost VandeVondele 2007-07-04 09:09:19 UTC
(In reply to comment #15)
> The patch was also judged to be
> untimely
> 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...
Comment 17 Joost VandeVondele 2007-07-04 09:09:51 UTC
> 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 

Comment 18 Paul Thomas 2007-07-04 09:26:50 UTC
(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.

Paul
Comment 19 Joost VandeVondele 2007-07-04 10:05:45 UTC
(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 ;-)

Comment 20 Tobias Schlüter 2007-07-04 10:51:49 UTC
Subject: Re:  [4.2/4.3 regression] TRANSPOSE/RESHAPE and
 strings

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 :-)
Comment 21 Paul Thomas 2007-07-04 11:32:03 UTC
> 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.

P
Comment 22 Paul Thomas 2007-08-30 22:11:13 UTC
Subject: Bug 31197

Author: pault
Date: Thu Aug 30 22:10:55 2007
New Revision: 127939

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127939
Log:
2007-08-31  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/31879
	PR fortran/31197
	PR fortran/31258
	PR fortran/32703
	* 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
	backend_decl.
	(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  <pault@gcc.gnu.org>

	PR fortran/31879
	* 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.

	PR fortran/31197
	PR fortran/31258
	* gfortran.dg/char_length_8.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/char_length_7.f90
    trunk/gcc/testsuite/gfortran.dg/char_length_8.f90
    trunk/gcc/testsuite/gfortran.dg/char_length_9.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/char_assign_1.f90

Comment 23 Paul Thomas 2007-08-30 22:13:21 UTC
Fixed on trunk.

Thanks for the report!

Paul