[C++] Fix PR bootstrap/81926

[C++] Fix PR bootstrap/81926

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

Hi,

this is the bootstrap failure reported for the 7 branch on SPARC64/Solaris:

Comparing stages 2 and 3
Bootstrap comparison failure!
gcc/go/parse.o differs
make[2]: *** [compare] Error 1

On Solaris, the bootstrap is usually an old-style bootstrap, meaning that the 
debug info is generated during stage2 & stage3 so the comparison also involves 
the debug info, unlike on Linux.  This can be emulated on Linux by configuring 
--with-build-config=no and indeed you get the comparison failure there too:

Target: x86_64-suse-linux
Configured with: /home/eric/svn/gcc-7.2.0/configure --build=x86_64-suse-linux 
--prefix=/home/eric/install/gcc-7.2.0 --enable-languages=c,c++,go --enable-
__cxa_atexit --disable-nls --with-build-config=no
Thread model: posix
gcc version 7.2.0 (GCC) 

make[3]: Leaving directory '/home/eric/build/gcc-7.2.0'
Comparing stages 2 and 3
Bootstrap comparison failure!
gcc/go/parse.o differs
Makefile:24421: recipe for target 'compare' failed
make[2]: *** [compare] Error 1

The difference is:

 35 .debug_info   0016367a  0000000000000000  0000000000000000  0001faf8  2**0
                  CONTENTS, RELOC, READONLY, DEBUGGING

 35 .debug_info   00163670  0000000000000000  0000000000000000  0001faf8  2**0
                  CONTENTS, RELOC, READONLY, DEBUGGING

 	.byte	0	! end of children of DIE 0x161dbc
 	.byte	0	! end of children of DIE 0x161d9c
 	.byte	0	! end of children of DIE 0x161d5b
-	.byte	0xf2,0x1	! uleb128 0xf2; (DIE (0x161dd2) 
DW_TAG_ptr_to_member_type)
-	.uaword	0x10445d	! DW_AT_containing_type
-	.uaword	0x14806e	! DW_AT_type
-	.byte	0xa5,0x1	! uleb128 0xa5; (DIE (0x161ddc) 
DW_TAG_subprogram)
+	.byte	0xa5,0x1	! uleb128 0xa5; (DIE (0x161dd2) 
DW_TAG_subprogram)
 	.uaword	0x146733	! DW_AT_abstract_origin

It's a garbage collection issue similar to
  https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00541.html
but for build_offset_type instead of build_complex_type.

It's called from the get_debug_type langhook in C++:

tree
cp_get_debug_type (const_tree type)
{
  if (TYPE_PTRMEMFUNC_P (type) && !typedef_variant_p (type))
    return build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
			      TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));

  return NULL_TREE;
}

Since the OFFSET_TYPEs created there are not attached to any GC root, they are 
swept by every collection, meaning that the contents of the debug info depends 
on the actual collection points.

The proposed fix is to build OFFSET_TYPEs manually instead.  As witnessed by 
the change to g++.dg/debug/dwarf2/ref-3.C, this generates more DIEs in the 
debug info, but DW_TAG_ptr_to_member_type DIEs only contain 10 bytes.

Bootstrapped on x86-64/Linux & SPARC64/Solaris, OK for mainline and 7 branch?


2017-09-02  Eric Botcazou  <ebotcazou@adacore.com>

	PR bootstrap/81926
	* cp-objcp-common.c: Include stor-layout.h.
	(cp_get_debug_type): Build OFFSET_TYPEs manually.


2017-09-02  Eric Botcazou  <ebotcazou@adacore.com>

	* g++.dg/debug/dwarf2/ref-3.C: Adjust DW_TAG_ptr_to_member_type count.

-- 
Eric Botcazou

