Patchwork Backport fix for PR 48190 (excess memory in dwarf2 location tracking)

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 24, 2011, 3:48 p.m.
Message ID <g4k46pplmx.fsf@richards-thinkpad.stglab.manchester.uk.ibm.com>
Download mbox | patch
Permalink /patch/127547/
State New
Headers show

Comments

Richard Sandiford - Nov. 24, 2011, 3:48 p.m.
Is it OK to backport the fix for PR48190:

   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00255.html

to 4.5 and 4.6?  It fixes a case of excessive memory usage in dwarf2
location tracking, to the extent that the testcase blows VM on most
32-bit hosts.

For the record, Richard OKed it here:

   http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01933.html

but I thought that was only for trunk, so I pinged the backport here:

   http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00201.html

Unfortunately, I dropped the ball at that point and didn't ping again.
I'm resurrecting it now because AIUI both Fedora and Debian would like
to see it backported.

I've attached both versions below.  The 4.5 one includes a backport of:

	* dwarf2out.c (dw_loc_list_node): Add emitted field.
	(output_loc_list): Return immediately if emitted is set, set it.

from Jakub's 2010-10-12 commit.  OK for both?

Thanks,
Richard
Jakub Jelinek - Nov. 24, 2011, 3:56 p.m.
On Thu, Nov 24, 2011 at 03:48:06PM +0000, Richard Sandiford wrote:
> from Jakub's 2010-10-12 commit.  OK for both?

Yes.

	Jakub

Patch

gcc/
	Backport from mainline:

	2011-03-29  Richard Sandiford  <richard.sandiford@linaro.org>

	PR debug/48190
	* dwarf2out.c (dw_loc_list_node): Add resolved_addr and replaced.
	(cached_dw_loc_list_def): New structure.
	(cached_dw_loc_list): New typedef.
	(cached_dw_loc_list_table): New variable.
	(cached_dw_loc_list_table_hash): New function.
	(cached_dw_loc_list_table_eq): Likewise.
	(add_location_or_const_value_attribute): Take a bool cache_p.
	Cache the list when the parameter is true.
	(gen_formal_parameter_die): Update caller.
	(gen_variable_die): Likewise.
	(dwarf2out_finish): Likewise.
	(dwarf2out_abstract_function): Nullify cached_dw_loc_list_table
	while generating debug info for the decl.
	(dwarf2out_function_decl): Clear cached_dw_loc_list_table.
	(dwarf2out_init): Initialize cached_dw_loc_list_table.
	(resolve_addr): Cache the result of resolving a chain of
	location lists.

	From: 2010-10-12  Jakub Jelinek  <jakub@redhat.com>
	* dwarf2out.c (dw_loc_list_node): Add emitted field.
	(output_loc_list): Return immediately if emitted is set, set it.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	2011-08-24 18:00:45.000000000 +0100
+++ gcc/dwarf2out.c	2011-11-24 15:41:15.933007220 +0000
@@ -4254,6 +4254,12 @@  typedef struct GTY(()) dw_loc_list_struc
 		      Only on head of list */
   const char *section; /* Section this loclist is relative to */
   dw_loc_descr_ref expr;
+  /* True if all addresses in this and subsequent lists are known to be
+     resolved.  */
+  bool resolved_addr;
+  /* True if this list has been replaced by dw_loc_next.  */
+  bool replaced;
+  bool emitted;
 } dw_loc_list_node;
 
 #if defined (DWARF2_DEBUGGING_INFO) || defined (DWARF2_UNWIND_INFO)
@@ -5757,6 +5763,19 @@  typedef struct var_loc_list_def var_loc_
 /* Table of decl location linked lists.  */
 static GTY ((param_is (var_loc_list))) htab_t decl_loc_table;
 
+/* A cached location list.  */
+struct GTY (()) cached_dw_loc_list_def {
+  /* The DECL_UID of the decl that this entry describes.  */
+  unsigned int decl_id;
+
+  /* The cached location list.  */
+  dw_loc_list_ref loc_list;
+};
+typedef struct cached_dw_loc_list_def cached_dw_loc_list;
+
+/* Table of cached location lists.  */
+static GTY ((param_is (cached_dw_loc_list))) htab_t cached_dw_loc_list_table;
+
 /* A pointer to the base of a list of references to DIE's that
    are uniquely identified by their tag, presence/absence of
    children DIE's, and list of attribute/value pairs.  */
@@ -6110,7 +6129,7 @@  static bool add_const_value_attribute (d
 static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *);
 static void insert_float (const_rtx, unsigned char *);
 static rtx rtl_for_decl_location (tree);
-static bool add_location_or_const_value_attribute (dw_die_ref, tree,
+static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool,
 						   enum dwarf_attribute);
 static bool tree_add_const_value_attribute (dw_die_ref, tree);
 static bool tree_add_const_value_attribute_for_decl (dw_die_ref, tree);
