diff mbox

Try harder to fix recently introduced crashes in ggc_collect

Message ID AM4PR0701MB216279E6102BF06782342240E4E40@AM4PR0701MB2162.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger May 18, 2017, 8:40 p.m. UTC
Hi,

unfortunately the first patch was still insufficient.  I identified many
more relatively new places where static tree objects are invisible to
GC.

Nathan, whatever you are doing, please do it a bit more slowly, thanks.

Bootstrap and reg-testing on x86_64-pc-linux-gnu in progress.
Is it OK after successful reg-testing?


Thanks
Bernd.


On 05/18/17 17:33, Richard Biener wrote:
> On May 18, 2017 5:15:43 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:

>> Hi,

>>

>>

>> this attempts to fix occasional segmentation faults that are present in

>> the current snapshot, while previous snapshot was stable.

>>

>> I observed numerous crashes but all were non-reproducible,

>> like the following example:

>>

>> In file included from

>> /home/ed/gnu/gcc-build-1/x86_64-pc-linux-gnu/libstdc++-v3/include/string:52:0,

>>                   from

>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_test_config.h:19,

>>                   from

>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_test_utils.h:17,

>>                   from

>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_globals_test.cc:12,

>>                   from

>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_globals_test-wrapper.cc:2:

>> /home/ed/gnu/gcc-build-1/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:6277:22:

>>

>> internal compiler error: Segmentation fault

>> 0xd7e17f crash_signal

>>          ../../gcc-8-20170514-1/gcc/toplev.c:337

>> 0x8f23fe ggc_set_mark(void const*)

>>          ../../gcc-8-20170514-1/gcc/ggc-page.c:1546

>> 0x7e6a5f gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:133

>> 0x7e8c7a gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:235

>> 0x7e8882 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:365

>> 0x81b26d gt_ggc_mx_cp_binding_level(void*)

>>          ./gt-cp-name-lookup.h:72

>> 0x7e6d85 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:648

>> 0x7e8ad2 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:221

>> 0x7e8eeb gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:337

>> 0x7e8a3c gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:441

>> 0x7e7304 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:606

>> 0x81b352 gt_ggc_mx_cxx_binding(void*)

>>          ./gt-cp-name-lookup.h:60

>> 0x7e6d85 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:648

>> 0x7e8ef5 gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:336

>> 0x7e8a3c gt_ggc_mx_lang_tree_node(void*)

>>          ./gt-cp-tree.h:441

>> 0xb2edbe void gt_ggc_mx<tree_node*>(vec<tree_node*, va_gc, vl_embed>*)

>>          ../../gcc-8-20170514-1/gcc/vec.h:1110

>> 0xb2edbe gt_ggc_mx_vec_tree_va_gc_(void*)

>>          /home/ed/gnu/gcc-build-1/gcc/gtype-desc.c:1737

>> 0xac59f5 ggc_mark_root_tab

>>          ../../gcc-8-20170514-1/gcc/ggc-common.c:77

>> 0xac5c50 ggc_mark_roots()

>>          ../../gcc-8-20170514-1/gcc/ggc-common.c:94

>> 0x8f2de7 ggc_collect()

>>          ../../gcc-8-20170514-1/gcc/ggc-page.c:2206

>> Please submit a full bug report,

>> with preprocessed source if appropriate.

>> Please include the complete backtrace with any bug report.

>>

>>

>> The following patch fixes one rather suspicious static tree

>> object that did not have the GTY attribute, and was therefore

>> apparently not in the GC root set.

>>

>>

>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.

>> Is it OK for trunk?

> 

> OK.

> 

> Richard.

> 

>>

>> Thanks

>> Bernd.

>

Comments

Nathan Sidwell May 18, 2017, 11:17 p.m. UTC | #1
On 05/18/2017 04:40 PM, Bernd Edlinger wrote:
> Hi,
>
> unfortunately the first patch was still insufficient.  I identified many
> more relatively new places where static tree objects are invisible to
> GC.
>

Hm, I didn't think the fns in except.c etc needed to be GTY'd because they'd 
still be reachable via the symbol table.  Oh, is it PCH dying? Sorry about that. 
  The C++ changes are ok  (I'll probably go an turn the separate fns as an array 
as penance.)

