Patchwork [google/gcc-4_7] Backport patch to add line info to non-virtual thunks

login
register
mail settings
Submitter Cary Coutant
Date Aug. 7, 2012, 12:03 a.m.
Message ID <20120807000327.C5195E07FE@ccoutant.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/175476/
State New
Headers show

Comments

Cary Coutant - Aug. 7, 2012, 12:03 a.m.
This patch is for the google/gcc-4_7 branch.  It backports the following
patch from trunk at r190190:

  http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00321.html

GCC generates non-virtual thunks directly to assembly code, which
causes a couple of problems. First, it doesn't add source location
information to the thunk, so the thunk simply inherits the random
location from the end of the function in front of it (the function
it's a thunk for). In two different compilation units compiled
with different options, this could result in two different locations
for the same thunk, and gold will give a false positive ODR
violation warning.

Second, if you try to compile with -S -dA, GCC crashes in final(),
where it's trying to build a mapping from insn to bb, because the
function has no cfg, and FOR_EACH_BB_REVERSE tries to dereference
cfun->cfg without checking for non-NULL.

Bootstrapped and ran testsuite with no regressions.

Google ref b/6891766.


2012-08-06  Cary Coutant  <ccoutant@google.com>

gcc/
	* cgraphunit.c (assemble_thunk): Add source line info.
	* final.c (final): Check for non-null cfg pointer.

gcc/testsuite/
	* g++.dg/debug/dwarf2/non-virtual-thunk.C: New test case.
Rong Xu - Aug. 7, 2012, 8:09 p.m.
Looks good to me for google-4_7 branches.

-Rong

On Mon, Aug 6, 2012 at 5:03 PM, Cary Coutant <ccoutant@google.com> wrote:
> This patch is for the google/gcc-4_7 branch.  It backports the following
> patch from trunk at r190190:
>
>   http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00321.html
>
> GCC generates non-virtual thunks directly to assembly code, which
> causes a couple of problems. First, it doesn't add source location
> information to the thunk, so the thunk simply inherits the random
> location from the end of the function in front of it (the function
> it's a thunk for). In two different compilation units compiled
> with different options, this could result in two different locations
> for the same thunk, and gold will give a false positive ODR
> violation warning.
>
> Second, if you try to compile with -S -dA, GCC crashes in final(),
> where it's trying to build a mapping from insn to bb, because the
> function has no cfg, and FOR_EACH_BB_REVERSE tries to dereference
> cfun->cfg without checking for non-NULL.
>
> Bootstrapped and ran testsuite with no regressions.
>
> Google ref b/6891766.
>
>
> 2012-08-06  Cary Coutant  <ccoutant@google.com>
>
> gcc/
>         * cgraphunit.c (assemble_thunk): Add source line info.
>         * final.c (final): Check for non-null cfg pointer.
>
> gcc/testsuite/
>         * g++.dg/debug/dwarf2/non-virtual-thunk.C: New test case.
>
>
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c (revision 190189)
> +++ gcc/final.c (working copy)
> @@ -1761,11 +1761,13 @@ final (rtx first, FILE *file, int optimi
>        start_to_bb = XCNEWVEC (basic_block, bb_map_size);
>        end_to_bb = XCNEWVEC (basic_block, bb_map_size);
>
> -      FOR_EACH_BB_REVERSE (bb)
> -       {
> -         start_to_bb[INSN_UID (BB_HEAD (bb))] = bb;
> -         end_to_bb[INSN_UID (BB_END (bb))] = bb;
> -       }
> +      /* There is no cfg for a thunk.  */
> +      if (!cfun->is_thunk)
> +       FOR_EACH_BB_REVERSE (bb)
> +         {
> +           start_to_bb[INSN_UID (BB_HEAD (bb))] = bb;
> +           end_to_bb[INSN_UID (BB_END (bb))] = bb;
> +         }
>      }
>
>    /* Output the insns.  */
> Index: gcc/cgraphunit.c
> ===================================================================
> --- gcc/cgraphunit.c    (revision 190189)
> +++ gcc/cgraphunit.c    (working copy)
> @@ -1799,6 +1799,10 @@ assemble_thunk (struct cgraph_node *node
>        init_function_start (thunk_fndecl);
>        cfun->is_thunk = 1;
>        assemble_start_function (thunk_fndecl, fnname);
> +      (*debug_hooks->source_line) (DECL_SOURCE_LINE (thunk_fndecl),
> +                                  DECL_SOURCE_FILE (thunk_fndecl),
> +                                  /* discriminator */ 0,
> +                                  /* is_stmt */ 1);
>
>        targetm.asm_out.output_mi_thunk (asm_out_file, thunk_fndecl,
>                                        fixed_offset, virtual_value, alias);
> Index: gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C
> ===================================================================
> --- gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C       (revision 0)
> +++ gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C       (revision 0)
> @@ -0,0 +1,39 @@
> +// { dg-do compile }
> +// { dg-options "-g2 -dA" }
> +
> +// Verify that line number info is output for the non-virtual
> +// thunks for C::~C().
> +// { dg-final { scan-assembler "thunk.C:30" } }
> +
> +class A
> +{
> + public:
> +  A();
> +  virtual ~A();
> + private:
> +  int i;
> +};
> +
> +class B
> +{
> + public:
> +  B();
> +  virtual ~B();
> + private:
> +  int i;
> +};
> +
> +class C : public A, public B
> +{
> + public:
> +  C();
> +  virtual ~C(); // line 30
> +};
> +
> +C::C()
> +{
> +}
> +
> +C::~C()
> +{
> +}

Patch

Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 190189)
+++ gcc/final.c	(working copy)
@@ -1761,11 +1761,13 @@  final (rtx first, FILE *file, int optimi
       start_to_bb = XCNEWVEC (basic_block, bb_map_size);
       end_to_bb = XCNEWVEC (basic_block, bb_map_size);
 
-      FOR_EACH_BB_REVERSE (bb)
-	{
-	  start_to_bb[INSN_UID (BB_HEAD (bb))] = bb;
-	  end_to_bb[INSN_UID (BB_END (bb))] = bb;
-	}
+      /* There is no cfg for a thunk.  */
+      if (!cfun->is_thunk)
+	FOR_EACH_BB_REVERSE (bb)
+	  {
+	    start_to_bb[INSN_UID (BB_HEAD (bb))] = bb;
+	    end_to_bb[INSN_UID (BB_END (bb))] = bb;
+	  }
     }
 
   /* Output the insns.  */
Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c	(revision 190189)
+++ gcc/cgraphunit.c	(working copy)
@@ -1799,6 +1799,10 @@  assemble_thunk (struct cgraph_node *node
       init_function_start (thunk_fndecl);
       cfun->is_thunk = 1;
       assemble_start_function (thunk_fndecl, fnname);
+      (*debug_hooks->source_line) (DECL_SOURCE_LINE (thunk_fndecl),
+				   DECL_SOURCE_FILE (thunk_fndecl),
+				   /* discriminator */ 0,
+				   /* is_stmt */ 1);
 
       targetm.asm_out.output_mi_thunk (asm_out_file, thunk_fndecl,
 				       fixed_offset, virtual_value, alias);
Index: gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C
===================================================================
--- gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C	(revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/non-virtual-thunk.C	(revision 0)
@@ -0,0 +1,39 @@ 
+// { dg-do compile }
+// { dg-options "-g2 -dA" }
+
+// Verify that line number info is output for the non-virtual
+// thunks for C::~C().
+// { dg-final { scan-assembler "thunk.C:30" } }
+
+class A
+{
+ public:
+  A();
+  virtual ~A();
+ private:
+  int i;
+};
+
+class B
+{
+ public:
+  B();
+  virtual ~B();
+ private:
+  int i;
+};
+
+class C : public A, public B
+{
+ public:
+  C();
+  virtual ~C(); // line 30
+};
+
+C::C()
+{
+}
+
+C::~C()
+{
+}