[-- Attachment #2: pr81926.diff --]
[-- Type: text/x-patch, Size: 2201 bytes --]

Index: cp/cp-objcp-common.c
===================================================================
--- cp/cp-objcp-common.c	(revision 251553)
+++ cp/cp-objcp-common.c	(working copy)
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.
 #include "coretypes.h"
 #include "cp-tree.h"
 #include "cp-objcp-common.h"
+#include "stor-layout.h"
 #include "dwarf2.h"
 
 /* Special routine to get the alias set for C++.  */
@@ -138,8 +139,20 @@ tree
 cp_get_debug_type (const_tree type)
 {
   if (TYPE_PTRMEMFUNC_P (type) && !typedef_variant_p (type))
-    return build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
-			      TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));
+    {
+      /* We cannot call build_offset_type here because the function uses the
+	 type canonicalization hashtable, which is GC-ed, so its behavior
+	 depends on the actual collection points.  Since we are building these
+	 types on the fly for the debug info, they are not attached to any
+	 GC root and will always be swept, so we would make the contents of
+	 the debug info depend on the collection points.  */
+      tree t = make_node (OFFSET_TYPE);
+      TYPE_OFFSET_BASETYPE (t)
+	= TYPE_MAIN_VARIANT (TYPE_PTRMEMFUNC_OBJECT_TYPE (type));
+      TREE_TYPE (t) = TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type));
+      layout_type (t);
+      return t;
+    }
 
   return NULL_TREE;
 }
Index: testsuite/g++.dg/debug/dwarf2/ref-3.C
===================================================================
--- testsuite/g++.dg/debug/dwarf2/ref-3.C	(revision 251553)
+++ testsuite/g++.dg/debug/dwarf2/ref-3.C	(working copy)
@@ -3,7 +3,7 @@
 // { dg-final { scan-assembler-times " DW_AT_reference" 5 { xfail *-*-aix* } } }
 // { dg-final { scan-assembler-times " DW_AT_rvalue_reference" 5 { xfail *-*-aix* } } }
 // { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_subroutine_type" 6 { xfail *-*-aix* } } }
-// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_ptr_to_member_type" 7 { xfail *-*-aix* } } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_ptr_to_member_type" 13 { xfail *-*-aix* } } }
 // { dg-final { scan-assembler-times " DW_AT_use_location" 1 { xfail *-*-aix* } } }
 
 struct S

Re: [C++] Fix PR bootstrap/81926

[Richard Biener]

On September 2, 2017 1:01:11 PM GMT+02:00, Eric Botcazou <ebotcazou@adacore.com> wrote:
>Hi,
>
>this is the bootstrap failure reported for the 7 branch on
>SPARC64/Solaris:
>
>Comparing stages 2 and 3
>Bootstrap comparison failure!
>gcc/go/parse.o differs
>make[2]: *** [compare] Error 1
>
>On Solaris, the bootstrap is usually an old-style bootstrap, meaning
>that the 
>debug info is generated during stage2 & stage3 so the comparison also
>involves 
>the debug info, unlike on Linux.  This can be emulated on Linux by
>configuring 
>--with-build-config=no and indeed you get the comparison failure there
>too:
>
>Target: x86_64-suse-linux
>Configured with: /home/eric/svn/gcc-7.2.0/configure
>--build=x86_64-suse-linux 
>--prefix=/home/eric/install/gcc-7.2.0 --enable-languages=c,c++,go
>--enable-
>__cxa_atexit --disable-nls --with-build-config=no
>Thread model: posix
>gcc version 7.2.0 (GCC) 
>
>make[3]: Leaving directory '/home/eric/build/gcc-7.2.0'
>Comparing stages 2 and 3
>Bootstrap comparison failure!
>gcc/go/parse.o differs
>Makefile:24421: recipe for target 'compare' failed
>make[2]: *** [compare] Error 1
>
>The difference is:
>
>35 .debug_info   0016367a  0000000000000000  0000000000000000  0001faf8
> 2**0
>                  CONTENTS, RELOC, READONLY, DEBUGGING
>
>35 .debug_info   00163670  0000000000000000  0000000000000000  0001faf8
> 2**0
>                  CONTENTS, RELOC, READONLY, DEBUGGING
>
> 	.byte	0	! end of children of DIE 0x161dbc
> 	.byte	0	! end of children of DIE 0x161d9c
> 	.byte	0	! end of children of DIE 0x161d5b
>-	.byte	0xf2,0x1	! uleb128 0xf2; (DIE (0x161dd2) 
>DW_TAG_ptr_to_member_type)
>-	.uaword	0x10445d	! DW_AT_containing_type
>-	.uaword	0x14806e	! DW_AT_type
>-	.byte	0xa5,0x1	! uleb128 0xa5; (DIE (0x161ddc) 
>DW_TAG_subprogram)
>+	.byte	0xa5,0x1	! uleb128 0xa5; (DIE (0x161dd2) 
>DW_TAG_subprogram)
> 	.uaword	0x146733	! DW_AT_abstract_origin
>
>It's a garbage collection issue similar to
>  https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00541.html
>but for build_offset_type instead of build_complex_type.
>
>It's called from the get_debug_type langhook in C++:
>
>tree
>cp_get_debug_type (const_tree type)
>{
>  if (TYPE_PTRMEMFUNC_P (type) && !typedef_variant_p (type))
>    return build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
>			      TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));
>
>  return NULL_TREE;
>}
>
>Since the OFFSET_TYPEs created there are not attached to any GC root,
>they are 
>swept by every collection, meaning that the contents of the debug info
>depends 
>on the actual collection points.
>
>The proposed fix is to build OFFSET_TYPEs manually instead.  As
>witnessed by 
>the change to g++.dg/debug/dwarf2/ref-3.C, this generates more DIEs in
>the 
>debug info, but DW_TAG_ptr_to_member_type DIEs only contain 10 bytes.

