From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7951 invoked by alias); 29 Oct 2015 14:18:30 -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 7939 invoked by uid 89); 29 Oct 2015 14:18:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 29 Oct 2015 14:18:25 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 4BBE5C0A524A; Thu, 29 Oct 2015 14:18:24 +0000 (UTC) Received: from localhost.localdomain (vpn1-4-201.ams2.redhat.com [10.36.4.201]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9TEILg6014374; Thu, 29 Oct 2015 10:18:22 -0400 Subject: Re: [PATCH v2] [PR debug/67192] Fix C loops' back-jump location To: Andreas Arnez , Joseph Myers , Richard Henderson References: Cc: gcc-patches@gcc.gnu.org, David Malcolm , =?UTF-8?B?TWFudWVsIEzDs3Blei1JYsOhw7Fleg==?= , Patrick Palka , Andreas Krebbel From: Bernd Schmidt Message-ID: <56322AAD.7080002@redhat.com> Date: Thu, 29 Oct 2015 14:24:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg03172.txt.bz2 On 10/23/2015 11:14 AM, Andreas Arnez wrote: > This patch adds a new parameter to c_finish_loop that expclitly > specifies the location to be used for the loop iteration. All calls to > c_finish_loop are adjusted accordingly. I think the general principle of this is probably ok. > + bool is_braced = c_parser_next_token_is (parser, CPP_OPEN_BRACE); > body = c_parser_c99_block_statement (parser); > + location_t iter_loc = is_braced ? input_location : loc; > > token_indent_info next_tinfo > = get_token_indent_info (c_parser_peek_token (parser)); > warn_for_misleading_indentation (while_tinfo, body_tinfo, next_tinfo); > > - c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true); > + c_finish_loop (loc, iter_loc, cond, NULL, body, c_break_label, > + c_cont_label, true); I'm not entirely sure I understand what the is_braced logic is trying to achieve. I tried the patch out with the debugger on the testcase you provided, and I get slightly strange behaviour in f2: Starting program: /local/src/egcs/bwork-git/gcc/a.out Breakpoint 7, f2 () at pr67192.c:32 32 if (last ()) (gdb) cont Continuing. Breakpoint 6, f2 () at pr67192.c:31 31 while (1) (gdb) i.e. the breakpoint on the code inside the loop is reached before the while statement itself. This may be the expected behaviour with your patch, but I'm not sure it's really desirable for debugging. In f4 placing a breakpoint on the while (1) just places it on the if (last ()) line. The same behaviour appears to occur for both while loops with the system gcc-4.8.5. So I think there are some strange inconsistencies going on here, and for debugging the old behaviour may well be better. I'd suggest you commit your original patch to fix the misleading-indent problem while we sort this out. Bernd