From patchwork Sun Oct 13 22:17:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 283124 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id ED43B2C035A for ; Mon, 14 Oct 2013 09:17:27 +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:content-type; q=dns; s=default; b=jsVQQO5YkPVV4cSpORXjlRYuKeavponQ7J9SDTtR3kP uyaWyV+K8+BzLDRzbxIWdaXb3we9e4CrXFuC7F9fL5XXkpzKzM1M+9zvQwvkazyo akNShPb43gGQHOaljaI9hEp+eTIIxqIU20uezYkgrATFaANHh7WJF3apJwSGZtic = 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:content-type; s=default; bh=MZYZrWwkTbJSaHCXhLU+J+zHyGs=; b=et7pt7rgvvR3Zhqka n1Zq5Gg9pvs4siJHwvr6H3yEdgCFtttiOHHHUOGLzWaJ/jKOWP/PfOCtPRtRd157 133Aq4tkWvcqRCqjBy9P+Xp/7m46swU2VClYuGGOxWCgOaUVFhKjzfp38tyRofPi WTHprhnOrNsB1r+dVP4JwfRN7o= Received: (qmail 12613 invoked by alias); 13 Oct 2013 22:17:20 -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 12598 invoked by uid 89); 13 Oct 2013 22:17:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 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; Sun, 13 Oct 2013 22:17:15 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1VVTyF-0005Gj-IQ from Tom_deVries@mentor.com ; Sun, 13 Oct 2013 15:17:07 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Sun, 13 Oct 2013 15:17:07 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.2.247.3; Sun, 13 Oct 2013 23:17:04 +0100 Message-ID: <525B1BDD.2000306@mentor.com> Date: Mon, 14 Oct 2013 00:17:01 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-Version: 1.0 To: Richard Earnshaw CC: "gcc-patches@gcc.gnu.org" , Ulrich Weigand , Jakub Jelinek Subject: [PATCH, ARM] Fix line number data for PIC register setup code Richard, This patch fixes line number data for the PIC register setup code for ARM, resulting in 174 removed FAILs for the gdb testsuite with -fPIC. Consider break.c (minimized from gdb/testsuite/gdb.base/break.c): ... 1 void *v; 2 void a (void *x) { } 3 void b (void) { } 4 5 int 6 main (int argc) 7 { 8 if (argc == 12345) 9 { 10 a (v); 11 return 1; 12 } 13 b (); 14 15 return 0; 16 } ... We compile like this with -fPIC: ... $ arm-none-linux-gnueabi-gcc break.c -g -fPIC ... and run to a breakpoint in main: ... (gdb) b main Breakpoint 1 at 0x8410: file break.c, line 7. (gdb) c Continuing. Breakpoint 1, main (argc=1) at break.c:7 7 { ... The breakpoint should be at line number 8, the first line with user code. The assembly for the start of main looks like this: ... main: .LFB2: .loc 1 7 0 .cfi_startproc @ args = 0, pretend = 0, frame = 8 @ frame_needed = 1, uses_anonymous_args = 0 stmfd sp!, {fp, lr} .cfi_def_cfa_offset 8 .cfi_offset 11, -8 .cfi_offset 14, -4 add fp, sp, #4 .cfi_def_cfa 11, 4 sub sp, sp, #8 str r0, [fp, #-8] .loc 1 7 0 ldr r2, .L6 .LPIC0: add r2, pc, r2 .loc 1 8 0 ldr r1, [fp, #-8] ldr r3, .L6+4 cmp r1, r3 bne .L4 .loc 1 10 0 ... From the point of view of the debugger, in the presence of .loc info, the prologue is the code in between the 2 first .loc markers. See this comment in gdb/arm-tdep.c:arm_skip_prologue: ... /* GCC always emits a line note before the prologue and another one after, even if the two are at the same address or on the same line. Take advantage of this so that we do not need to know every instruction that might appear in the prologue. ... So gdb breaks at line 7, because it considers the prologue the code between the first 2 .locs, and the user code for main to start at the second .loc, which has line number 7. The code at the second .loc is the PIC register setup code, generated by require_pic_register: ... .loc 1 7 0 ldr r2, .L6 .LPIC0: add r2, pc, r2 ... The second .loc is emitted by gcc, because it's the first insn after NOTE_INSNS_FUNCTION_BEG (which sets final.c:force_source_line in final.c:final_scan_insn). The PIC register setup code is emitted after NOTE_INSNS_FUNCTION_BEG, because it uses the insert_insn_on_edge mechanism, and the corresponding insertion in cfgexpand.c:gimple_expand_cfg takes care to insert the code after the parm_birth_insn: ... /* Avoid putting insns before parm_birth_insn. */ if (e->src == ENTRY_BLOCK_PTR && single_succ_p (ENTRY_BLOCK_PTR) && parm_birth_insn) { rtx insns = e->insns.r; e->insns.r = NULL_RTX; emit_insn_after_noloc (insns, parm_birth_insn, e->dest); } ... And in the case for this test-case, parm_birth_insn is the NOTE_INSNS_FUNCTION_BEG. In summary, the problem seems to be caused by a discrepancy between: - the line number of the PIC register setup code (prologue_location), and - the location of the PIC register setup code (after NOTE_INSN_FUNCTION_BEG). The problem can be reproduced using Ulrich's example here ( http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01570.html ), but though related it's not the same problem as explained there. There the wrong line number of the breakpoint was after the first user line. Here the wrong line number of the breakpoint is before the first user line. We can see from that email that this problem was not present after the fix. The problem was introduced by Jakub's fix for PR47028 (Insert entry edge insertions after parm_birth_insn instead of at the beginning of first bb), 12 days after Ulrich's fix. This patch makes sure we emit insertions scheduled for the first real BB before NOTE_INSN_FUNCTION_BEG. As a consequence, it moves the PIC register setup code to before the NOTE_INSN_FUNCTION_BEG. This removes the second .loc, and the breakpoint of main ends up at line 8. Bootstrapped and regtested on x86_64 (ada inclusive), no issues found. Tested gdb with target arm-none-linux-gnueabi and CFLAGS_FOR_TARGET=-fPIC. The patch removes 174 FAILs. Re-testing gcc with target arm-none-linux-gnueabi atm. OK for trunk? Thanks, - Tom 2013-10-13 Tom de Vries * cfgexpand.c (gimple_expand_cfg): Don't commit insertions after NOTE_INSN_FUNCTION_BEG. * gcc.target/arm/require-pic-register-loc.c: New test. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c (revision 421892) +++ gcc/cfgexpand.c (working copy) @@ -4618,14 +4618,19 @@ gimple_expand_cfg (void) if (e->insns.r) { rebuild_jump_labels_chain (e->insns.r); - /* Avoid putting insns before parm_birth_insn. */ + /* Put insns after parm birth, but before + NOTE_INSNS_FUNCTION_BEG. */ if (e->src == ENTRY_BLOCK_PTR && single_succ_p (ENTRY_BLOCK_PTR) && parm_birth_insn) { rtx insns = e->insns.r; e->insns.r = NULL_RTX; - emit_insn_after_noloc (insns, parm_birth_insn, e->dest); + if (NOTE_P (parm_birth_insn) + && NOTE_KIND (parm_birth_insn) == NOTE_INSN_FUNCTION_BEG) + emit_insn_before_noloc (insns, parm_birth_insn, e->dest); + else + emit_insn_after_noloc (insns, parm_birth_insn, e->dest); } else commit_one_edge_insertion (e); Index: gcc/testsuite/gcc.target/arm/require-pic-register-loc.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.target/arm/require-pic-register-loc.c (revision 0) @@ -0,0 +1,29 @@ +/* { dg-do compile } */ +/* { dg-options "-g -fPIC" } */ + +void *v; +void a (void *x) { } +void b (void) { } + /* line 7. */ +int /* line 8. */ +main (int argc) /* line 9. */ +{ /* line 10. */ + if (argc == 12345) /* line 11. */ + { + a (v); + return 1; + } + b (); + + return 0; +} + +/* { dg-final { scan-assembler-not "\.loc 1 7 0" } } */ +/* { dg-final { scan-assembler-not "\.loc 1 8 0" } } */ +/* { dg-final { scan-assembler-not "\.loc 1 9 0" } } */ + +/* The loc at the start of the prologue. */ +/* { dg-final { scan-assembler-times "\.loc 1 10 0" 1 } } */ + +/* The loc at the end of the prologue, with the first user line. */ +/* { dg-final { scan-assembler-times "\.loc 1 11 0" 1 } } */