From patchwork Tue Apr 28 10:43:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andre Vehreschild X-Patchwork-Id: 465475 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5E72814007D for ; Tue, 28 Apr 2015 20:43:52 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=eQaQGaEA; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=hz5XbaPHpy526/BB +MYVZsSyX47PPLVJNRuD9oi28JGqDcXjepmdZ8s3LRAo3UB87Sx/zjPP2hPhknxo ni4lakvAKQR2Oe6ksMrxibc18M9CyyAemTOBkuGm3St39OFlkhksfLcnDayefhV9 fA38x1DXNoe+83r9/LczfaiW5dM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=zkIvkYcGG686Uyv3PeVLSd 7OySY=; b=eQaQGaEAWQfXlG/UMwJZDtlvltl5vtiqhXtwpquTFuMesrT09BOPGt yHVgXnYdlJdxo1R2G1EhgPYOOOX4hB/N15m83m61GdY7hENIS1r6jhsfvuF8WWlv ZQLLWoHovUScEXhWPirB50tZmuNEOTrwvYYvRv1H3XAXEEilpA9w8= Received: (qmail 37339 invoked by alias); 28 Apr 2015 10:43:45 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 36422 invoked by uid 89); 28 Apr 2015 10:43:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mout.gmx.net Received: from mout.gmx.net (HELO mout.gmx.net) (212.227.17.20) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 28 Apr 2015 10:43:42 +0000 Received: from localhost ([88.75.104.20]) by mail.gmx.com (mrgmx102) with ESMTPSA (Nemesis) id 0LpPg1-1ZGcdr1VPJ-00fEdw; Tue, 28 Apr 2015 12:43:38 +0200 Date: Tue, 28 Apr 2015 12:43:37 +0200 From: Andre Vehreschild To: Mikael Morin Cc: GCC-Patches-ML , GCC-Fortran-ML Subject: Re: [Patch, Fortran] Prevent segfault with dump-*-original for implicit class expressions. Message-ID: <20150428124337.79d3f638@gmx.de> In-Reply-To: <553C037E.3090006@sfr.fr> References: <20150313113339.32440f8c@vepi2> <553C037E.3090006@sfr.fr> MIME-Version: 1.0 X-UI-Out-Filterresults: notjunk:1; Hi Mikael, hi all, Mikael, thanks for the comments. I have worked in your suggestions and the patch now looks like the attached one. To get the testsuite accepting a dump-fortran-original, I had to insert a dg-prune-output. Without the prune output, dejagnu was reporting excess errors. The prune removes all output after the first Namespace: that starts the fortran-dump. I am not that happy with it, because ICEs in the dump will not be detected that way. But I could not find a better solution yet. Does anyone know of a better way to solve this? Best would be to have the fortran dump go to a file like all other dumps, but that is not scope of this patch. Bootstraps and regtests ok on x86_64-linux-gnu/F21. OK for trunk now? Regards, Andre On Sat, 25 Apr 2015 23:13:34 +0200 Mikael Morin wrote: > Hello, > > Le 13/03/2015 11:33, Andre Vehreschild a écrit : > > Hi all, > > > > this is another patch preventing a segfault. This time the segfault > > occurred, when -fdump-(fortran|tree)-original was given with the program > > having an implicit class set. The issue is that the _data component is > > assumed to be present in a BT_CLASS w/o checking and trying to access the > > unlimited polymorphic flag there. The patch fixes this by redirecting the > > access to the flag to the correct position whether the _data component is > > present or not. > > > > Building a testcase for this is difficult for me. May be I am just blocked > > in the head there. The issue occurred when trying to dump the > > (fortran|tree)-original of the testcase gfortran.dg/implicit_class_1.f90. So > > one could argue to add the flag to that testcase, but does it pay in > > contrast to the additional effort each time the testsuite is executed? I > > have added the test now, not being happy with it, but having no clue how to > > do it better. > > > > Bootstraps and regtests ok on x86_64-linux-gnu/F20. > > > > Ok, for trunk? > > > > Comments below: > > b/gcc/fortran/symbol.c > > index 32eea21..3379f47 100644 > > --- a/gcc/fortran/symbol.c > > +++ b/gcc/fortran/symbol.c > > (skipped) > > The beginning looks good. > I suggest using 'ts->u.derived->attr.is_class' instead of > 'strcmp (ts->u.derived.components->name, "_data") == 0'. > No strong opinion, your choice. > > > > @@ -4576,13 +4579,14 @@ gfc_type_compatible (gfc_typespec *ts1, > > gfc_typespec *ts2) if (is_derived1 && is_derived2) > > return gfc_compare_derived_types (ts1->u.derived, ts2->u.derived); > > > > - if (is_derived1 && is_class2) > > + if (is_derived1 && is_class2 && ts2->u.derived->components) > > return gfc_compare_derived_types (ts1->u.derived, > > ts2->u.derived->components->ts.u.derived); > > - if (is_class1 && is_derived2) > > + if (is_class1 && is_derived2 && ts1->u.derived->components) > > return gfc_type_is_extension_of > > (ts1->u.derived->components->ts.u.derived, ts2->u.derived); > > - else if (is_class1 && is_class2) > > + else if (is_class1 && is_class2 && ts1->u.derived->components > > + && ts2->u.derived->components) > > return gfc_type_is_extension_of > > (ts1->u.derived->components->ts.u.derived, > > ts2->u.derived->components->ts.u.derived); else > > The above change don't seem right. > In the case where the class container is missing, you want to use > "ts->u.derived" instead of "ts->u.derived->components->ts.u.derived", > not skip the procedure call entirely. > > > diff --git a/gcc/testsuite/gfortran.dg/implicit_class_1.f90 > > b/gcc/testsuite/gfortran.dg/implicit_class_1.f90 index 329f57a..fff1f2b > > 100644 --- a/gcc/testsuite/gfortran.dg/implicit_class_1.f90 > > +++ b/gcc/testsuite/gfortran.dg/implicit_class_1.f90 > > @@ -4,6 +4,10 @@ > > ! > > ! Contributed by Reinhold Bader > > > > +! Add dump-tree-original to check, if the patch preventing a gfortran > > +! segfault is working correctly. > > +! { dg-options "-fdump-tree-original" } > > + > -fdump-tree-original doesn't trigger any bug here. > So use -fdump-fortran-original (I'm not sure the testsuite will like it). > > Mikael > diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c index 320eb01..d4c2629 100644 --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -484,13 +484,24 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec *ts2) if (ts1->type == BT_VOID || ts2->type == BT_VOID) return 1; - if (ts1->type == BT_CLASS - && ts1->u.derived->components->ts.u.derived->attr.unlimited_polymorphic) + /* The _data component is not always present, therefore check for its + presence before assuming, that its derived->attr is available. + When the _data component is not present, then nevertheless the + unlimited_polymorphic flag may be set in the derived type's attr. */ + if (ts1->type == BT_CLASS && ts1->u.derived->components + && ((ts1->u.derived->attr.is_class + && ts1->u.derived->components->ts.u.derived->attr + .unlimited_polymorphic) + || ts1->u.derived->attr.unlimited_polymorphic)) return 1; /* F2003: C717 */ if (ts2->type == BT_CLASS && ts1->type == BT_DERIVED - && ts2->u.derived->components->ts.u.derived->attr.unlimited_polymorphic + && ts2->u.derived->components + && ((ts2->u.derived->attr.is_class + && ts2->u.derived->components->ts.u.derived->attr + .unlimited_polymorphic) + || ts2->u.derived->attr.unlimited_polymorphic) && (ts1->u.derived->attr.sequence || ts1->u.derived->attr.is_bind_c)) return 1; diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c index 44392e8..b18608b 100644 --- a/gcc/fortran/symbol.c +++ b/gcc/fortran/symbol.c @@ -4567,7 +4567,10 @@ gfc_type_compatible (gfc_typespec *ts1, gfc_typespec *ts2) if (is_class1 && ts1->u.derived->components - && ts1->u.derived->components->ts.u.derived->attr.unlimited_polymorphic) + && ((ts1->u.derived->attr.is_class + && ts1->u.derived->components->ts.u.derived->attr + .unlimited_polymorphic) + || ts1->u.derived->attr.unlimited_polymorphic)) return 1; if (!is_derived1 && !is_derived2 && !is_class1 && !is_class2) @@ -4578,13 +4581,21 @@ gfc_type_compatible (gfc_typespec *ts1, gfc_typespec *ts2) if (is_derived1 && is_class2) return gfc_compare_derived_types (ts1->u.derived, - ts2->u.derived->components->ts.u.derived); + ts2->u.derived->attr.is_class ? + ts2->u.derived->components->ts.u.derived + : ts2->u.derived); if (is_class1 && is_derived2) - return gfc_type_is_extension_of (ts1->u.derived->components->ts.u.derived, + return gfc_type_is_extension_of (ts1->u.derived->attr.is_class ? + ts1->u.derived->components->ts.u.derived + : ts1->u.derived, ts2->u.derived); else if (is_class1 && is_class2) - return gfc_type_is_extension_of (ts1->u.derived->components->ts.u.derived, - ts2->u.derived->components->ts.u.derived); + return gfc_type_is_extension_of (ts1->u.derived->attr.is_class ? + ts1->u.derived->components->ts.u.derived + : ts1->u.derived, + ts2->u.derived->attr.is_class ? + ts2->u.derived->components->ts.u.derived + : ts2->u.derived); else return 0; } diff --git a/gcc/testsuite/gfortran.dg/implicit_class_1.f90 b/gcc/testsuite/gfortran.dg/implicit_class_1.f90 index 329f57a..46516ca 100644 --- a/gcc/testsuite/gfortran.dg/implicit_class_1.f90 +++ b/gcc/testsuite/gfortran.dg/implicit_class_1.f90 @@ -4,6 +4,12 @@ ! ! Contributed by Reinhold Bader +! Add dump-fortran-original to check, if the patch preventing a gfortran +! segfault is working correctly. No cleanup needed, because the dump +! goes to stdout. +! { dg-options "-fdump-fortran-original" } +! { dg-prune-output "Namespace:.*" } + program upimp implicit class(foo) (a-b) implicit class(*) (c)