nathan
diff mbox

Patch

gcc/c-family:
2017-05-18  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-format.c (hwi, locus): Move out of function scope,
	add GTY attribute.

gcc/cp:
2017-05-18  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* config-lang.in (gtfiles): Add c-family/c-format.c,
	except.c, init.c, lambda.c and friend.c.
	* class.c (dvirt_fn): Move out of function scope,
	add GTY attribute.
	* except.c (fn1-5, throw_fn, rethrow_fn, spec): Likewise.
	* init.c (fn): Likewise.
	* lambda.c (ptr_id, max_id): Likewise.
	* friend.c (global_friend): Add GTY attribute.

Index: gcc/c-family/c-format.c
===================================================================
--- gcc/c-family/c-format.c	(revision 248242)
+++ gcc/c-family/c-format.c	(working copy)
@@ -55,6 +55,8 @@  struct function_format_info
 
 /* Initialized in init_dynamic_diag_info.  */
 static GTY(()) tree local_tree_type_node;
+static GTY(()) tree hwi;
+static GTY(()) tree locus;
 
 static bool decode_format_attr (tree, function_format_info *, int);
 static int decode_format_type (const char *);
@@ -3675,8 +3677,6 @@  find_length_info_modifier_index (const format_leng
 static void
 init_dynamic_asm_fprintf_info (void)
 {
-  static tree hwi;
-
   if (!hwi)
     {
       format_length_info *new_asm_fprintf_length_specs;
@@ -3734,8 +3734,6 @@  init_dynamic_asm_fprintf_info (void)
 static void
 init_dynamic_gfc_info (void)
 {
-  static tree locus;
-
   if (!locus)
     {
       static format_char_info *gfc_fci;
@@ -3828,8 +3826,6 @@  init_dynamic_diag_info (void)
 	local_tree_type_node = void_type_node;
     }
 
-  static tree hwi;
-
   if (!hwi)
     {
       static format_length_info *diag_ls;
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 248242)
+++ gcc/cp/class.c	(working copy)
@@ -9541,6 +9541,7 @@  dfs_accumulate_vtbl_inits (tree binfo,
 }
 
 static GTY(()) tree abort_fndecl_addr;
+static GTY(()) tree dvirt_fn;
 
 /* Construct the initializer for BINFO's virtual function table.  BINFO
    is part of the hierarchy dominated by T.  If we're building a
@@ -9714,8 +9715,6 @@  build_vtbl_initializer (tree binfo,
 	  /* Likewise for deleted virtuals.  */
 	  else if (DECL_DELETED_FN (fn_original))
 	    {
-	      static tree dvirt_fn;
-
 	      if (!dvirt_fn)
 		{
 		  tree name = get_identifier ("__cxa_deleted_virtual");
Index: gcc/cp/config-lang.in
===================================================================
--- gcc/cp/config-lang.in	(revision 248242)
+++ gcc/cp/config-lang.in	(working copy)
@@ -29,4 +29,4 @@  compilers="cc1plus\$(exeext)"
 
 target_libs="target-libstdc++-v3"
 
-gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.h \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/cp-lang.c \$(srcdir)/cp/except.c \$(srcdir)/cp/vtable-class-hierarchy.c \$(srcdir)/cp/constexpr.c \$(srcdir)/cp/cp-gimplify.c"
+gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.h \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/c-family/c-format.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/cp-lang.c \$(srcdir)/cp/except.c \$(srcdir)/cp/vtable-class-hierarchy.c \$(srcdir)/cp/constexpr.c \$(srcdir)/cp/cp-gimplify.c \$(srcdir)/cp/init.c \$(srcdir)/cp/friend.c \$(srcdir)/cp/lambda.c"
Index: gcc/cp/except.c
===================================================================
--- gcc/cp/except.c	(revision 248242)
+++ gcc/cp/except.c	(working copy)
@@ -42,6 +42,15 @@  static int complete_ptr_ref_or_void_ptr_p (tree, t
 static bool is_admissible_throw_operand_or_catch_parameter (tree, bool);
 static int can_convert_eh (tree, tree);
 
+static GTY(()) tree fn1;
+static GTY(()) tree fn2;
+static GTY(()) tree fn3;
+static GTY(()) tree fn4;
+static GTY(()) tree fn5;
+static GTY(()) tree throw_fn;
+static GTY(()) tree rethrow_fn;
+static GTY(()) tree spec;
+
 /* Sets up all the global eh stuff that needs to be initialized at the
    start of compilation.  */
 
@@ -154,20 +163,18 @@  declare_library_fn (tree name, tree return_type, t
 static tree
 do_get_exception_ptr (void)
 {
-  static tree fn;
-
-  if (!fn)
+  if (!fn1)
     {
       tree name = get_identifier ("__cxa_get_exception_ptr");
-      fn = IDENTIFIER_GLOBAL_VALUE (name);
-      if (!fn)
+      fn1 = IDENTIFIER_GLOBAL_VALUE (name);
+      if (!fn1)
 	/* Declare void* __cxa_get_exception_ptr (void *) throw().  */
-	fn = declare_library_fn
+	fn1 = declare_library_fn
 	  (name, ptr_type_node, ptr_type_node,
 	   ECF_NOTHROW | ECF_PURE | ECF_LEAF | ECF_TM_PURE);
     }
 
-  return cp_build_function_call_nary (fn, tf_warning_or_error,
+  return cp_build_function_call_nary (fn1, tf_warning_or_error,
 				      build_exc_ptr (), NULL_TREE);
 }
 
@@ -177,16 +184,14 @@  do_get_exception_ptr (void)
 static tree
 do_begin_catch (void)
 {
-  static tree fn;
-
-  if (!fn)
+  if (!fn2)
     {
-      tree name = fn = get_identifier ("__cxa_begin_catch");
-      fn = IDENTIFIER_GLOBAL_VALUE (name);
-      if (!fn)
+      tree name = get_identifier ("__cxa_begin_catch");
+      fn2 = IDENTIFIER_GLOBAL_VALUE (name);
+      if (!fn2)
 	{
 	  /* Declare void* __cxa_begin_catch (void *) throw().  */
-	  fn = declare_library_fn
+	  fn2 = declare_library_fn
 	    (name, ptr_type_node, ptr_type_node, ECF_NOTHROW);
 
 	  /* Create its transactional-memory equivalent.  */
@@ -198,12 +203,12 @@  do_begin_catch (void)
 		itm_fn = declare_library_fn
 		  (itm_name, ptr_type_node, ptr_type_node,
 		   ECF_NOTHROW | ECF_TM_PURE);
-	      record_tm_replacement (fn, itm_fn);
+	      record_tm_replacement (fn2, itm_fn);
 	    }
 	}
     }
 
-  return cp_build_function_call_nary (fn, tf_warning_or_error,
+  return cp_build_function_call_nary (fn2, tf_warning_or_error,
 				      build_exc_ptr (), NULL_TREE);
 }
 
@@ -231,17 +236,15 @@  dtor_nothrow (tree type)
 static tree
 do_end_catch (tree type)
 {
-  static tree fn;
-
-  if (!fn)
+  if (!fn3)
     {
       tree name = get_identifier ("__cxa_end_catch");
-      fn = IDENTIFIER_GLOBAL_VALUE (name);
-      if (!fn)
+      fn3 = IDENTIFIER_GLOBAL_VALUE (name);
+      if (!fn3)
 	{
 	  /* Declare void __cxa_end_catch ().
 	     This can throw if the destructor for the exception throws.  */
-	  fn = push_void_library_fn (name, void_list_node, 0);
+	  fn3 = push_void_library_fn (name, void_list_node, 0);
 
 	  /* Create its transactional-memory equivalent.  */
 	  if (flag_tm)
@@ -251,12 +254,12 @@  do_end_catch (tree type)
 	      if (!itm_fn)
 		itm_fn = push_void_library_fn
 		  (itm_name, void_list_node, ECF_TM_PURE);
-	      record_tm_replacement (fn, itm_fn);
+	      record_tm_replacement (fn3, itm_fn);
 	    }
 	}
     }
 
-  tree cleanup = cp_build_function_call_vec (fn, NULL, tf_warning_or_error);
+  tree cleanup = cp_build_function_call_vec (fn3, NULL, tf_warning_or_error);
   TREE_NOTHROW (cleanup) = dtor_nothrow (type);
 
   return cleanup;
@@ -516,17 +519,15 @@  finish_eh_spec_block (tree raw_raises, tree eh_spe
 static tree
 do_allocate_exception (tree type)
 {
-  static tree fn;
-
-  if (!fn)
+  if (!fn4)
     {
       tree name = get_identifier ("__cxa_allocate_exception");
-      fn = IDENTIFIER_GLOBAL_VALUE (name);
-      if (!fn)
+      fn4 = IDENTIFIER_GLOBAL_VALUE (name);
+      if (!fn4)
 	{
 	  /* Declare void *__cxa_allocate_exception(size_t) throw().  */
-	  fn = declare_library_fn (name, ptr_type_node, size_type_node,
-				   ECF_NOTHROW | ECF_MALLOC);
+	  fn4 = declare_library_fn (name, ptr_type_node, size_type_node,
+				    ECF_NOTHROW | ECF_MALLOC);
 
 	  if (flag_tm)
 	    {
@@ -536,12 +537,12 @@  do_allocate_exception (tree type)
 		itm_fn = declare_library_fn
 		  (itm_name, ptr_type_node, size_type_node, 
 		   ECF_NOTHROW | ECF_MALLOC | ECF_TM_PURE);
-	      record_tm_replacement (fn, itm_fn);
+	      record_tm_replacement (fn4, itm_fn);
 	    }
 	}
     }
 
-  return cp_build_function_call_nary (fn, tf_warning_or_error,
+  return cp_build_function_call_nary (fn4, tf_warning_or_error,
 				      size_in_bytes (type), NULL_TREE);
 }
 
@@ -551,17 +552,15 @@  do_allocate_exception (tree type)
 static tree
 do_free_exception (tree ptr)
 {
-  static tree fn;
-
-  if (!fn)
+  if (!fn5)
     {
       tree name = get_identifier ("__cxa_free_exception");
-      fn = IDENTIFIER_GLOBAL_VALUE (name);
-      if (!fn)
+      fn5 = IDENTIFIER_GLOBAL_VALUE (name);
+      if (!fn5)
 	{
 	  /* Declare void __cxa_free_exception (void *) throw().  */
-	  fn = declare_library_fn (name, void_type_node, ptr_type_node,
-				   ECF_NOTHROW | ECF_LEAF);
+	  fn5 = declare_library_fn (name, void_type_node, ptr_type_node,
+				    ECF_NOTHROW | ECF_LEAF);
 
 	  if (flag_tm)
 	    {
@@ -571,12 +570,12 @@  do_free_exception (tree ptr)
 		itm_fn = declare_library_fn
 		  (itm_name, void_type_node, ptr_type_node,
 		   ECF_NOTHROW | ECF_LEAF | ECF_TM_PURE);
-	      record_tm_replacement (fn, itm_fn);
+	      record_tm_replacement (fn5, itm_fn);
 	    }
 	}
     }
 
-  return cp_build_function_call_nary (fn, tf_warning_or_error, ptr, NULL_TREE);
+  return cp_build_function_call_nary (fn5, tf_warning_or_error, ptr, NULL_TREE);
 }
 
 /* Wrap all cleanups for TARGET_EXPRs in MUST_NOT_THROW_EXPR.
@@ -640,7 +639,6 @@  build_throw (tree exp)
 
   if (exp)
     {
-      static tree throw_fn;
       tree throw_type;
       tree temp_type;
       tree cleanup;
@@ -793,8 +791,6 @@  build_throw (tree exp)
   else
     {
       /* Rethrow current exception.  */
-      static tree rethrow_fn;
-
       if (!rethrow_fn)
 	{
 	  tree name = get_identifier ("__cxa_rethrow");
@@ -1261,7 +1257,6 @@  build_noexcept_spec (tree expr, int complain)
 tree
 unevaluated_noexcept_spec (void)
 {
-  static tree spec;
   if (spec == NULL_TREE)
     spec = build_noexcept_spec (make_node (DEFERRED_NOEXCEPT), tf_none);
   return spec;
Index: gcc/cp/friend.c
===================================================================
--- gcc/cp/friend.c	(revision 248242)
+++ gcc/cp/friend.c	(working copy)
@@ -32,7 +32,7 @@  along with GCC; see the file COPYING3.  If not see
    template overload resolution results when accessibility matters
    (e.g. tests for an accessible member).  */
 
-static tree global_friend;
+static GTY(()) tree global_friend;
 
 /* Set the GLOBAL_FRIEND for this compilation session.  It might be
    set multiple times, but always to the same scope.  */
@@ -668,3 +668,5 @@  do_friend (tree ctype, tree declarator, tree decl,
 
   return decl;
 }
+
+#include "gt-cp-friend.h"
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 248242)
+++ gcc/cp/init.c	(working copy)
@@ -46,6 +46,8 @@  static tree dfs_initialize_vtbl_ptrs (tree, void *
 static tree build_field_list (tree, tree, int *);
 static int diagnose_uninitialized_cst_or_ref_member_1 (tree, tree, bool, bool);
 
+static GTY(()) tree fn;
+
 /* We are about to generate some complex initialization code.
    Conceptually, it is all a single expression.  However, we may want
    to include conditionals, loops, and other such statement-level
@@ -2402,7 +2404,6 @@  diagnose_uninitialized_cst_or_ref_member (tree typ
 tree
 throw_bad_array_new_length (void)
 {
-  static tree fn;
   if (!fn)
     {
       tree name = get_identifier ("__cxa_throw_bad_array_new_length");
@@ -4911,3 +4912,5 @@  build_vec_delete (tree base, tree maxindex,
 
   return rval;
 }
+
+#include "gt-cp-init.h"
Index: gcc/cp/lambda.c
===================================================================
--- gcc/cp/lambda.c	(revision 248242)
+++ gcc/cp/lambda.c	(working copy)
@@ -427,6 +427,9 @@  build_capture_proxy (tree member)
   return var;
 }
 
+static GTY(()) tree ptr_id;
+static GTY(()) tree max_id;
+
 /* Return a struct containing a pointer and a length for lambda capture of
    an array of runtime length.  */
 
@@ -433,7 +436,6 @@  build_capture_proxy (tree member)
 static tree
 vla_capture_type (tree array_type)
 {
-  static tree ptr_id, max_id;
   tree type = xref_tag (record_type, make_anon_name (), ts_current, false);
   xref_basetypes (type, NULL_TREE);
   type = begin_class_definition (type);
@@ -1248,3 +1250,5 @@  is_lambda_ignored_entity (tree val)
 
   return false;
 }
+
+#include "gt-cp-lambda.h"
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 248242)
+++ gcc/cp/typeck.c	(working copy)
@@ -4366,6 +4366,29 @@  cp_build_binary_op (location_t location,
     case FLOOR_DIV_EXPR:
     case ROUND_DIV_EXPR:
     case EXACT_DIV_EXPR:
+      if (TREE_CODE (op0) == SIZEOF_EXPR && TREE_CODE (op1) == SIZEOF_EXPR)
+	{
+	  tree type0 = TREE_OPERAND (op0, 0);
+	  tree type1 = TREE_OPERAND (op1, 0);
+	  tree first_arg = type0;
+	  if (!TYPE_P (type0))
+	    type0 = TREE_TYPE (type0);
+	  if (!TYPE_P (type1))
+	    type1 = TREE_TYPE (type1);
+	  if (POINTER_TYPE_P (type0) && same_type_p (TREE_TYPE (type0), type1)
+	      && !(TREE_CODE (first_arg) == PARM_DECL
+		   && DECL_ARRAY_PARAMETER_P (first_arg)
+		   && warn_sizeof_array_argument)
+	      && (complain & tf_warning))
+	    if (warning_at (location, OPT_Wsizeof_pointer_div,
+			    "division %<sizeof (%T) / sizeof (%T)%> does "
+			    "not compute the number of array elements",
+			    type0, type1))
+	      if (DECL_P (first_arg))
+		inform (DECL_SOURCE_LOCATION (first_arg),
+			"first %<sizeof%> operand was declared here");
+	}
+
       if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE
 	   || code0 == COMPLEX_TYPE || code0 == VECTOR_TYPE)
 	  && (code1 == INTEGER_TYPE || code1 == REAL_TYPE