A solution would be to put them into a global GCed pointer-map or vector, freeing that at free-lang-data time. 

Richard. 

>Bootstrapped on x86-64/Linux & SPARC64/Solaris, OK for mainline and 7
>branch?
>
>
>2017-09-02  Eric Botcazou  <ebotcazou@adacore.com>
>
>	PR bootstrap/81926
>	* cp-objcp-common.c: Include stor-layout.h.
>	(cp_get_debug_type): Build OFFSET_TYPEs manually.
>
>
>2017-09-02  Eric Botcazou  <ebotcazou@adacore.com>
>
>	* g++.dg/debug/dwarf2/ref-3.C: Adjust DW_TAG_ptr_to_member_type count.

Re: [C++] Fix PR bootstrap/81926

[Eric Botcazou]

> A solution would be to put them into a global GCed pointer-map or vector,
> freeing that at free-lang-data time.

The first part sounds good, but not the second part, as rest_of_handle_final 
generates debug info too.

-- 
Eric Botcazou

Re: [C++] Fix PR bootstrap/81926

[Richard Biener]

On September 3, 2017 10:05:19 AM GMT+02:00, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> A solution would be to put them into a global GCed pointer-map or
>vector,
>> freeing that at free-lang-data time.
>
>The first part sounds good, but not the second part, as
>rest_of_handle_final 
>generates debug info too.

But not types, less using langhooks. 

Richard. 

Re: [C++] Fix PR bootstrap/81926

[Eric Botcazou]

> But not types, less using langhooks.

#0  cp_get_debug_type (type=0x7ffff07702a0)
    at /home/eric/svn/gcc-7.2.0/gcc/cp/cp-objcp-common.c:142
#1  0x00000000009f2c86 in gen_type_die_with_usage (
    type=type@entry=0x7ffff07702a0, 
    context_die=context_die@entry=0x7fffed7e90a0, 
    usage=usage@entry=DINFO_USAGE_DIR_USE)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:24566
#2  0x00000000009f4e55 in gen_type_die (type=0x7ffff07702a0, 
    context_die=context_die@entry=0x7fffed7e90a0)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:24745
#3  0x00000000009f9f67 in gen_decl_die (decl=decl@entry=0x7ffff076a980, 
    origin=origin@entry=0x0, ctx=ctx@entry=0x0, 
    context_die=context_die@entry=0x7fffed7e90a0)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25422
#4  0x00000000009f8c13 in gen_subprogram_die (decl=decl@entry=0x7ffff1109a00, 
    context_die=context_die@entry=0x7ffff6c5f000)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:22386
#5  0x00000000009f95dc in gen_decl_die (decl=decl@entry=0x7ffff1109a00, 
    origin=<optimized out>, origin@entry=0x0, ctx=ctx@entry=0x0, 
    context_die=context_die@entry=0x7ffff6c5f000)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25335
