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 <r...@redhat.com> 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 <de...@google.com>
* 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);
