From patchwork Fri Aug 17 22:02:17 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dehao Chen X-Patchwork-Id: 178401 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]) by ozlabs.org (Postfix) with SMTP id 2E62F2C007F for ; Sat, 18 Aug 2012 08:02:45 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1345845766; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=lp4CGO+bOomxnxiyyjrcrxW9aqY=; b=aawhSKfl7fFE3v226/MmGsLQAXRWUBnu23vK+Ojg1FowUuzd/Mq6UB0zFwbdWM ZD2Ck8ApeK+j3LLoC4xJB75un/gyx3xJ+/0q5BnwNIsH2jyqtMiJacetXQOVnGSJ HeSrvqjCosKiIlNx4k6cM2geU/PwDyXnQNRpIeUabxzHw= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:X-Google-DKIM-Signature:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:X-System-Of-Record:X-Gm-Message-State:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=ARspdw7zPG9AEsYeKlob4O7cQc7zRteqYe+N83lBXaBN3Jd90tN5m1OWomyRaz 6NNRmTj9r8MBjvuSlgsGNTtJsiaX36rw0yd4bpKS6dVB2f9rdG854HirMxWRKqW7 xPt+NSsIXLzInUlXAhSiRmJNzCNBJY8j2Pk0ik+VIdDVY=; Received: (qmail 12896 invoked by alias); 17 Aug 2012 22:02:38 -0000 Received: (qmail 12859 invoked by uid 22791); 17 Aug 2012 22:02:32 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, TW_GC, TW_IB, TW_TM X-Spam-Check-By: sourceware.org Received: from mail-pb0-f47.google.com (HELO mail-pb0-f47.google.com) (209.85.160.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Aug 2012 22:02:19 +0000 Received: by pbcwy7 with SMTP id wy7so4068329pbc.20 for ; Fri, 17 Aug 2012 15:02:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-system-of-record:x-gm-message-state; bh=0ZjYsx9jewORGl1yV5Dj7PuE5DU8qDdAEyOw4L2/xSk=; b=JTIeKZVZgJOfqqnzobP/YLU6LegsCFRWKEV5cTMpzE/zkgj+YznlhmC2tUQ4z67UR7 izg3hd0PqZiu+SvEnpgMBpOpWElLuczGQVIOpd0peMzCQZecgSUEoguwihijKXLy/EP2 i7t7BuHqLF+sDUCc8UBaR37JyUJqC7J4p3a/1j9jG8+8m4a6Sg4NDV24VuBxZpYOtJ4Q YDsSbC0bfYvZuhTPtslpPF1dEfSp1KRuUk2Mim322EyH/HLoCTWIYPUIrxd3BzfJVyMQ 5lfTkzDfC9vEr6dkmBhd3wfiAS1yTC+ZoZBnhJjxtYtBFjSYyZxrd3Ht/0BVEKksa4Nt oMag== Received: by 10.68.224.161 with SMTP id rd1mr15082274pbc.133.1345240938370; Fri, 17 Aug 2012 15:02:18 -0700 (PDT) MIME-Version: 1.0 Received: by 10.68.224.161 with SMTP id rd1mr15082244pbc.133.1345240938073; Fri, 17 Aug 2012 15:02:18 -0700 (PDT) Received: by 10.68.189.100 with HTTP; Fri, 17 Aug 2012 15:02:17 -0700 (PDT) In-Reply-To: <502E6774.8050609@redhat.com> References: <50228C38.5080703@redhat.com> <502294A1.3060800@redhat.com> <50243480.7090803@redhat.com> <50254A50.8070208@redhat.com> <50255B35.9020705@redhat.com> <50258712.4070002@redhat.com> <502E6774.8050609@redhat.com> Date: Fri, 17 Aug 2012 15:02:17 -0700 Message-ID: Subject: Re: [PATCH] Set correct source location for deallocator calls From: Dehao Chen To: Richard Henderson Cc: Jason Merrill , Richard Guenther , gcc-patches@gcc.gnu.org, David Li X-System-Of-Record: true X-Gm-Message-State: ALoCoQm7ZdxuolbiCeSuuKFbBroIKXivrEQoAIzRTC+qAAdF6mIHFA+Kqh6Kdh4bW4tCDEbTn7i5r+8nVddndPb75GVnihD4aO83/fJGFNdh6KmOlgiyu+gblVtF4/O5f9M3bddXEeZ1P1IOpupYE+PGZxj19xx9rMdcfQtvUIG62P/b5Wnjsht/2W4MKJteoqo2oC7H3yVu X-IsSubscribed: yes 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 Hi, Richard, Thanks for the review. I've addressed most of the issues except the java unittest (see comments below). The new patch is attached in the end of this email. Thanks, Dehao On Fri, Aug 17, 2012 at 8:47 AM, Richard Henderson wrote: > On 2012-08-10 20:38, Dehao Chen wrote: >> + // { dg-final { scan-assembler "1 28 0" } } > > This test case isn't going to work except with dwarf2, and with gas. > You can use -dA so that you can scan for file.c:line. There are > other examples in the testsuite. Done. > > This doesn't belong in guality. It belongs in g++.dg/debug/. Done. > It would be nice if you could add a java testcase to see that it > doesn't regress. I spend a whole day working on this, but find it very difficult to add such a java test because: * First, libjava testsuits are all runtime tests, i.e., it compiles the byte code to native code, execute it, and compares the output to expected output. There is no way to scan the assembly. * Though there is a way to derive the line number at runtime in java (using Exception().getStackTrace()), this method only works on VM, and the gcj generated native code does not get the lineno. Any suggestions on this? > >> ! record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, >> tree label, >> ! location_t location) > > BTW, for the future, please fix your mailer to not wrap lines. Okay, I'll try. The problem is that we have to send mail in plain txt. And in "plain text mode" gmail wraps each line to 80 characters and wouldn't allow you change that... > I'll quibble with the wording here. It reads as if we want to force > all calls to have UNKNOWN_LOC, whereas all we want is to prevent any > calls that already have UNKNOWN_LOC from gaining input_loc via gimplify_call_expr. Done. New Patch: gcc/ChangeLog * tree-eh.c (goto_queue_node): New field. (record_in_goto_queue): New parameter. (record_in_goto_queue_label): New parameter. (lower_try_finally_dup_block): New parameter. (maybe_record_in_goto_queue): Update source location. (lower_try_finally_copy): Likewise. (honor_protect_cleanup_actions): Likewise. * gimplify.c (gimplify_expr): Reset the location to unknown. gcc/testsuite/ChangeLog 2012-08-07 Dehao Chen * g++.dg/debug/dwarf2/deallocator.C: New test. Index: gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C =================================================================== --- gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) +++ gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) @@ -0,0 +1,33 @@ +// Test that debug info generated for auto-inserted deallocator is +// correctly attributed. +// This patch scans for the lineno directly from assembly, which may +// differ between different architectures. Because it mainly tests +// FE generated debug info, without losing generality, only x86 +// assembly is scanned in this test. +// { dg-do compile { target { i?86-*-* x86_64-*-* } } } +// { dg-options "-O2 -fno-exceptions -g -dA" } + +struct t { + t (); + ~t (); + void foo(); + void bar(); +}; + +int bar(); + +void foo(int i) +{ + for (int j = 0; j < 10; j++) + { + t test; + test.foo(); + if (i + j) + { + test.bar(); + return; + } + } + return; +} +// { dg-final { scan-assembler "1 28 0" } } Index: gcc/tree-eh.c =================================================================== --- gcc/tree-eh.c (revision 190209) +++ gcc/tree-eh.c (working copy) @@ -321,6 +321,7 @@ struct goto_queue_node { treemple stmt; + location_t location; gimple_seq repl_stmt; gimple cont_stmt; int index; @@ -560,7 +561,8 @@ record_in_goto_queue (struct leh_tf_state *tf, treemple new_stmt, int index, - bool is_label) + bool is_label, + location_t location) { size_t active, size; struct goto_queue_node *q; @@ -583,6 +585,7 @@ memset (q, 0, sizeof (*q)); q->stmt = new_stmt; q->index = index; + q->location = location; q->is_label = is_label; } @@ -590,7 +593,8 @@ TF is not null. */ static void -record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label) +record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label, + location_t location) { int index; treemple temp, new_stmt; @@ -629,7 +633,7 @@ since with a GIMPLE_COND we have an easy access to the then/else labels. */ new_stmt = stmt; - record_in_goto_queue (tf, new_stmt, index, true); + record_in_goto_queue (tf, new_stmt, index, true, location); } /* For any GIMPLE_GOTO or GIMPLE_RETURN, decide whether it leaves a try_finally @@ -649,19 +653,22 @@ { case GIMPLE_COND: new_stmt.tp = gimple_op_ptr (stmt, 2); - record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt), + EXPR_LOCATION (*new_stmt.tp)); new_stmt.tp = gimple_op_ptr (stmt, 3); - record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt), + EXPR_LOCATION (*new_stmt.tp)); break; case GIMPLE_GOTO: new_stmt.g = stmt; - record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt), + gimple_location (stmt)); break; case GIMPLE_RETURN: tf->may_return = true; new_stmt.g = stmt; - record_in_goto_queue (tf, new_stmt, -1, false); + record_in_goto_queue (tf, new_stmt, -1, false, gimple_location (stmt)); break; default: @@ -866,13 +873,19 @@ Make sure to record all new labels found. */ static gimple_seq -lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state) +lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state, + location_t loc) { gimple region = NULL; gimple_seq new_seq; + gimple_stmt_iterator gsi; new_seq = copy_gimple_seq_and_replace_locals (seq); + for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi)) + if (gimple_location (gsi_stmt (gsi)) == UNKNOWN_LOCATION) + gimple_set_location (gsi_stmt (gsi), loc); + if (outer_state->tf) region = outer_state->tf->try_finally_expr; collect_finally_tree_1 (new_seq, region); @@ -967,7 +980,8 @@ gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); } else if (this_state) - finally = lower_try_finally_dup_block (finally, outer_state); + finally = lower_try_finally_dup_block (finally, outer_state, + UNKNOWN_LOCATION); finally_may_fallthru = gimple_seq_may_fallthru (finally); /* If this cleanup consists of a TRY_CATCH_EXPR with TRY_CATCH_IS_CLEANUP @@ -1184,7 +1198,7 @@ if (tf->may_fallthru) { - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, tf_loc); lower_eh_constructs_1 (state, &seq); gimple_seq_add_seq (&new_stmt, seq); @@ -1200,7 +1214,7 @@ if (eh_else) seq = gimple_eh_else_e_body (eh_else); else - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, tf_loc); lower_eh_constructs_1 (state, &seq); emit_post_landing_pad (&eh_seq, tf->region); @@ -1250,7 +1264,7 @@ x = gimple_build_label (lab); gimple_seq_add_stmt (&new_stmt, x); - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, q->location); lower_eh_constructs_1 (state, &seq); gimple_seq_add_seq (&new_stmt, seq); Index: gcc/gimplify.c =================================================================== --- gcc/gimplify.c (revision 190209) +++ gcc/gimplify.c (working copy) @@ -7434,6 +7434,15 @@ gimple_seq eval, cleanup; gimple try_; + /* Calls to destructors are generated automatically in FINALL/CATCH + block. They should have location as UNKNOWN_LOCATION. However, + gimplify_call_expr will reset these call stmts to input_location + if it finds stmt's location is unknown. To prevent resetting for + destructors, we set the input_location to unknown. + Note that this only affects the destructor calls in FINALL/CATCH + block, and will automatically reset to its original value by the + end of gimplify_expr. */ + input_location = UNKNOWN_LOCATION; eval = cleanup = NULL; gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval); gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);