@@ -7729,6 +7748,24 @@  lookup_decl_loc (const_tree decl)
     htab_find_with_hash (decl_loc_table, decl, DECL_UID (decl));
 }
 
+/* Returns a hash value for X (which really is a cached_dw_loc_list_list).  */
+
+static hashval_t
+cached_dw_loc_list_table_hash (const void *x)
+{
+  return (hashval_t) ((const cached_dw_loc_list *) x)->decl_id;
+}
+
+/* Return nonzero if decl_id of cached_dw_loc_list X is the same as
+   UID of decl *Y.  */
+
+static int
+cached_dw_loc_list_table_eq (const void *x, const void *y)
+{
+  return (((const cached_dw_loc_list *) x)->decl_id
+	  == DECL_UID ((const_tree) y));
+}
+
 /* Equate a DIE to a particular declaration.  */
 
 static void
@@ -10353,6 +10390,10 @@  output_loc_list (dw_loc_list_ref list_he
 {
   dw_loc_list_ref curr = list_head;
 
+  if (list_head->emitted)
+    return;
+  list_head->emitted = true;
+
   ASM_OUTPUT_LABEL (asm_out_file, list_head->ll_symbol);
 
   /* Walk the location list, and output each range + expression.  */
@@ -15918,15 +15959,22 @@  fortran_common (tree decl, HOST_WIDE_INT
    these things can crop up in other ways also.)  Note that one type of
    constant value which can be passed into an inlined function is a constant
    pointer.  This can happen for example if an actual argument in an inlined
-   function call evaluates to a compile-time constant address.  */
+   function call evaluates to a compile-time constant address.
+
+   CACHE_P is true if it is worth caching the location list for DECL,
+   so that future calls can reuse it rather than regenerate it from scratch.
+   This is true for BLOCK_NONLOCALIZED_VARS in inlined subroutines,
+   since we will need to refer to them each time the function is inlined.  */
 
 static bool
-add_location_or_const_value_attribute (dw_die_ref die, tree decl,
+add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p,
 				       enum dwarf_attribute attr)
 {
   rtx rtl;
   dw_loc_list_ref list;
   var_loc_list *loc_list;
+  cached_dw_loc_list *cache;
+  void **slot;
 
   if (TREE_CODE (decl) == ERROR_MARK)
     return false;
@@ -15962,7 +16010,34 @@  add_location_or_const_value_attribute (d
 	  && add_const_value_attribute (die, rtl))
 	 return true;
     }
-  list = loc_list_from_tree (decl, decl_by_reference_p (decl) ? 0 : 2);
+  /* If this decl is from BLOCK_NONLOCALIZED_VARS, we might need its
+     list several times.  See if we've already cached the contents.  */
+  list = NULL;
+  if (loc_list == NULL || cached_dw_loc_list_table == NULL)
+    cache_p = false;
+  if (cache_p)
+    {
+      cache = (cached_dw_loc_list *)
+	htab_find_with_hash (cached_dw_loc_list_table, decl, DECL_UID (decl));
+      if (cache)
+	list = cache->loc_list;
+    }
+  if (list == NULL)
+    {
+      list = loc_list_from_tree (decl, decl_by_reference_p (decl) ? 0 : 2);
+      /* It is usually worth caching this result if the decl is from
+	 BLOCK_NONLOCALIZED_VARS and if the list has at least two elements.  */
+      if (cache_p && list && list->dw_loc_next)
+	{
+	  slot = htab_find_slot_with_hash (cached_dw_loc_list_table, decl,
+					   DECL_UID (decl), INSERT);
+	  cache = (cached_dw_loc_list *)
+	    ggc_alloc_cleared (sizeof (cached_dw_loc_list));
+	  cache->decl_id = DECL_UID (decl);
+	  cache->loc_list = list;
+	  *slot = cache;
+	}
+    }
   if (list)
     {
       add_AT_location_description (die, attr, list);
@@ -17530,7 +17605,7 @@  gen_formal_parameter_die (tree node, tre
         equate_decl_number_to_die (node, parm_die);
       if (! DECL_ABSTRACT (node_or_origin))
 	add_location_or_const_value_attribute (parm_die, node_or_origin,
-					       DW_AT_location);
+					       node == NULL, DW_AT_location);
 
       break;
 
@@ -17710,6 +17785,7 @@  dwarf2out_abstract_function (tree decl)
   tree context;
   int was_abstract;
   htab_t old_decl_loc_table;
+  htab_t old_cached_dw_loc_list_table;
 
   /* Make sure we have the actual abstract inline, not a clone.  */
   decl = DECL_ORIGIN (decl);
@@ -17723,7 +17799,9 @@  dwarf2out_abstract_function (tree decl)
      DIE.  Be sure to not clobber the outer location table nor use it or we would
      get locations in abstract instantces.  */
   old_decl_loc_table = decl_loc_table;
+  old_cached_dw_loc_list_table = cached_dw_loc_list_table;
   decl_loc_table = NULL;
+  cached_dw_loc_list_table = NULL;
 
   /* Be sure we've emitted the in-class declaration DIE (if any) first, so
      we don't get confused by DECL_ABSTRACT.  */
@@ -17748,6 +17826,7 @@  dwarf2out_abstract_function (tree decl)
 
   current_function_decl = save_fn;
   decl_loc_table = old_decl_loc_table;
+  cached_dw_loc_list_table = old_cached_dw_loc_list_table;
   pop_cfun ();
 }
 
@@ -18429,9 +18508,8 @@  gen_variable_die (tree decl, tree origin
           && !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl_or_origin)))
 	defer_location (decl_or_origin, var_die);
       else
