From: Dehao Chen <dehao@google.com>
To: Mark Wielaard <mark@klomp.org>
Cc: Bryce McKinlay <bmckinlay@gmail.com>,
Andrew Haley <aph@redhat.com>,
Richard Henderson <rth@redhat.com>,
Jason Merrill <jason@redhat.com>,
Richard Guenther <richard.guenther@gmail.com>,
gcc-patches@gcc.gnu.org, David Li <davidxl@google.com>,
java@gcc.gnu.org
Subject: Re: [PATCH] Set correct source location for deallocator calls
Date: Sat, 08 Sep 2012 21:42:00 -0000 [thread overview]
Message-ID: <CAO2gOZVhW2+VfKU8+n2qrcKr9fgM1G4bRyUTUB_0DATH9eEW6w@mail.gmail.com> (raw)
In-Reply-To: <1346839095.9368.1.camel@springer.wildebeest.org>
[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]
Hi,
I've added a libjava unittest which verifies that this patch will not
break Java debug info. I've also incorporated Richard's review in the
previous mail. Attached is the new patch, which passed bootstrap and
all gcc/libjava testsuites on x86.
Is it ok for trunk?
Thanks,
Dehao
gcc/ChangeLog:
2012-09-08 Dehao Chen <dehao@google.com>
* 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-09-08 Dehao Chen <dehao@google.com>
* g++.dg/debug/dwarf2/deallocator.C: New test.
libjava/ChangeLog:
2012-09-08 Dehao Chen <dehao@google.com>
* testsuite/libjava.lang/sourcelocation.java: New cases.
* testsuite/libjava.lang/sourcelocation.out: New cases.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8676 bytes --]
Index: libjava/testsuite/libjava.lang/sourcelocation.java
===================================================================
--- libjava/testsuite/libjava.lang/sourcelocation.java (revision 0)
+++ libjava/testsuite/libjava.lang/sourcelocation.java (revision 0)
@@ -0,0 +1,18 @@
+/* This test should test the source location attribution.
+ We print the line number of different parts of the program to make sure
+ that the source code attribution is correct.
+ To make this test pass, one need to have up-to-date addr2line installed
+ to parse the dwarf4 data format.
+*/
+public class sourcelocation {
+ public static void main(String args[]) {
+ try {
+ System.out.println(new Exception().getStackTrace()[0].getLineNumber());
+ throw new Exception();
+ } catch (Exception e) {
+ System.out.println(new Exception().getStackTrace()[0].getLineNumber());
+ } finally {
+ System.out.println(new Exception().getStackTrace()[0].getLineNumber());
+ }
+ }
+}
Index: libjava/testsuite/libjava.lang/sourcelocation.out
===================================================================
--- libjava/testsuite/libjava.lang/sourcelocation.out (revision 0)
+++ libjava/testsuite/libjava.lang/sourcelocation.out (revision 0)
@@ -0,0 +1,3 @@
+10
+13
+15
Index: libjava/testsuite/libjava.lang/sourcelocation.jar
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: libjava/testsuite/libjava.lang/sourcelocation.jar
___________________________________________________________________
Added: svn:mime-type
+ application/octet-stream
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 "deallocator.C:28" } }
Index: gcc/tree-eh.c
===================================================================
--- gcc/tree-eh.c (revision 191083)
+++ gcc/tree-eh.c (working copy)
@@ -321,6 +321,7 @@ static bitmap eh_region_may_contain_throw_map;
struct goto_queue_node
{
treemple stmt;
+ location_t location;
gimple_seq repl_stmt;
gimple cont_stmt;
int index;
@@ -560,7 +561,8 @@ static void
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 @@ record_in_goto_queue (struct leh_tf_state *tf,
memset (q, 0, sizeof (*q));
q->stmt = new_stmt;
q->index = index;
+ q->location = location;
q->is_label = is_label;
}
@@ -590,7 +593,8 @@ record_in_goto_queue (struct leh_tf_state *tf,
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 @@ static void
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 @@ maybe_record_in_goto_queue (struct leh_state *stat
{
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 @@ frob_into_branch_around (gimple tp, eh_region regi
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 @@ honor_protect_cleanup_actions (struct leh_state *o
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 @@ lower_try_finally_copy (struct leh_state *state, s
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 @@ lower_try_finally_copy (struct leh_state *state, s
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 @@ lower_try_finally_copy (struct leh_state *state, s
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 191083)
+++ gcc/gimplify.c (working copy)
@@ -7429,6 +7429,15 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gi
gimple_seq eval, cleanup;
gimple try_;
+ /* Calls to destructors are generated automatically in FINALLY/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 FINALLY/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);
next prev parent reply other threads:[~2012-09-08 21:42 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAO2gOZXfnETUe4wqjT7p6fd61hXreu9PDfqKxNz+HxpE0E7K0g@mail.gmail.com>
[not found] ` <CAO2gOZX_hZ1m0xKvxUg+6gWTOX4V5ok-vvCEg3Vhfpt=qXj8-g@mail.gmail.com>
[not found] ` <CAFiYyc00wud5Oa3k51ntm8KeZHZnfm4rpnBsOJjURZMcoqFdTA@mail.gmail.com>
[not found] ` <50228C38.5080703@redhat.com>
[not found] ` <CAO2gOZUbkTsnJ2fwgeNhcRmxkzzJEifECeiF=_YRUjeKDeRMdA@mail.gmail.com>
[not found] ` <502294A1.3060800@redhat.com>
[not found] ` <50243480.7090803@redhat.com>
[not found] ` <CAO2gOZURCuWUk2MVwCwmwrijNzWxJt3q=HUpU7=Qv6zB9e-uqA@mail.gmail.com>
[not found] ` <50254A50.8070208@redhat.com>
[not found] ` <CAO2gOZXhHuFGJ0z=jvkYKZ84EoDaPSYVjxS7QzGore56SyhWyQ@mail.gmail.com>
[not found] ` <50255B35.9020705@redhat.com>
[not found] ` <CAO2gOZWe+qMrVyvOoo7Ek-di0NKZxxU4Z=pqrDCqqCCAfctZOw@mail.gmail.com>
[not found] ` <50258712.4070002@redhat.com>
[not found] ` <CAO2gOZUQQjmKtooyXXAfgFoNVbeNwtT+P=E7pQ6jC=e07Nsr2g@mail.gmail.com>
[not found] ` <CAO2gOZX-Gn+b6LEzps5zxhgmwQWcJ-zFqG=a7hesW6fbVyxZYQ@mail.gmail.com>
[not found] ` <502E6774.8050609@redhat.com>
[not found] ` <CAO2gOZWhueDAShNLbNcJpkHA6QqXk1LNZhEMFvfT77aTZTCH9w@mail.gmail.com>
2012-08-30 14:28 ` Richard Henderson
2012-08-30 14:45 ` Bryce McKinlay
2012-08-30 15:20 ` Andrew Haley
2012-08-30 16:33 ` Richard Henderson
2012-09-04 16:07 ` Dehao Chen
2012-09-04 16:22 ` Andrew Haley
2012-09-04 20:40 ` Dehao Chen
2012-09-04 16:32 ` Bryce McKinlay
2012-09-04 16:40 ` Andrew Haley
2012-09-04 17:08 ` Bryce McKinlay
2012-09-04 17:12 ` Andrew Haley
2012-09-04 17:17 ` Bryce McKinlay
2012-09-04 17:21 ` Andrew Haley
2012-09-04 20:32 ` Dehao Chen
2012-09-05 7:29 ` Andrew Haley
2012-09-05 7:31 ` Andrew Pinski
2012-09-05 9:58 ` Mark Wielaard
2012-09-08 21:42 ` Dehao Chen [this message]
2012-09-14 15:14 ` Dehao Chen
2012-09-14 15:20 ` Andrew Haley
2012-09-15 4:25 ` H.J. Lu
2012-09-15 4:27 ` Andrew Pinski
2012-09-15 12:55 ` H.J. Lu
2012-09-15 16:09 ` Dehao Chen
2012-09-15 18:06 ` H.J. Lu
2012-09-15 22:03 ` Dehao Chen
2012-09-15 22:31 ` Mark Wielaard
2012-09-15 22:39 ` Mark Wielaard
2012-09-15 4:27 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAO2gOZVhW2+VfKU8+n2qrcKr9fgM1G4bRyUTUB_0DATH9eEW6w@mail.gmail.com \
--to=dehao@google.com \
--cc=aph@redhat.com \
--cc=bmckinlay@gmail.com \
--cc=davidxl@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=java@gcc.gnu.org \
--cc=mark@klomp.org \
--cc=richard.guenther@gmail.com \
--cc=rth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).