From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3117 invoked by alias); 29 May 2017 04:39:10 -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 3094 invoked by uid 89); 29 May 2017 04:39:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-detected-operating-system:Genre, recovery, enters, HX-detected-operating-system:recognized X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 29 May 2017 04:39:05 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38760) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1dFCSR-0003pF-II for gcc-patches@gnu.org; Mon, 29 May 2017 00:39:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFCSN-0007AE-NW for gcc-patches@gnu.org; Mon, 29 May 2017 00:39:06 -0400 Received: from mail-lf0-x243.google.com ([2a00:1450:4010:c07::243]:35080) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dFCSN-0007AA-Bc for gcc-patches@gnu.org; Mon, 29 May 2017 00:39:03 -0400 Received: by mail-lf0-x243.google.com with SMTP id 99so5061878lfu.2 for ; Sun, 28 May 2017 21:39:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=88QovcedD9veZIxfpzgxzG95leTzPIxjBX9iVFbBdIM=; b=UZrI4XOYYb6u911S9KZG5hvVrnqktRu4nkCiBkDg4u/xWZ0dfpChAEecELsN1o0bOZ PvugJ2SrajUidAAH5wGjMd1zHmoY4vwrimi5r6SS0VXLuJR5rd5YP6hMGtgIrNfGmxel esxtgnbHp7DORNnttZo89ZiMHG+2/O1M3mpDN36UggxaedMsalL5agW4DDM1kVATq0pe QMJ6tXWMitj9V3uiV/OY1Jvv5y2h/Jwe2aW8Vr9bWy0mqS7MmcMkXnAF5T3lmpyyR7Vx c0npeCBOf0k43aijoFw7YU086riJaw8Fw9y8RefFbMQQ1qGlR4P9BHjUsjCy9QsfVcT9 HcHQ== X-Gm-Message-State: AODbwcB4KiivKREG2piGp3E3s4R0EHCeqorZzrP288VEQoYJI5OHFIve //p3ZeMq+0b+x1s9XbViXg== X-Received: by 10.46.32.152 with SMTP id g24mr3837545lji.80.1496032741784; Sun, 28 May 2017 21:39:01 -0700 (PDT) Received: from [192.168.123.200] ([77.41.78.126]) by smtp.gmail.com with ESMTPSA id j89sm1745971lfk.67.2017.05.28.21.39.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 28 May 2017 21:39:00 -0700 (PDT) Subject: Re: [PATCH 1/5][GIMPLE FE] PR testsuite/80580. Handle missing labels in goto statements To: Richard Biener References: <9e3b3ee6-0c67-f974-c38e-7f9f25249914@gmail.com> Cc: gcc-patches , Prathamesh Kulkarni From: Mikhail Maltsev Message-ID: <377838f2-a96b-3147-20ba-d14b1a4bea40@gmail.com> Date: Mon, 29 May 2017 05:04:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------661D796A606E8F3D2A0870C4" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4010:c07::243 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg02149.txt.bz2 This is a multi-part message in MIME format. --------------661D796A606E8F3D2A0870C4 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 2965 Hi. Sorry for a long delay. On 02.05.2017 17:16, Richard Biener wrote: > Certainly an improvement. I suppose we can do better error recovery > for cases like > > if (1) > goto > else > goto bar; > > but I guess this is better than nothing. I think it's worth spending a bit more time to get this right. > > And yes, we use c_parser_error -- input_location should be ok but here > we just peek which may upset the parser. Maybe it works better > when consuming the token before issueing the error? Thus > > Index: gcc/c/gimple-parser.c > =================================================================== > --- gcc/c/gimple-parser.c (revision 247498) > +++ gcc/c/gimple-parser.c (working copy) > @@ -1315,8 +1315,8 @@ c_parser_gimple_if_stmt (c_parser *parse > loc = c_parser_peek_token (parser)->location; > c_parser_consume_token (parser); > label = c_parser_peek_token (parser)->value; > - t_label = lookup_label_for_goto (loc, label); > c_parser_consume_token (parser); > + t_label = lookup_label_for_goto (loc, label); > if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) > return; > } > I was more focused on cases with missing labels (i.e. 'label' is NULL), rather than cases with syntactically correct if-statements referencing undefined labels. And, frankly speaking, I'm not sure that swapping 'c_parser_consume_token' with 'lookup_label_for_goto' will help, because 'lookup_label_for_goto' already gets a 'loc' parameter. BTW, unfortunately GIMPLE FE does not handle undefined labels properly. I.e., this test case __GIMPLE() void foo() { bb_0: if (0) goto bb_0; else goto bb_1; } causes an ICE somewhere in build_gimple_cfg/cleanup_dead_labels. But this is a separate issue, of course. I attached a slightly modified patch, which incorporates your changes and also uses if (! c_parser_next_token_is (parser, CPP_NAME)) ... instead of label = c_parser_peek_token (parser)->value; ... if (!label) ... for better readability. This version correctly handles missing labels and semicolons in both branches of the 'if' statement. The only major problem, which I want to fix is error recovery in the following example: __GIMPLE() void foo() { if (1) goto lbl; else goto ; /* { dg-error "expected label" } */ } __GIMPLE() void bar() { if (1) goto lbl; else goto } /* { dg-error "expected label" } */ In this case GCC correctly diagnoses both errors. But if I swap these two functions so that 'bar' comes before 'foo', the error in 'foo' is not diagnosed. I did not dive into details, but my speculation is that the parser enters some strange state and skips 'foo' entirely (-fdump-tree-gimple contains only 'bar'). If I add another function after 'foo', it is handled correctly. Any ideas, why this could happen and how to improve error recovery in this case? -- Regards, Mikhail Maltsev --------------661D796A606E8F3D2A0870C4 Content-Type: text/plain; charset=UTF-8; name="wip1.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="wip1.patch" Content-length: 2632 ZGlmZiAtLWdpdCBhL2djYy9jL2dpbXBsZS1wYXJzZXIuYyBiL2djYy9jL2dp bXBsZS1wYXJzZXIuYwppbmRleCBlZDllN2M1Li40NGNhNzM4IDEwMDY0NAot LS0gYS9nY2MvYy9naW1wbGUtcGFyc2VyLmMKKysrIGIvZ2NjL2MvZ2ltcGxl LXBhcnNlci5jCkBAIC0xMzM2LDkgKzEzMzYsMTQgQEAgY19wYXJzZXJfZ2lt cGxlX2lmX3N0bXQgKGNfcGFyc2VyICpwYXJzZXIsIGdpbXBsZV9zZXEgKnNl cSkKICAgICB7CiAgICAgICBsb2MgPSBjX3BhcnNlcl9wZWVrX3Rva2VuIChw YXJzZXIpLT5sb2NhdGlvbjsKICAgICAgIGNfcGFyc2VyX2NvbnN1bWVfdG9r ZW4gKHBhcnNlcik7CisgICAgICBpZiAoISBjX3BhcnNlcl9uZXh0X3Rva2Vu X2lzIChwYXJzZXIsIENQUF9OQU1FKSkKKwl7CisJICBjX3BhcnNlcl9lcnJv ciAocGFyc2VyLCAiZXhwZWN0ZWQgbGFiZWwiKTsKKwkgIHJldHVybjsKKwl9 CiAgICAgICBsYWJlbCA9IGNfcGFyc2VyX3BlZWtfdG9rZW4gKHBhcnNlcikt PnZhbHVlOwotICAgICAgdF9sYWJlbCA9IGxvb2t1cF9sYWJlbF9mb3JfZ290 byAobG9jLCBsYWJlbCk7CiAgICAgICBjX3BhcnNlcl9jb25zdW1lX3Rva2Vu IChwYXJzZXIpOworICAgICAgdF9sYWJlbCA9IGxvb2t1cF9sYWJlbF9mb3Jf Z290byAobG9jLCBsYWJlbCk7CiAgICAgICBpZiAoISBjX3BhcnNlcl9yZXF1 aXJlIChwYXJzZXIsIENQUF9TRU1JQ09MT04sICJleHBlY3RlZCAlPDslPiIp KQogCXJldHVybjsKICAgICB9CkBAIC0xMzYwLDkgKzEzNjUsMTQgQEAgY19w YXJzZXJfZ2ltcGxlX2lmX3N0bXQgKGNfcGFyc2VyICpwYXJzZXIsIGdpbXBs ZV9zZXEgKnNlcSkKICAgICB7CiAgICAgICBsb2MgPSBjX3BhcnNlcl9wZWVr X3Rva2VuIChwYXJzZXIpLT5sb2NhdGlvbjsKICAgICAgIGNfcGFyc2VyX2Nv bnN1bWVfdG9rZW4gKHBhcnNlcik7CisgICAgICBpZiAoISBjX3BhcnNlcl9u ZXh0X3Rva2VuX2lzIChwYXJzZXIsIENQUF9OQU1FKSkKKwl7CisJICBjX3Bh cnNlcl9lcnJvciAocGFyc2VyLCAiZXhwZWN0ZWQgbGFiZWwiKTsKKwkgIHJl dHVybjsKKwl9CiAgICAgICBsYWJlbCA9IGNfcGFyc2VyX3BlZWtfdG9rZW4g KHBhcnNlciktPnZhbHVlOwotICAgICAgZl9sYWJlbCA9IGxvb2t1cF9sYWJl bF9mb3JfZ290byAobG9jLCBsYWJlbCk7CiAgICAgICBjX3BhcnNlcl9jb25z dW1lX3Rva2VuIChwYXJzZXIpOworICAgICAgZl9sYWJlbCA9IGxvb2t1cF9s YWJlbF9mb3JfZ290byAobG9jLCBsYWJlbCk7CiAgICAgICBpZiAoISBjX3Bh cnNlcl9yZXF1aXJlIChwYXJzZXIsIENQUF9TRU1JQ09MT04sICJleHBlY3Rl ZCAlPDslPiIpKQogCXJldHVybjsKICAgICB9CmRpZmYgLS1naXQgYS9nY2Mv dGVzdHN1aXRlL2djYy5kZy9naW1wbGVmZS1lcnJvci03LmMgYi9nY2MvdGVz dHN1aXRlL2djYy5kZy9naW1wbGVmZS1lcnJvci03LmMKbmV3IGZpbGUgbW9k ZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uN2Q1ZmYzNwotLS0gL2Rldi9udWxs CisrKyBiL2djYy90ZXN0c3VpdGUvZ2NjLmRnL2dpbXBsZWZlLWVycm9yLTcu YwpAQCAtMCwwICsxLDI3IEBACisvKiB7IGRnLWRvIGNvbXBpbGUgfSAqLwor LyogeyBkZy1vcHRpb25zICItZmdpbXBsZSIgfSAqLworCitfX0dJTVBMRSgp IHZvaWQgZm4xKCkKK3sKKyAgaWYgKDEpCisgICAgZ290bworICBlbHNlIC8q IHsgZGctZXJyb3IgImV4cGVjdGVkIGxhYmVsIiB9ICovCisgICAgZ290byBs Ymw7Cit9CisKK19fR0lNUExFKCkgdm9pZCBmbjIoKQoreworICBpZiAoMSkK KyAgICBnb3RvIGxibDsKKyAgZWxzZQorICAgIGdvdG8gOyAvKiB7IGRnLWVy cm9yICJleHBlY3RlZCBsYWJlbCIgfSAqLworfQorCitfX0dJTVBMRSgpIHZv aWQgZm4zKCkKK3sKKyAgaWYgKDEpCisgICAgZ290byBsYmw7CisgIGVsc2UK KyAgICBnb3RvCit9IC8qIHsgZGctZXJyb3IgImV4cGVjdGVkIGxhYmVsIiB9 ICovCisK --------------661D796A606E8F3D2A0870C4--