[PR82155] Fix crash in dwarf2out_abstract_function

Message ID 20170912060003.32378-1-derodat@adacore.com
State New
Headers show
Series
  • [PR82155] Fix crash in dwarf2out_abstract_function
Related show

Commit Message

Pierre-Marie de Rodat Sept. 12, 2017, 6 a.m.
Hello,

This patch is an attempt to fix the crash reported in PR82155.

When generating a C++ class method for a class that is itself nested in
a class method, dwarf2out_early_global_decl currently leaves the
existing context DIE as it is if it already exists.  However, it is
possible that this call happens at a point where this context DIE is
just a declaration that is itself not located in its own context.

From there, if dwarf2out_early_global_decl is not called on any of the
FUNCTION_DECL in the context chain, DIEs will be left badly scoped and
some (such as the nested method) will be removed by the type pruning
machinery.  As a consequence, dwarf2out_abstract_function will will
crash when called on the corresponding DECL because it asserts that the
DECL has a DIE.

This patch fixes this crash making dwarf2out_early_global_decl process
context DIEs the same way we process abstract origins for FUNCTION_DECL:
if the corresponding DIE exists but is only a declaration, call
dwarf2out_decl anyway on it so that it is turned into a more complete
DIE and so that it is relocated in the proper context.

Bootstrapped and regtested on x86_64-linux.  The crash this addresses is
present both on trunk and on the gcc-7 branch: I suggest we commit this
patch on both branches.  Ok to commit? Thank you in advance!

gcc/

	PR debug/82155
	* dwarf2out.c (dwarf2out_early_global_decl): Call dwarf2out_decl
	on the FUNCTION_DECL function context if it has a DIE that is a
	declaration.

gcc/testsuite/

	* g++.dg/pr82155.C: New testcase.
---
 gcc/dwarf2out.c                | 10 ++++++++--
 gcc/testsuite/g++.dg/pr82155.C | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr82155.C

Comments

Richard Biener Sept. 25, 2017, 11:54 a.m. | #1
On Tue, Sep 12, 2017 at 8:00 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello,
>
> This patch is an attempt to fix the crash reported in PR82155.
>
> When generating a C++ class method for a class that is itself nested in
> a class method, dwarf2out_early_global_decl currently leaves the
> existing context DIE as it is if it already exists.  However, it is
> possible that this call happens at a point where this context DIE is
> just a declaration that is itself not located in its own context.
>
> From there, if dwarf2out_early_global_decl is not called on any of the
> FUNCTION_DECL in the context chain, DIEs will be left badly scoped and
> some (such as the nested method) will be removed by the type pruning
> machinery.  As a consequence, dwarf2out_abstract_function will will
> crash when called on the corresponding DECL because it asserts that the
> DECL has a DIE.
>
> This patch fixes this crash making dwarf2out_early_global_decl process
> context DIEs the same way we process abstract origins for FUNCTION_DECL:
> if the corresponding DIE exists but is only a declaration, call
> dwarf2out_decl anyway on it so that it is turned into a more complete
> DIE and so that it is relocated in the proper context.
>
> Bootstrapped and regtested on x86_64-linux.  The crash this addresses is
> present both on trunk and on the gcc-7 branch: I suggest we commit this
> patch on both branches.  Ok to commit? Thank you in advance!

Ok for trunk and gcc-7 branch after a while.

Thanks,
Richard.