#6  0x00000000009fc2d2 in dwarf2out_decl (decl=0x7ffff1109a00)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25844
#7  0x0000000000a13404 in dwarf2out_function_decl (decl=<optimized out>)
    at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25859
#8  0x0000000000a74197 in rest_of_handle_final ()
    at /home/eric/svn/gcc-7.2.0/gcc/final.c:4520

-- 
Eric Botcazou

Re: [C++] Fix PR bootstrap/81926

[Richard Biener]

On Sun, Sep 3, 2017 at 4:15 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> But not types, less using langhooks.
>
> #0  cp_get_debug_type (type=0x7ffff07702a0)
>     at /home/eric/svn/gcc-7.2.0/gcc/cp/cp-objcp-common.c:142
> #1  0x00000000009f2c86 in gen_type_die_with_usage (
>     type=type@entry=0x7ffff07702a0,
>     context_die=context_die@entry=0x7fffed7e90a0,
>     usage=usage@entry=DINFO_USAGE_DIR_USE)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:24566
> #2  0x00000000009f4e55 in gen_type_die (type=0x7ffff07702a0,
>     context_die=context_die@entry=0x7fffed7e90a0)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:24745
> #3  0x00000000009f9f67 in gen_decl_die (decl=decl@entry=0x7ffff076a980,
>     origin=origin@entry=0x0, ctx=ctx@entry=0x0,
>     context_die=context_die@entry=0x7fffed7e90a0)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25422
> #4  0x00000000009f8c13 in gen_subprogram_die (decl=decl@entry=0x7ffff1109a00,
>     context_die=context_die@entry=0x7ffff6c5f000)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:22386
> #5  0x00000000009f95dc in gen_decl_die (decl=decl@entry=0x7ffff1109a00,
>     origin=<optimized out>, origin@entry=0x0, ctx=ctx@entry=0x0,
>     context_die=context_die@entry=0x7ffff6c5f000)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25335
> #6  0x00000000009fc2d2 in dwarf2out_decl (decl=0x7ffff1109a00)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25844
> #7  0x0000000000a13404 in dwarf2out_function_decl (decl=<optimized out>)
>     at /home/eric/svn/gcc-7.2.0/gcc/dwarf2out.c:25859
> #8  0x0000000000a74197 in rest_of_handle_final ()
>     at /home/eric/svn/gcc-7.2.0/gcc/final.c:4520

Ah, wishful thinking on my side ;)  Well, I suppose it'll be true in the future
when free-lang-data runs unconditionally and resets all langhooks.

Richard.

> --
> Eric Botcazou

Re: [C++] Fix PR bootstrap/81926

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]

> A solution would be to put them into a global GCed pointer-map or vector,
> freeing that at free-lang-data time.

Here's a version that implements a bona-fide type->offset_type map.


        PR bootstrap/81926
        * cp-objcp-common.c (struct offset_type_hasher): New class.
	(offset_type_hash): New variable.
        (cp_get_debug_type): Associate the OFFSET_TYPEs with the types.

-- 
Eric Botcazou

[-- Attachment #2: pr81926-2.diff --]
[-- Type: text/x-patch, Size: 2188 bytes --]

Index: cp/cp-objcp-common.c
===================================================================
--- cp/cp-objcp-common.c	(revision 251538)
+++ cp/cp-objcp-common.c	(working copy)
@@ -131,6 +131,20 @@ cxx_types_compatible_p (tree x, tree y)
   return same_type_ignoring_top_level_qualifiers_p (x, y);
 }
 
+struct offset_type_hasher : ggc_cache_ptr_hash<tree_map>
+{
+  static hashval_t hash (tree_map *m) { return tree_map_hash (m); }
+  static bool equal (tree_map *a, tree_map *b) { return tree_map_eq (a, b); }
+
+  static int
+  keep_cache_entry (tree_map *&e)
+  {
+    return ggc_marked_p (e->base.from);
+  }
+};
+
+static GTY((cache)) hash_table<offset_type_hasher> *offset_type_hash;
+
 /* Return a type to use in the debug info instead of TYPE, or NULL_TREE to
    keep TYPE.  */
 
