From patchwork Fri Feb 7 20:42:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mikael Morin X-Patchwork-Id: 318193 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 193382C00A4 for ; Sat, 8 Feb 2014 07:43:15 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; q=dns; s= default; b=iKRXkRXWnlOIf2ZOsPpofzKraCOXOvrzwqt2kE80vc785v/uxeGHQ S5JKIyFK3pDtYLuUZ3k+3tgYu6oUZbwwUqBTGGotkb0bXdWqMAe+6hK+6w4lED3D OrKw5ozLRJatRy3PZH6tNtn09JrMONwOrdNg4m6yJtsVEEqbMsMCkQ= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; s=default; bh=AyWqiY14qIkyfA+FY2ij/+kCo78=; b=srWdcO5RGRuhhH4IAbyI3rOsl7wj AxfODuH6Ys5YjRKwpWssgRmyoIh8h0/nbTeppWZf1YYy9JruajZ5zRKSpLYHMKGD A5iwXousqcu2Zrk0Y/DgXR10ZNFesd7tlhjUlAhB89a9X5yxXxJDBDVXhaEA/9+A nrR/wnIxuY7XhfM= Received: (qmail 23090 invoked by alias); 7 Feb 2014 20:43:08 -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 23063 invoked by uid 89); 7 Feb 2014 20:43:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 3 recipients X-HELO: smtp21.services.sfr.fr Received: from smtp21.services.sfr.fr (HELO smtp21.services.sfr.fr) (93.17.128.3) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Feb 2014 20:43:06 +0000 Received: from filter.sfr.fr (localhost [127.0.0.1]) by msfrf2119.sfr.fr (SMTP Server) with ESMTP id BCA1A70000AE; Fri, 7 Feb 2014 21:43:03 +0100 (CET) Received: from [192.168.0.16] (did75-4-82-66-46-21.fbx.proxad.net [82.66.46.21]) by msfrf2119.sfr.fr (SMTP Server) with ESMTP id 68F2B70000AB; Fri, 7 Feb 2014 21:43:03 +0100 (CET) X-SFR-UUID: 20140207204303429.68F2B70000AB@msfrf2119.sfr.fr Message-ID: <52F54545.4090009@sfr.fr> Date: Fri, 07 Feb 2014 21:42:45 +0100 From: Mikael Morin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Janus Weil CC: gfortran , gcc-patches Subject: Re: [Patch, Fortran] PR 58470: [4.9 Regression] [OOP] ICE on invalid with FINAL procedure and type extension References: <52F3EDE4.8010603@sfr.fr> In-Reply-To: X-IsSubscribed: yes Le 06/02/2014 23:40, Janus Weil a écrit : > Hi Mikael, > > thanks for your comments ... > >>> attached is a small patch which fixes an ICE-on-invalid regression >>> with finalization. In the PR, Dominique objected to the patch, but I >>> think it's the correct thing to do after all. The line that I'm >>> removing was added in a patch authored by Tobias and myself. I suspect >>> it was added to work around some other problem in the finalization >>> implementation, and there is no evidence it's actually needed. >>> >>> The patch regtests cleanly on x86_64-unknown-linux-gnu. Ok for trunk? >>> >> Wait a bit; let's try to understand the problem. >> >> Normally I would say calling gfc_is_finalizable here in >> resolve_fl_derived0 is harmless because gfc_resolve_finalizers has been >> called before in resolve_fl_derived. >> BUT: >> resolve_fl_derived0 recurses on its parent type, while >> gfc_resolve_finalizers doesn't; and in this case we end up recursing on >> type "cfml" whose finalizers haven't been resolved yet. > > Yes, that's more or less what happens. > > And the real problem is that gfc_is_finalzable already generates the > finalization wrapper (via gfc_find_derived_vtab -> > generate_finalizaton_wrapper) before we have checked that the > finalizer is actually valid (which is what gfc_resolve_finalizers > does). Once we get into gfc_resolve_finalizers, it is fooled to > believe that the finalizer has already been resolved and therefore > skips the checks and produces no error message. > > >> Now whether your patch is the right thing to do... I'm a bit skeptical >> about removing the one use of gfc_is_finalizable in resolve.c. > > Well, all others occurrences of 'gfc_is_finalizable' are in trans*, so > this is the only one that comes too early. > Yeah OK. gfc_is_finalizable is almost a no-op anyway (assuming the vtab has been generated at resolution stage). I suggest the following additional patch to make sure that the finalization wrapper is never generated without prior resolution (in this case, it replaces one ICE with another); maybe add gcc_assert to make it clear that fini->proc_tree should be set at this point. Patch is OK anyway. Thanks. Mikael { fini_elem = fini; diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c index d3569fd..20488c0 100644 --- a/gcc/fortran/class.c +++ b/gcc/fortran/class.c @@ -1880,8 +1880,6 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns, for (fini = derived->f2k_derived->finalizers; fini; fini = fini->next) { - if (!fini->proc_tree) - fini->proc_tree = gfc_find_sym_in_symtree (fini->proc_sym); if (fini->proc_tree->n.sym->attr.elemental)