From patchwork Mon Jul 27 14:29:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 500449 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 2E397140216 for ; Tue, 28 Jul 2015 00:30:25 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=EdX/+58D; 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:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=gP2qD7OQm3qoDhl1 sAD6QWHN7vuQrwq63H5BGuN8q5uOS+es8aDBVmWKyQPK+Vt7RUvrGNfVFqYmMqY1 W3JWuVsDMWv7Gvssuc06W1qk8thU6tCrntCRLO0YKA1iy0nv8J+TKzHQyVghhq4y FsARwmSHan1eex90wQtCP02PEPA= 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:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=q1BM/8ZR7TvFDl+LFbHS+d +WkZ0=; b=EdX/+58Dbdi0tSmC2UkG0uwkKeuQ4Rr1XRbR5NUiCboxrVOEzozKyS 3t0PJmKb7Au5KSU3/mJSFt73Ul1m5u9tU0w/paQVr00oS8iGO6IoQLjUPnkHhmD1 TUDFR7tRaTpDbN07W5dqKhI1XF/JgZdtWOs0cmEXtmMLtPe3oqFy0= Received: (qmail 34722 invoked by alias); 27 Jul 2015 14:30:15 -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 34666 invoked by uid 89); 27 Jul 2015 14:30:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Jul 2015 14:30:08 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZJjPn-0004mE-1m from Thomas_Schwinge@mentor.com ; Mon, 27 Jul 2015 07:30:03 -0700 Received: from feldtkeller.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Mon, 27 Jul 2015 15:29:59 +0100 From: Thomas Schwinge To: , CC: Tobias Burnus , David Malcolm , Mikael Morin , Ilmir Usmanov Subject: Re: Fix logic error in Fortran OpenACC parsing In-Reply-To: <554C9CDF.7000605@samsung.com> References: <5536936F.8090004@gmail.com> <1430265776-8157-1-git-send-email-dmalcolm@redhat.com> <1430265776-8157-3-git-send-email-dmalcolm@redhat.com> <5540CA39.7070308@sfr.fr> <1430854683.32584.275.camel@surprise> <87d22endfn.fsf@schwinge.name> <554C9CDF.7000605@samsung.com> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Mon, 27 Jul 2015 16:29:56 +0200 Message-ID: <87d1zdzna3.fsf@schwinge.name> MIME-Version: 1.0 Hi! On Fri, 8 May 2015 14:24:15 +0300, Ilmir Usmanov wrote: > On 06.05.2015 14:38, Thomas Schwinge wrote: > > On Tue, 5 May 2015 15:38:03 -0400, David Malcolm wrote: > >> On Wed, 2015-04-29 at 14:10 +0200, Mikael Morin wrote: > >>> Le 29/04/2015 02:02, David Malcolm a écrit : > >>>> diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c > >>>> index 2c7c554..30e4eab 100644 > >>>> --- a/gcc/fortran/parse.c > >>>> +++ b/gcc/fortran/parse.c > >>>> @@ -4283,7 +4283,7 @@ parse_oacc_structured_block (gfc_statement acc_st) > >>>> unexpected_eof (); > >>>> else if (st != acc_end_st) > >>>> gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); > >>>> - reject_statement (); > >>>> + reject_statement (); > >>>> } > >>>> while (st != acc_end_st); > >>>> > >>> I think this one is a bug; there should be braces around 'gfc_error' and > >>> 'reject_statement'. > If 'st' is 'acc_end_st', as it shall be, the statement is rejected. So, > this is a bug. > > > > >>> At least that's the pattern in 'parse_oacc_loop', and how the > >>> 'unexpected_statement' function is used. > >> FWIW, Jeff had approved that patch, so I've committed the patch to trunk > >> (as r222823), making the indentation reflect the block structure. > >> > >> Thomas: should the > >> reject_statement (); > >> call in the above be guarded by the > >> else if (st != acc_end_st) > >> clause? > > Indeed, this seems to be a bug that has been introduced very early in the > > OpenACC Fortran front end development -- see how the > > parse_oacc_structured_block function evolved in the patches posted in > > > > and following (Ilmir, CCed "just in case"). I also see that the > > corresponding OpenMP code, parse_omp_structured_block, just calls > > unexpected_statement, which Ilmir's initial patch also did, but at some > > point, he then changed this to the current code: gfc_error followed by > > reject_statement, as cited above -- I would guess for the reason to get a > > better error message? (Tobias, should this thus also be done for OpenMP, > > and/or extend unexpected_statement accordingly?) > That's true. > I've checked abandoned openacc-1_0-branch and I used > unexpected_statement there (there still odd *_acc_* naming presents > instead of new-and-shiny *_oacc_* one), but, as you mentioned, I've > changed this for better error reporting... and introduced the bug. > > > > > And then, I'm a bit confused: is it "OK" that despite this presumed logic > > error, which affects all (?) valid executions of this parsing code, we're > > not running into any issues with the OpenACC Fortran front end test > > cases? > I think, this is OK, since this is an !$ACC END _smth_ statement and it > shall not present in the AST. So, it is abandoned later anyway ;) (if I > remember correctly, during gfc_clear_new_st call). Although the bug does > not affect the logic, it is still a bug. > > > OK for trunk? > From my point of view, OK. Thanks for your review! Nobody else had any comments, so I have now committed my patch to trunk in r226246: commit 1ed4ddb2615c807c239e1b3eb214655a4cc87f1a Author: tschwinge Date: Mon Jul 27 14:26:41 2015 +0000 Fix logic error in Fortran OpenACC parsing gcc/fortran/ * parse.c (parse_oacc_structured_block): Fix logic error. Reported by Mikael Morin . git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@226246 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/fortran/ChangeLog | 5 +++++ gcc/fortran/parse.c | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) Grüße, Thomas diff --git gcc/fortran/ChangeLog gcc/fortran/ChangeLog index 0ed6b9b..e5b7681 100644 --- gcc/fortran/ChangeLog +++ gcc/fortran/ChangeLog @@ -1,3 +1,8 @@ +2015-07-27 Thomas Schwinge + + * parse.c (parse_oacc_structured_block): Fix logic error. + Reported by Mikael Morin . + 2015-07-24 Mikael Morin PR fortran/64986 diff --git gcc/fortran/parse.c gcc/fortran/parse.c index 45ad63f..04b4c80 100644 --- gcc/fortran/parse.c +++ gcc/fortran/parse.c @@ -4383,8 +4383,10 @@ parse_oacc_structured_block (gfc_statement acc_st) if (st == ST_NONE) unexpected_eof (); else if (st != acc_end_st) - gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); - reject_statement (); + { + gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); + reject_statement (); + } } while (st != acc_end_st);