@@ -138,8 +152,35 @@ tree
 cp_get_debug_type (const_tree type)
 {
   if (TYPE_PTRMEMFUNC_P (type) && !typedef_variant_p (type))
-    return build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
-			      TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));
+    {
+      if (offset_type_hash == NULL)
+	offset_type_hash = hash_table<offset_type_hasher>::create_ggc (512);
+
+      /* We cannot simply use build_offset_type here because the function uses
+	 the type canonicalization hashtable, which is GC-ed, so its behavior
+	 depends on the actual collection points.  Since we are building these
+	 types on the fly for the debug info only, they would not be attached
+	 to any GC root and always be swept, so we would make the contents of
+	 the debug info depend on the collection points.  */
+      struct tree_map in, *h;
+
+      in.base.from = CONST_CAST_TREE (type);
+      in.hash = htab_hash_pointer (type);
+      h = offset_type_hash->find_with_hash (&in, in.hash);
+      if (h)
+	return h->to;
+
+      tree t = build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
+				  TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));
+
+      h = ggc_alloc<tree_map> ();
+      h->base.from = CONST_CAST_TREE (type);
+      h->hash = htab_hash_pointer (type);
+      h->to = t;
+      *offset_type_hash->find_slot_with_hash (h, h->hash, INSERT) = h;
+
+      return t;
+    }
 
   return NULL_TREE;
 }

Re: [C++] Fix PR bootstrap/81926

[Jason Merrill]

On Mon, Sep 4, 2017 at 10:08 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> A solution would be to put them into a global GCed pointer-map or vector,
>> freeing that at free-lang-data time.
>
> Here's a version that implements a bona-fide type->offset_type map.
>
>         PR bootstrap/81926
>         * cp-objcp-common.c (struct offset_type_hasher): New class.
>         (offset_type_hash): New variable.
>         (cp_get_debug_type): Associate the OFFSET_TYPEs with the types.

This looks good.  But let's call it debug_type_hash instead.  OK with
that change.

Jason

Re: [C++] Fix PR bootstrap/81926

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

> This looks good.  But let's call it debug_type_hash instead.  OK with
> that change.

Thanks.  It occured to me that we don't need to look up the type twice when we 
need to insert it so I have installed the attached patchlet as obvious after 
boostrapping/regtesting on x86-64/Linux.


	PR bootstrap/81926
	* cp-objcp-common.c (cp_get_debug_type): Do only one lookup.

-- 
Eric Botcazou

[-- Attachment #2: pr81926-2b.diff --]
[-- Type: text/x-patch, Size: 1195 bytes --]

Index: cp-objcp-common.c
===================================================================
--- cp-objcp-common.c	(revision 253049)
+++ cp-objcp-common.c	(working copy)
@@ -162,13 +162,13 @@ cp_get_debug_type (const_tree type)
 	 types on the fly for the debug info only, they would not be attached
 	 to any GC root and always be swept, so we would make the contents of
 	 the debug info depend on the collection points.  */
-      struct tree_map in, *h;
+      struct tree_map in, *h, **slot;
 
       in.base.from = CONST_CAST_TREE (type);
       in.hash = htab_hash_pointer (type);
-      h = debug_type_hash->find_with_hash (&in, in.hash);
-      if (h)
-	return h->to;
+      slot = debug_type_hash->find_slot_with_hash (&in, in.hash, INSERT);
+      if (*slot)
+	return (*slot)->to;
 
       tree t = build_offset_type (TYPE_PTRMEMFUNC_OBJECT_TYPE (type),
 				  TREE_TYPE (TYPE_PTRMEMFUNC_FN_TYPE (type)));
@@ -177,7 +177,7 @@ cp_get_debug_type (const_tree type)
       h->base.from = CONST_CAST_TREE (type);
       h->hash = htab_hash_pointer (type);
       h->to = t;
-      *debug_type_hash->find_slot_with_hash (h, h->hash, INSERT) = h;
+      *slot = h;
 
       return t;
     }