> gcc/
>
>         PR debug/82155
>         * dwarf2out.c (dwarf2out_early_global_decl): Call dwarf2out_decl
>         on the FUNCTION_DECL function context if it has a DIE that is a
>         declaration.
>
> gcc/testsuite/
>
>         * g++.dg/pr82155.C: New testcase.
> ---
>  gcc/dwarf2out.c                | 10 ++++++++--
>  gcc/testsuite/g++.dg/pr82155.C | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/pr82155.C
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 00d6d951ba3..4cfc9c186af 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -25500,10 +25500,16 @@ dwarf2out_early_global_decl (tree decl)
>              so that all nested DIEs are generated at the proper scope in the
>              first shot.  */
>           tree context = decl_function_context (decl);
> -         if (context != NULL && lookup_decl_die (context) == NULL)
> +         if (context != NULL)
>             {
> +             dw_die_ref context_die = lookup_decl_die (context);
>               current_function_decl = context;
> -             dwarf2out_decl (context);
> +
> +             /* Avoid emitting DIEs multiple times, but still process CONTEXT
> +                enough so that it lands in its own context.  This avoids type
> +                pruning issues later on.  */
> +             if (context_die == NULL || is_declaration_die (context_die))
> +               dwarf2out_decl (context);
>             }
>
>           /* Emit an abstract origin of a function first.  This happens
> diff --git a/gcc/testsuite/g++.dg/pr82155.C b/gcc/testsuite/g++.dg/pr82155.C
> new file mode 100644
> index 00000000000..75d9b615f39
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr82155.C
> @@ -0,0 +1,36 @@
> +/* { dg-do compile { target c++11 } } */
> +/* { dg-options "-g -O2" } */
> +
> +template <typename a> struct b { a c; };
> +template <typename d> struct e { d *operator->(); };
> +template <typename d> class h {
> +public:
> +  typedef e<d> ag;
> +};
> +class i {
> +protected:
> +  i(int);
> +};
> +class j {
> +  virtual void k(int) = 0;
> +
> +public:
> +  int f;
> +  void l() { k(f); }
> +};
> +struct m : i {
> +  int cn;
> +  m() : i(cn) {
> +    struct n : j {
> +      n() {}
> +      void k(int) {}
> +    };
> +  }
> +};
> +struct o {
> +  o() {
> +    for (h<b<b<j *>>>::ag g;;)
> +      g->c.c->l();
> +  }
> +};
> +void fn1() { o(); }
> --
> 2.14.1
>
Pierre-Marie de Rodat Sept. 25, 2017, 12:27 p.m. | #2
On 09/25/2017 01:54 PM, Richard Biener wrote:
> Ok for trunk and gcc-7 branch after a while.

Thank you, Richard! Committed on trunk as 253147; I’ll wait at least one 
week to revisit the gcc-7 branch commit.
Pierre-Marie de Rodat Nov. 15, 2017, 9:11 a.m. | #3
Hello Richard,

On 09/25/2017 01:54 PM, Richard Biener wrote:
> Ok for trunk and gcc-7 branch after a while.
Is it still okay to commit to gcc-7, now?
Richard Biener Nov. 15, 2017, 11:16 a.m. | #4
On Wed, Nov 15, 2017 at 10:11 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello Richard,
>
> On 09/25/2017 01:54 PM, Richard Biener wrote:
>>
>> Ok for trunk and gcc-7 branch after a while.
>
> Is it still okay to commit to gcc-7, now?

Yes.

Richard.

> --
> Pierre-Marie de Rodat
Pierre-Marie de Rodat Nov. 15, 2017, 11:54 a.m. | #5
On 11/15/2017 12:16 PM, Richard Biener wrote:
>> Is it still okay to commit to gcc-7, now?
> 
> Yes.

Done. Thank you!

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 00d6d951ba3..4cfc9c186af 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -25500,10 +25500,16 @@  dwarf2out_early_global_decl (tree decl)
 	     so that all nested DIEs are generated at the proper scope in the
 	     first shot.  */
 	  tree context = decl_function_context (decl);
-	  if (context != NULL && lookup_decl_die (context) == NULL)
+	  if (context != NULL)
 	    {
+	      dw_die_ref context_die = lookup_decl_die (context);
 	      current_function_decl = context;
-	      dwarf2out_decl (context);
+
+	      /* Avoid emitting DIEs multiple times, but still process CONTEXT
+		 enough so that it lands in its own context.  This avoids type
+		 pruning issues later on.  */
+	      if (context_die == NULL || is_declaration_die (context_die))
+		dwarf2out_decl (context);
 	    }
 
 	  /* Emit an abstract origin of a function first.  This happens
diff --git a/gcc/testsuite/g++.dg/pr82155.C b/gcc/testsuite/g++.dg/pr82155.C
new file mode 100644
index 00000000000..75d9b615f39
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr82155.C
@@ -0,0 +1,36 @@ 
+/* { dg-do compile { target c++11 } } */
+/* { dg-options "-g -O2" } */
+
+template <typename a> struct b { a c; };
+template <typename d> struct e { d *operator->(); };
+template <typename d> class h {
+public:
+  typedef e<d> ag;
+};
+class i {
+protected:
+  i(int);
+};
+class j {
+  virtual void k(int) = 0;
+
+public:
+  int f;
+  void l() { k(f); }
+};
+struct m : i {
+  int cn;
+  m() : i(cn) {
+    struct n : j {
+      n() {}
+      void k(int) {}
+    };
+  }
+};
+struct o {
+  o() {
+    for (h<b<b<j *>>>::ag g;;)
+      g->c.c->l();
+  }
+};
+void fn1() { o(); }