Bug 32954 - pack with -fdefault-integer-8
Summary: pack with -fdefault-integer-8
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: 4.3.0
Assignee: Francois-Xavier Coudert
URL:
Keywords: wrong-code
Depends on:
Blocks: default-integer-8
  Show dependency treegraph
 
Reported: 2007-08-01 05:50 UTC by Thomas Koenig
Modified: 2007-11-03 12:40 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-10-10 10:37:13


Attachments
proposed patch (1.06 KB, patch)
2007-08-01 05:52 UTC, Thomas Koenig
Details | Diff
proposed patch (second try) (1.06 KB, patch)
2007-08-01 19:21 UTC, Thomas Koenig
Details | Diff
gdb session from entering pack_internal to the crash (1.73 KB, text/plain)
2007-08-02 15:49 UTC, Dominique d'Humieres
Details
new gdb session with 'watch sptr' (1.87 KB, text/plain)
2007-08-02 17:42 UTC, Dominique d'Humieres
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koenig 2007-08-01 05:50:33 UTC
This bug is for the mask issues with -fdefault-integer-8,
which cause (among others) the failures in minmaxloc etc.
Comment 1 Thomas Koenig 2007-08-01 05:52:07 UTC
Created attachment 14002 [details]
proposed patch

This should fix it.
Comment 2 Francois-Xavier Coudert 2007-08-01 08:21:57 UTC
(In reply to comment #1)
> This should fix it.

This patch is pre-approved (as well as small variations and improvements of it), though it might be worth opening an enhancement PR to note that if the user code has carefully used an array mask of logical(kind=1) to spare memory, we end just wasting memory by converting it all to a default logical kind, don't we?
Comment 3 Dominique d'Humieres 2007-08-01 13:51:18 UTC
The first test of PR32770, i.e.:

program main
  real, dimension(2) :: a
  call random_number(a)
  print *,maxloc(a,mask=.true.)
end program main

with -fdefault-integer-8 and your patch, gives (PPC Darwin8):

pr32770.f90:5.16:

end program main
               1
Internal Error at (1):
pr32770.f90:4.24:

  print *,maxloc(a,mask=.true.)
                       1
Can't convert LOGICAL(8) to LOGICAL(8) at (1)

Anything wrong on my side?
Comment 4 Dominique d'Humieres 2007-08-01 14:28:25 UTC
gfortran.dg/minmaxloc_1.f90 gives the same error in my build.
Comment 5 Dominique d'Humieres 2007-08-01 15:36:06 UTC
I have had a nonexpert look at the patch and I wonder if

+      ts.kind = gfc_default_logical_kind;

should not be

+      ts.kind = newkind;

???
Comment 6 kargls 2007-08-01 17:16:42 UTC
(In reply to comment #5)
> I have had a nonexpert look at the patch and I wonder if
> 
> +      ts.kind = gfc_default_logical_kind;
> 
> should not be
> 
> +      ts.kind = newkind;
> 

Yes, I believe you are correct.  Thomas, can you update your patch?

Comment 7 tkoenig@alice-dsl.net 2007-08-01 17:46:32 UTC
Subject: Re:  mask and -fdefault-integer-8

On Wed, 2007-08-01 at 15:36 +0000, dominiq at lps dot ens dot fr wrote:
> 
> ------- Comment #5 from dominiq at lps dot ens dot fr  2007-08-01 15:36 -------
> I have had a nonexpert look at the patch and I wonder if
> 
> +      ts.kind = gfc_default_logical_kind;
> 
> should not be
> 
> +      ts.kind = newkind;

You are entirely correct.  I've updated the patch and will commit
the corrected version once the regression-test has passed and
I've written up a ChangeLog entry.

I'll use minmaxloc_1.f90 with "-fdefault-integer-8" as the test case.

	Thomas

Comment 8 Thomas Koenig 2007-08-01 18:49:07 UTC
Even with the correction, the patch didn't pass regression-testing.
It's a good thing we do this.

I'll continue my investigations.
Comment 9 Dominique d'Humieres 2007-08-01 19:05:32 UTC
With the change my tests now compile (regtesting!-), but gfortran.dg/zero_sized_1.f90 aborts.

BTW I don't understand the error:

Can't convert LOGICAL(8) to LOGICAL(8) at (1)

How can a "no operation" trigger an error?

Comment 10 Dominique d'Humieres 2007-08-01 19:08:56 UTC
Sorry it was a "Bus error" and not an abort:

Host Name:      pbook
Date/Time:      2007-08-01 20:54:37.875 +0200
OS Version:     10.4.10 (Build 8R218)
Report Version: 4

Command: a.out
Path:    a.out
Parent:  tcsh [16119]

Version: ??? (???)

PID:    2084
Thread: 0

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000004

Thread 0 Crashed:
0   libgfortran.3.dylib         0x00473cf4 pack_internal + 468 (pack_generic.c:240)
1   a.out                       0x0000c0ac test_pack_ + 3124
2   a.out                       0x0000f380 MAIN__ + 68
3   a.out                       0x0000f3c8 main + 48 (fmain.c:26)
4   a.out                       0x000025dc _start + 760
5   a.out                       0x000022e0 start + 48

Thread 0 crashed with PPC Thread State 64:
  srr0: 0x0000000000473cf4 srr1: 0x100000000000f030                        vrsave: 0x0000000000000000
    cr: 0x42084024          xer: 0x0000000000000000   lr: 0x0000000000473ee8  ctr: 0x0000000090003f28
    r0: 0x0000000000000000   r1: 0x00000000bfffdba0   r2: 0x0000000000000000   r3: 0x00000000006005e0
    r4: 0x0000000000000040   r5: 0x00000000bfffdfac   r6: 0x0000000000000007   r7: 0x000000000000000a
    r8: 0x000000000070200f   r9: 0x000000000000003c  r10: 0x000000000070000b  r11: 0x0000000042080022
   r12: 0x0000000090003938  r13: 0x0000000000000000  r14: 0x00000000bfffdd50  r15: 0x00000000bfffdd38
   r16: 0x00000000bfffdc14  r17: 0x0000000000000008  r18: 0x0000000000000000  r19: 0x0000000000000008
   r20: 0x0000000000000008  r21: 0x0000000000000000  r22: 0x0000000000000008  r23: 0x0000000000000000
   r24: 0x00000000006005e0  r25: 0x0000000000000002  r26: 0x0000000000000000  r27: 0x00000000bfffdc2c
   r28: 0x00000000bfffdbf4  r29: 0x00000000006005d0  r30: 0x0000000000000004  r31: 0x0000000000473b30

Binary Images Description:
    0x1000 -     0xffff a.out   /Users/dominiq/Documents/Fortran/g95bench/win/f90/bug/a.out
   0x15000 -    0x20fff libgcc_s.1.dylib        /opt/gcc/gcc4.3w/lib/libgcc_s.1.dylib
   0x7f000 -    0x8afff libgcc_s.1.dylib        /libgcc_s.1.dylib
  0x405000 -   0x481fff libgfortran.3.dylib     /opt/gcc/gcc4.3w/lib/libgfortran.3.dylib
0x8fe00000 - 0x8fe52fff dyld 46.12      /usr/lib/dyld
0x90000000 - 0x901bcfff libSystem.B.dylib       /usr/lib/libSystem.B.dylib
0x90214000 - 0x90219fff libmathCommon.A.dylib   /usr/lib/system/libmathCommon.A.dylib

Comment 11 Dominique d'Humieres 2007-08-01 19:11:57 UTC
The problem is with PACK. If I comment the line

call test_pack

the test pass.

Comment 12 Thomas Koenig 2007-08-01 19:21:16 UTC
Created attachment 14003 [details]
proposed patch (second try)

This one should be better.  Currently regtesting.
Comment 13 Dominique d'Humieres 2007-08-01 20:17:59 UTC
I still have the "Bus error". From the backtrace I think the culprit is libgfortran/intrinsics/pack_generic.c. Probably around the lines:

pack_internal (gfc_array_char *ret, const gfc_array_char *array,
               const gfc_array_l4 *mask, const gfc_array_char *vector,
               index_type size)

and

  const GFC_LOGICAL_4 *mptr;

and may be other places I may have missed.  What makes pack_internal special?

Comment 14 Thomas Koenig 2007-08-01 20:27:42 UTC
Subject: Bug 32954

Author: tkoenig
Date: Wed Aug  1 20:27:27 2007
New Revision: 127137

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127137
Log:
2007-08-01  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR libfortran/32954
	* intrinsic.c (resolve_mask_arg):  New function.
	(gfc_resolve_maxloc):  Use resolve_mask_arg for mask resolution.
	(gfc_resolve_maxval):  Likewise.
	(gfc_resolve_minloc):  Likewise.
	(gfc_resolve_minval):  Likewise.
	(gfc_resolve_pack):  Likewise.
	(gfc_resolve_product):  Likewise.
	(gfc_resolve_sum):  Likewise.
	(gfc_resolve_unpack):  Likewise.

2007-08-01  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR libfortran/32954
	* minmaxloc_3.f90:  New test case.


Added:
    trunk/gcc/testsuite/gfortran.dg/minmaxloc_3.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/testsuite/ChangeLog

Comment 15 Thomas Koenig 2007-08-01 20:44:40 UTC
(In reply to comment #13)
> I still have the "Bus error". From the backtrace I think the culprit is
> libgfortran/intrinsics/pack_generic.c. Probably around the lines:

Hi Dominique,

I just committed a corrected version of the patch.  Does the failure
persist?

I'm leaving this open for the moment, to catch anything that
may have gone wrong (or that still hasn't been corrected).

BTW, internal_pack is called by the compiler itself
to pack array slices etc. where necessary.
Comment 16 Dominique d'Humieres 2007-08-01 21:19:15 UTC
As far as I can tell, I have applied correctly your latest patch. But the following reduced test

! { dg-do run }
! Transformational functions for zero-sized array and array sections
! Contributed by Francois-Xavier Coudert  <coudert@clipper.ens.fr>

  integer,allocatable :: foo(:,:)
  allocate(foo(0,1:7))
  foo = 1
  if (size(pack(foo,foo/=0,(/1,3,4,5,1,0,7,9/))) /= 8 ) call abort
  deallocate(foo)
end

gives a "Bus error" with -fdefault-integer-8.

Comment 17 Dominique d'Humieres 2007-08-02 08:06:40 UTC
The following reduced cas:

  integer,allocatable :: foo(:,:)
  allocate(foo(0,1:7))
  print *, pack(foo(:,1),foo(:,1)==0,(/1,2/))
  deallocate(foo)
end

gives a "Bus error" when run. It works if I replace the mask by a scalar or if I remove the vector (/1,2/).
This is for gfortran 4.3.0 revision 127147 for which I have reloaded iresolve.c from the repository (anyway I had the right version). Do you see the bus error? or is it a bug for big endian machines only?
Comment 18 Dominique d'Humieres 2007-08-02 09:53:38 UTC
The test of #17 pass on AMD64 with gfortran 4.3.0 20070713, but fails on PPC Darwin8 with gfortran 4.3.0 20070802.
Comment 19 Dominique d'Humieres 2007-08-02 15:49:42 UTC
Created attachment 14009 [details]
gdb session from entering pack_internal to the crash

I have also had a look to the results of -fdump-tree-original and to the assembly with and without the flag, but did not see anything obvious.
Comment 20 Thomas Koenig 2007-08-02 17:17:55 UTC
(In reply to comment #19)

> I have also had a look to the results of -fdump-tree-original and to the
> assembly with and without the flag, but did not see anything obvious.

This is very strange.  sptr gets clobbered somewhere.

Within gdb, can you set a watchpoint on sptr and rerun the session?
That should allow you to catch where the change is made.

Comment 21 Dominique d'Humieres 2007-08-02 17:42:28 UTC
Created attachment 14011 [details]
new gdb session with 'watch sptr'
Comment 22 Thomas Koenig 2007-10-04 20:36:30 UTC
This is hard to debug without access to a big-endian
machine.  Renaming, unapplying myself.
Comment 23 Francois-Xavier Coudert 2007-11-03 12:40:09 UTC
(In reply to comment #22)
> This is hard to debug without access to a big-endian
> machine.  Renaming, unapplying myself.

This bug isn't present any more, AFAICT. I suspect it was fixed by your GFOR_POINTER_TO_L1 patch:

2007-08-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/32972
        * libgfortran.h:  Remove GFOR_POINTER_L8_TO_L4 macro.
        Add GFOR_POINTER_TO_L1 macro.
        [...]
        * intrinsics/pack_generic.c(pack_internal):  Likewise.
        * intrinsics/unpack_generic.c(unpack_internal):  Likewise.

Great work, Thomas! (well, except that this code creates a few warnings for arguments of incompatible type, see the last lines of comment #14 in PR 22423).