-        add_location_or_const_value_attribute (var_die,
-					       decl_or_origin,
-					       DW_AT_location);
+        add_location_or_const_value_attribute (var_die, decl_or_origin,
+					       decl == NULL, DW_AT_location);
       add_pubname (decl_or_origin, var_die);
     }
   else
@@ -20037,6 +20115,7 @@  dwarf2out_function_decl (tree decl)
   dwarf2out_decl (decl);
 
   htab_empty (decl_loc_table);
+  htab_empty (cached_dw_loc_list_table);
 }
 
 /* Output a marker (i.e. a label) for the beginning of the generated code for
@@ -20637,6 +20716,11 @@  dwarf2out_init (const char *filename ATT
   decl_loc_table = htab_create_ggc (10, decl_loc_table_hash,
 				    decl_loc_table_eq, NULL);
 
+  /* Allocate the cached_dw_loc_list_table.  */
+  cached_dw_loc_list_table
+    = htab_create_ggc (10, cached_dw_loc_list_table_hash,
+		       cached_dw_loc_list_table_eq, NULL);
+
   /* Allocate the initial hunk of the decl_scope_table.  */
   decl_scope_table = VEC_alloc (tree, gc, 256);
 
@@ -21261,30 +21345,53 @@  resolve_addr (dw_die_ref die)
 {
   dw_die_ref c;
   dw_attr_ref a;
-  dw_loc_list_ref *curr;
+  dw_loc_list_ref *curr, *start, loc;
   unsigned ix;
 
   for (ix = 0; VEC_iterate (dw_attr_node, die->die_attr, ix, a); ix++)
     switch (AT_class (a))
       {
       case dw_val_class_loc_list:
-	curr = AT_loc_list_ptr (a);
-	while (*curr)
+	start = curr = AT_loc_list_ptr (a);
+	loc = *curr;
+	gcc_assert (loc);
+	/* The same list can be referenced more than once.  See if we have
+	   already recorded the result from a previous pass.  */
+	if (loc->replaced)
+	  *curr = loc->dw_loc_next;
+	else if (!loc->resolved_addr)
 	  {
-	    if (!resolve_addr_in_expr ((*curr)->expr))
+	    /* As things stand, we do not expect or allow one die to
+	       reference a suffix of another die's location list chain.
+	       References must be identical or completely separate.
+	       There is therefore no need to cache the result of this
+	       pass on any list other than the first; doing so
+	       would lead to unnecessary writes.  */
+	    while (*curr)
 	      {
-		dw_loc_list_ref next = (*curr)->dw_loc_next;
-		if (next && (*curr)->ll_symbol)
+		gcc_assert (!(*curr)->replaced && !(*curr)->resolved_addr);
+		if (!resolve_addr_in_expr ((*curr)->expr))
 		  {
-		    gcc_assert (!next->ll_symbol);
-		    next->ll_symbol = (*curr)->ll_symbol;
+		    dw_loc_list_ref next = (*curr)->dw_loc_next;
+		    if (next && (*curr)->ll_symbol)
+		      {
+			gcc_assert (!next->ll_symbol);
+			next->ll_symbol = (*curr)->ll_symbol;
+		      }
+		    *curr = next;
 		  }
-		*curr = next;
+		else
+		  curr = &(*curr)->dw_loc_next;
 	      }
+	    if (loc == *start)
+	      loc->resolved_addr = 1;
 	    else
-	      curr = &(*curr)->dw_loc_next;
+	      {
+		loc->replaced = 1;
+		loc->dw_loc_next = *start;
+	      }
 	  }
-	if (!AT_loc_list (a))
+	if (!*start)
 	  {
 	    remove_AT (die, a->dw_attr);
 	    ix--;
@@ -21344,6 +21451,7 @@  dwarf2out_finish (const char *filename)
       add_location_or_const_value_attribute (
         VEC_index (deferred_locations, deferred_locations_list, i)->die,
         VEC_index (deferred_locations, deferred_locations_list, i)->variable,
+	false,
 	DW_AT_location);
     }