Patchwork Fix a bug that breaks go build

login
register
mail settings
Submitter Dehao Chen
Date Sept. 21, 2012, 2:31 a.m.
Message ID <CAO2gOZWbd_5-irwLYEPx1m-PnkgoCgY2k5QrdcE-z4aHL9jHxQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/185545/
State New
Headers show

Comments

Dehao Chen - Sept. 21, 2012, 2:31 a.m.
This patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54649

The previous patch is incorrect and will cause regression to
g++.dg/debug/dwarf2/deallocator.C

A new patch is attached. It solved the bootstrap problem for go.
Basically, when we set the line number for stmts in the finally block,
we don't want to change its block info.

The new patch can bootstrap, and passed gcc regression tests.

Is it okay for trunk?

Thanks,
Dehao

gcc/ChangeLog:
2012-09-20  Dehao Chen  <dehao@google.com>

        PR go/54649
        * tree-eh.c (lower_try_finally_dup_block): Set the correct block for
        stmts in the duplicated EH block.
Ian Taylor - Sept. 21, 2012, 3:58 a.m.
On Thu, Sep 20, 2012 at 7:31 PM, Dehao Chen <dehao@google.com> wrote:
> This patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54649
>
> The previous patch is incorrect and will cause regression to
> g++.dg/debug/dwarf2/deallocator.C
>
> A new patch is attached. It solved the bootstrap problem for go.
> Basically, when we set the line number for stmts in the finally block,
> we don't want to change its block info.
>
> The new patch can bootstrap, and passed gcc regression tests.
>
> Is it okay for trunk?
>
> Thanks,
> Dehao
>
> gcc/ChangeLog:
> 2012-09-20  Dehao Chen  <dehao@google.com>
>
>         PR go/54649
>         * tree-eh.c (lower_try_finally_dup_block): Set the correct block for
>         stmts in the duplicated EH block.

This is OK.

Please also add the test case from
http://gcc.gnu.org/ml/gcc/2012-09/msg00183.html to the compiler
testsuite.  Best would be to add a simplified version that does not
#include <string>.  But I see that there are test cases under
testsuite/g++.dg that #include <string>, so I guess it is OK to do
that in a g++ test.

You can add the test case, however you decided to do, in a separate,
later patch.

Thanks.

Ian

Patch

Index: gcc/tree-eh.c
===================================================================
--- gcc/tree-eh.c	(revision 191586)
+++ gcc/tree-eh.c	(working copy)
@@ -883,8 +883,15 @@  lower_try_finally_dup_block (gimple_seq seq, struc
   new_seq = copy_gimple_seq_and_replace_locals (seq);

   for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi))
-    if (IS_UNKNOWN_LOCATION (gimple_location (gsi_stmt (gsi))))
-      gimple_set_location (gsi_stmt (gsi), loc);
+    {
+      gimple stmt = gsi_stmt (gsi);
+      if (IS_UNKNOWN_LOCATION (gimple_location (stmt)))
+	{
+	  tree block = gimple_block (stmt);
+	  gimple_set_location (stmt, loc);
+	  gimple_set_block (stmt, block);
+	}
+    }

   if (outer_state->tf)
     region = outer_state->tf->try_finally_expr;