From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106864 invoked by alias); 27 Mar 2015 12:48:37 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 106743 invoked by uid 89); 27 Mar 2015 12:48:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wi0-f182.google.com Received: from mail-wi0-f182.google.com (HELO mail-wi0-f182.google.com) (209.85.212.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 27 Mar 2015 12:48:35 +0000 Received: by wibgn9 with SMTP id gn9so29772540wib.1; Fri, 27 Mar 2015 05:48:32 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.180.102.73 with SMTP id fm9mr57660747wib.12.1427460512198; Fri, 27 Mar 2015 05:48:32 -0700 (PDT) Received: by 10.180.77.5 with HTTP; Fri, 27 Mar 2015 05:48:32 -0700 (PDT) In-Reply-To: <20150324180620.3c72960e@vepi2> References: <20150226181717.480e282c@vepi2> <551006FF.1080704@sfr.fr> <20150323134357.6af740d1@vepi2> <20150324180620.3c72960e@vepi2> Date: Fri, 27 Mar 2015 12:48:00 -0000 Message-ID: Subject: Re: [Patch, Fortran, pr60322] was: [Patch 1/2, Fortran, pr60322] [OOP] Incorrect bounds on polymorphic dummy array From: Paul Richard Thomas To: Andre Vehreschild Cc: Mikael Morin , GCC-Fortran-ML , GCC-Patches-ML , Antony Lewis , Dominique Dhumieres Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-03/txt/msg01437.txt.bz2 Dear Andre, I am in the UK as of last night. Before leaving, I bootstrapped and regtested your patch and all was well. I must drive to Cambridge this afternoon to see my mother and will try to get to it either this evening or tomorrow morning. There is so much of it and it touches many places; so I must give it a very careful looking over before giving the green light. Bear with me please. Great work though! Paul On 24 March 2015 at 18:06, Andre Vehreschild wrote: > Hi all, > > I have worked on the comments Mikael gave me. I am now checking for > class_pointer in the way he pointed out. > > Furthermore did I *join the two parts* of the patch into this one, because > keeping both in sync was no benefit but only tedious and did not prove to= be > reviewed faster. > > Paul, Dominique: I have addressed the LOC issue that came up lately. Or r= ather > the patch addressed it already. I feel like this is not tested very well,= not > the loc() call nor the sizeof() call as given in the 57305 second's downl= oad. > Unfortunately, is that download not runable. I would love to see a test s= imilar > to that download, but couldn't come up with one, that satisfied me. Given= that > the patch's review will last some days, I still have enough time to come = up > with something beautiful which I will add then. > > Bootstraps and regtests ok on x86_64-linux-gnu/F20. > > Regards, > Andre > > > On Tue, 24 Mar 2015 11:13:27 +0100 > Paul Richard Thomas wrote: > >> Dear Andre, >> >> Dominique pointed out to me that the 'loc' patch causes a ICE in the >> testsuite. It seems that 'loc' should provide the address of the class >> container in some places and the address of the data in others. I will >> put my thinking cap on tonight :-) >> >> Cheers >> >> Paul >> >> On 23 March 2015 at 13:43, Andre Vehreschild wrote: >> > Hi Mikael, >> > >> > thanks for looking at the patch. Please note, that Paul has sent an >> > addendum to the patches for 60322, which I deliberately have attached. >> > >> >> 26/02/2015 18:17, Andre Vehreschild a =E9crit : >> >> > This first patch is only preparatory and does not change any of the >> >> > semantics of gfortran at all. >> >> Sure? >> > >> > With the counterexample you found below, this of course is a wrong >> > statement. >> > >> >> > diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c >> >> > index ab6f7a5..d28cf77 100644 >> >> > --- a/gcc/fortran/expr.c >> >> > +++ b/gcc/fortran/expr.c >> >> > @@ -4059,10 +4060,10 @@ gfc_lval_expr_from_sym (gfc_symbol *sym) >> >> > lval->symtree =3D gfc_find_symtree (sym->ns->sym_root, sym->name= ); >> >> > >> >> > /* It will always be a full array. */ >> >> > - lval->rank =3D sym->as ? sym->as->rank : 0; >> >> > + as =3D sym->as; >> >> > + lval->rank =3D as ? as->rank : 0; >> >> > if (lval->rank) >> >> > - gfc_add_full_array_ref (lval, sym->ts.type =3D=3D BT_CLASS ? >> >> > - CLASS_DATA (sym)->as : sym->as); >> >> > + gfc_add_full_array_ref (lval, as); >> >> >> >> This is a change of semantics. Or do you know that sym->ts.type !=3D >> >> BT_CLASS? >> > >> > You are completely right. I have made a mistake here. I have to tell t= he >> > truth, I never ran a regtest with only part 1 of the patches applied. = The >> > second part of the patch will correct this, by setting the variable as >> > depending on whether type =3D=3D BT_CLASS or not. Sorry for the mistak= e. >> > >> >> > diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c >> >> > index 3664824..e571a17 100644 >> >> > --- a/gcc/fortran/trans-decl.c >> >> > +++ b/gcc/fortran/trans-decl.c >> >> > @@ -1013,16 +1017,24 @@ gfc_build_dummy_array_decl (gfc_symbol * sy= m, >> >> > tree dummy) tree decl; >> >> > tree type; >> >> > gfc_array_spec *as; >> >> > + symbol_attribute *array_attr; >> >> > char *name; >> >> > gfc_packed packed; >> >> > int n; >> >> > bool known_size; >> >> > >> >> > - if (sym->attr.pointer || sym->attr.allocatable >> >> > - || (sym->as && sym->as->type =3D=3D AS_ASSUMED_RANK)) >> >> > + /* Use the array as and attr. */ >> >> > + as =3D sym->as; >> >> > + array_attr =3D &sym->attr; >> >> > + >> >> > + /* The pointer attribute is always set on a _data component, the= refore >> >> > check >> >> > + the sym's attribute only. */ >> >> > + if (sym->attr.pointer || array_attr->allocatable >> >> > + || (as && as->type =3D=3D AS_ASSUMED_RANK)) >> >> > return dummy; >> >> > >> >> Any reason to sometimes use array_attr, sometimes not, like here? >> >> By the way, the comment is misleading: for classes, there is the >> >> class_pointer attribute (and it is a pain, I know). >> > >> > Yes, and a good one. Array_attr is sometimes sym->attr and sometimes >> > CLASS_DATA(sym)->attr aka sym->ts.u.derived->components->attr. In the = later >> > case .pointer is always set to 1 in the _data component's attr. I.e., = the >> > above if, would always yield true for a class_array, which is not inte= nded, >> > but rather destructive. I know about the class_pointer attribute, but I >> > figured, that it is not relevant here. Any idea how to formulate the >> > comment better, to reflect what I just explained? >> > >> > Regards, >> > Andre >> > -- >> > Andre Vehreschild * Email: vehre ad gmx dot de >> > >> > >> > ---------- Forwarded message ---------- >> > From: Paul Richard Thomas >> > To: Andre Vehreschild , Dominique Dhumieres >> > Cc: >> > Date: Sun, 22 Mar 2015 21:20:20 +0100 >> > Subject: Bug in intrinsic LOC for scalar class objects >> > Dear Andre and Dominique, >> > >> > I have found that LOC is returning the address of the class container >> > rather than the _data component for class scalars. See the source >> > below, which you will recognise! A fix is attached. >> > >> > Note that the scalar allocate fails with MOLD=3D and so I substituted = SOURCE=3D. >> > >> > Cheers >> > >> > Paul >> > >> > class(*), allocatable :: a(:), e ! Change 'e' to an array and >> > second memcpy works correctly >> > ! Problem is with loc(e), which >> > returns the address of the >> > ! class container. >> > allocate (e, source =3D 99.0) >> > allocate (a(2), source =3D [1.0, 2.0]) >> > call add_element_poly (a,e) >> > select type (a) >> > type is (real) >> > print *, a >> > end select >> > >> > contains >> > >> > subroutine add_element_poly(a,e) >> > use iso_c_binding >> > class(*),allocatable,intent(inout),target :: a(:) >> > class(*),intent(in),target :: e >> > class(*),allocatable,target :: tmp(:) >> > type(c_ptr) :: dummy >> > >> > interface >> > function memcpy(dest,src,n) bind(C,name=3D"memcpy") result(res) >> > import >> > type(c_ptr) :: res >> > integer(c_intptr_t),value :: dest >> > integer(c_intptr_t),value :: src >> > integer(c_size_t),value :: n >> > end function >> > end interface >> > >> > if (.not.allocated(a)) then >> > allocate(a(1), source=3De) >> > else >> > allocate(tmp(size(a)),source=3Da) >> > deallocate(a) >> > allocate(a(size(tmp)+1),source=3De) ! mold gives a segfault >> > dummy =3D memcpy(loc(a(1)),loc(tmp),sizeof(tmp)) >> > dummy =3D memcpy(loc(a(size(tmp)+1)),loc(e),sizeof(e)) >> > end if >> > end subroutine >> > end >> > >> >> >> > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de --=20 Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx